spatel created this revision.
spatel added reviewers: lhames, scanon, hfinkel, klimek.
spatel added a subscriber: cfe-commits.

We don't want to generate fmuladd if there's a use of the fmul expression, but 
this shouldn't be an assert.

The test case is derived from the commit message for r253337:
http://reviews.llvm.org/rL253337

That commit reverted r253269:
http://reviews.llvm.org/rL253269

...but the bug exists independently of the default fp-contract setting. It just 
became easier to hit with that change.

PR25719:
https://llvm.org/bugs/show_bug.cgi?id=25719

http://reviews.llvm.org/D15165

Files:
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/fp-contract-pragma.cpp

Index: test/CodeGen/fp-contract-pragma.cpp
===================================================================
--- test/CodeGen/fp-contract-pragma.cpp
+++ test/CodeGen/fp-contract-pragma.cpp
@@ -62,3 +62,15 @@
   return a * b + c;
 }
 
+// If the multiply has multiple uses, don't produce fmuladd.
+// This used to assert (PR25719):
+// https://llvm.org/bugs/show_bug.cgi?id=25719
+
+float fp_contract_7(float a, float b, float c) {
+// CHECK: _Z13fp_contract_7fff
+// CHECK:  %mul = fmul float %b, 2.000000e+00
+// CHECK-NEXT: fsub float %mul, %c
+  #pragma STDC FP_CONTRACT ON
+  return (a = 2 * b) - c;
+}
+
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2564,17 +2564,17 @@
     return nullptr;
 
   // We have a potentially fusable op. Look for a mul on one of the operands.
+  // Also, make sure that the mul result isn't used directly. In that case,
+  // there's no point creating a muladd operation.
   if (llvm::BinaryOperator* LHSBinOp = dyn_cast<llvm::BinaryOperator>(op.LHS)) 
{
-    if (LHSBinOp->getOpcode() == llvm::Instruction::FMul) {
-      assert(LHSBinOp->getNumUses() == 0 &&
-             "Operations with multiple uses shouldn't be contracted.");
+    if (LHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+        LHSBinOp->getNumUses() == 0) {
       return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, false, isSub);
     }
   } else if (llvm::BinaryOperator* RHSBinOp =
                dyn_cast<llvm::BinaryOperator>(op.RHS)) {
-    if (RHSBinOp->getOpcode() == llvm::Instruction::FMul) {
-      assert(RHSBinOp->getNumUses() == 0 &&
-             "Operations with multiple uses shouldn't be contracted.");
+    if (RHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+        RHSBinOp->getNumUses() == 0) {
       return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub, false);
     }
   }


Index: test/CodeGen/fp-contract-pragma.cpp
===================================================================
--- test/CodeGen/fp-contract-pragma.cpp
+++ test/CodeGen/fp-contract-pragma.cpp
@@ -62,3 +62,15 @@
   return a * b + c;
 }
 
+// If the multiply has multiple uses, don't produce fmuladd.
+// This used to assert (PR25719):
+// https://llvm.org/bugs/show_bug.cgi?id=25719
+
+float fp_contract_7(float a, float b, float c) {
+// CHECK: _Z13fp_contract_7fff
+// CHECK:  %mul = fmul float %b, 2.000000e+00
+// CHECK-NEXT: fsub float %mul, %c
+  #pragma STDC FP_CONTRACT ON
+  return (a = 2 * b) - c;
+}
+
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2564,17 +2564,17 @@
     return nullptr;
 
   // We have a potentially fusable op. Look for a mul on one of the operands.
+  // Also, make sure that the mul result isn't used directly. In that case,
+  // there's no point creating a muladd operation.
   if (llvm::BinaryOperator* LHSBinOp = dyn_cast<llvm::BinaryOperator>(op.LHS)) {
-    if (LHSBinOp->getOpcode() == llvm::Instruction::FMul) {
-      assert(LHSBinOp->getNumUses() == 0 &&
-             "Operations with multiple uses shouldn't be contracted.");
+    if (LHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+        LHSBinOp->getNumUses() == 0) {
       return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, false, isSub);
     }
   } else if (llvm::BinaryOperator* RHSBinOp =
                dyn_cast<llvm::BinaryOperator>(op.RHS)) {
-    if (RHSBinOp->getOpcode() == llvm::Instruction::FMul) {
-      assert(RHSBinOp->getNumUses() == 0 &&
-             "Operations with multiple uses shouldn't be contracted.");
+    if (RHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+        RHSBinOp->getNumUses() == 0) {
       return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub, false);
     }
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to