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

Hal Finkel hfinkel at anl.gov
Tue Oct 14 12:23:07 PDT 2014


Author: hfinkel
Date: Tue Oct 14 14:23:07 2014
New Revision: 219708

URL: http://llvm.org/viewvc/llvm-project?rev=219708&view=rev
Log:
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.

Added:
    llvm/trunk/test/Transforms/InstSimplify/fold-builtin-fma.ll
Modified:
    llvm/trunk/lib/Support/APFloat.cpp

Modified: llvm/trunk/lib/Support/APFloat.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=219708&r1=219707&r2=219708&view=diff
==============================================================================
--- llvm/trunk/lib/Support/APFloat.cpp (original)
+++ llvm/trunk/lib/Support/APFloat.cpp Tue Oct 14 14:23:07 2014
@@ -954,7 +954,7 @@ APFloat::multiplySignificand(const APFlo
   // 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
      extended-precision calculation.  */
   if (isFiniteNonZero() &&
       multiplicand.isFiniteNonZero() &&
-      addend.isFiniteNonZero()) {
+      addend.isFinite()) {
     lostFraction lost_fraction;
 
     lost_fraction = multiplySignificand(multiplicand, &addend);

Added: llvm/trunk/test/Transforms/InstSimplify/fold-builtin-fma.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/fold-builtin-fma.ll?rev=219708&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstSimplify/fold-builtin-fma.ll (added)
+++ llvm/trunk/test/Transforms/InstSimplify/fold-builtin-fma.ll Tue Oct 14 14:23:07 2014
@@ -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
+





More information about the llvm-commits mailing list