Revert "r216914 - Revert: [APFloat] Fixed a bug in method 'fusedMultiplyAdd'"

Reapply r216913, a fix for PR20832 by Andrea Di Biagio. The commit was reverted
because of buildbot failures, and credit goes to Ulrich Weigand for isolating
the underlying issue (which can be confirmed by Valgrind, which does helpfully
light up like the fourth of July). Uli explained the problem with the original
patch as:

  It seems the problem is calling multiplySignificand with an addend of category
  fcZero; that is not expected by this routine.  Note that for fcZero, the
  significand parts are simply uninitialized, but the code in (or rather, called
  from) multiplySignificand will unconditionally access them -- in effect using
  uninitialized contents.

This version avoids using a category == fcZero addend within
multiplySignificand, which avoids this problem (the Valgrind output is also now
clean).

Original commit message:

[APFloat] Fixed a bug in method 'fusedMultiplyAdd'.

When folding a fused multiply-add builtin call, make sure that we propagate the
correct result in the case where the addend is zero, and the two other operands
are finite non-zero.

Example:
  define double @test() {
    %1 = call double @llvm.fma.f64(double 7.0, double 8.0, double 0.0)
    ret double %1
  }

Before this patch, the instruction simplifier wrongly folded the builtin call
in function @test to constant 'double 7.0'.
With this patch, method 'fusedMultiplyAdd' correctly evaluates the multiply and
propagates the expected result (i.e. 56.0).

Added test fold-builtin-fma.ll with the reproducible from PR20832 plus extra
test cases to verify the behavior of method 'fusedMultiplyAdd' in the presence
of NaN/Inf operands.

This fixes PR20832.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@219708 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Support/APFloat.cpp b/lib/Support/APFloat.cpp
index 57fbe897fc87f0d4909cb22db5ac9a4f62114e77..f63d7748973c74262842cdc2809acde8290718a0 100644 (file)
--- a/lib/Support/APFloat.cpp
+++ b/lib/Support/APFloat.cpp
@@ -954,7 +954,7 @@ APFloat::multiplySignificand(const APFloat &rhs, const APFloat *addend)
   // exponent accordingly.
   exponent += 1;
 
-  if (addend) {
+  if (addend && addend->isNonZero()) {
     // The intermediate result of the multiplication has "2 * precision" 
     // signicant bit; adjust the addend to be consistent with mul result.
     //
@@ -1800,7 +1800,7 @@ APFloat::fusedMultiplyAdd(const APFloat &multiplicand,
      extended-precision calculation.  */
   if (isFiniteNonZero() &&
       multiplicand.isFiniteNonZero() &&
-      addend.isFiniteNonZero()) {
+      addend.isFinite()) {
     lostFraction lost_fraction;
 
     lost_fraction = multiplySignificand(multiplicand, &addend);

diff --git a/test/Transforms/InstSimplify/fold-builtin-fma.ll b/test/Transforms/InstSimplify/fold-builtin-fma.ll
new file mode 100644 (file)
index 0000000..6331b8c
--- /dev/null
+++ b/test/Transforms/InstSimplify/fold-builtin-fma.ll
@@ -0,0 +1,119 @@
+; RUN: opt -instsimplify -S < %s | FileCheck %s
+
+; Fixes PR20832
+; Make sure that we correctly fold a fused multiply-add where operands
+; are all finite constants and addend is zero.
+
+declare double @llvm.fma.f64(double, double, double)
+
+
+define double @PR20832()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 8.0, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @PR20832(
+; CHECK: ret double 5.600000e+01
+
+; Test builtin fma with all finite non-zero constants.
+define double @test_all_finite()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 8.0, double 5.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_all_finite(
+; CHECK: ret double 6.100000e+01
+
+; Test builtin fma with a +/-NaN addend.
+define double @test_NaN_addend()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 8.0, double 0x7FF8000000000000)
+  ret double %1
+}
+; CHECK-LABEL: @test_NaN_addend(
+; CHECK: ret double 0x7FF8000000000000
+
+define double @test_NaN_addend_2()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 8.0, double 0xFFF8000000000000)
+  ret double %1
+}
+; CHECK-LABEL: @test_NaN_addend_2(
+; CHECK: ret double 0xFFF8000000000000
+
+; Test builtin fma with a +/-Inf addend.
+define double @test_Inf_addend()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 8.0, double 0x7FF0000000000000)
+  ret double %1
+}
+; CHECK-LABEL: @test_Inf_addend(
+; CHECK: ret double 0x7FF0000000000000
+
+define double @test_Inf_addend_2()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 8.0, double 0xFFF0000000000000)
+  ret double %1
+}
+; CHECK-LABEL: @test_Inf_addend_2(
+; CHECK: ret double 0xFFF0000000000000
+
+; Test builtin fma with one of the operands to the multiply being +/-NaN.
+define double @test_NaN_1()  {
+  %1 = call double @llvm.fma.f64(double 0x7FF8000000000000, double 8.0, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_NaN_1(
+; CHECK: ret double 0x7FF8000000000000
+
+
+define double @test_NaN_2()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 0x7FF8000000000000, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_NaN_2(
+; CHECK: ret double 0x7FF8000000000000
+
+
+define double @test_NaN_3()  {
+  %1 = call double @llvm.fma.f64(double 0xFFF8000000000000, double 8.0, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_NaN_3(
+; CHECK: ret double 0x7FF8000000000000
+
+
+define double @test_NaN_4()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 0xFFF8000000000000, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_NaN_4(
+; CHECK: ret double 0x7FF8000000000000
+
+
+; Test builtin fma with one of the operands to the multiply being +/-Inf.
+define double @test_Inf_1()  {
+  %1 = call double @llvm.fma.f64(double 0x7FF0000000000000, double 8.0, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_Inf_1(
+; CHECK: ret double 0x7FF0000000000000
+
+
+define double @test_Inf_2()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 0x7FF0000000000000, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_Inf_2(
+; CHECK: ret double 0x7FF0000000000000
+
+
+define double @test_Inf_3()  {
+  %1 = call double @llvm.fma.f64(double 0xFFF0000000000000, double 8.0, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_Inf_3(
+; CHECK: ret double 0xFFF0000000000000
+
+
+define double @test_Inf_4()  {
+  %1 = call double @llvm.fma.f64(double 7.0, double 0xFFF0000000000000, double 0.0)
+  ret double %1
+}
+; CHECK-LABEL: @test_Inf_4(
+; CHECK: ret double 0xFFF0000000000000
+