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

Ulrich Weigand Ulrich.Weigand at de.ibm.com
Mon Oct 13 05:41:57 PDT 2014


Hi Hal,

I've applied the r216913 patch to current mainline, and can confirm that it
is failing on SystemZ.

>From looking at the patch, 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 could explain wildly varying test case results ...

I guess we'd either have to change fusedMultiplyAdd to pass a nullptr
addend to multiplySignificand if the addend is zero, or else update
multiplySignificand to handle zero addends correctly.


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand | Phone: +49-7031/16-3727
  STSM, GNU/Linux compilers and toolchain
  IBM Deutschland Research & Development GmbH
  Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk
Wittkopp
  Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht
Stuttgart, HRB 243294



From:	Hal Finkel <hfinkel at anl.gov>
To:	Andrea Di Biagio <andrea.dibiagio at gmail.com>
Cc:	Andrea Di Biagio <Andrea_DiBiagio at sn.scee.net>,
            "llvm-commits at cs.uiuc.edu for LLVM" <llvm-commits at cs.uiuc.edu>,
            Dmitri Gribenko <gribozavr at gmail.com>, Nick Lewycky
            <nicholas at mxc.ca>, Ulrich Weigand/Germany/IBM at IBMDE, Gabor
            Ballabas <gaborb at inf.u-szeged.hu>
Date:	10.10.2014 23:53
Subject:	Re: [llvm] r216914 - Revert: [APFloat] Fixed a bug in method
            'fusedMultiplyAdd'.



Hi,

I'd really like to get this fixed. Can the folks with builders please help
with this?

 -Hal

----- Original Message -----
> From: "Andrea Di Biagio" <andrea.dibiagio at gmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Andrea Di Biagio" <Andrea_DiBiagio at sn.scee.net>,
"llvm-commits at cs.uiuc.edu for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Tuesday, September 16, 2014 8:03:55 AM
> Subject: Re: [llvm] r216914 - Revert: [APFloat] Fixed a bug in method
'fusedMultiplyAdd'.
>
> Here is the list of buildbots that reported failures due to revision
> 216913:
> - clang-x86_64-debian-fast
> - llvm-s390x-linux1
> - llvm-aarch64-linux
> - clang-x86_64-ubuntu-gdb-75
> - clang-x86_64-linux-selfhost-rel
>
> I am only able access the build reports for the
> "clang-x86_64-linux-selfhost-rel" here:
>
http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-rel/builds/4191

>
>
> On Tue, Sep 16, 2014 at 1:11 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > ----- Original Message -----
> >> From: "Andrea Di Biagio" <andrea.dibiagio at gmail.com>
> >> To: "Hal Finkel" <hfinkel at anl.gov>
> >> Cc: "Andrea Di Biagio" <Andrea_DiBiagio at sn.scee.net>,
> >> "llvm-commits at cs.uiuc.edu for LLVM" <llvm-commits at cs.uiuc.edu>
> >> Sent: Tuesday, September 16, 2014 4:30:42 AM
> >> Subject: Re: [llvm] r216914 - Revert: [APFloat] Fixed a bug in
> >> method 'fusedMultiplyAdd'.
> >>
> >> On Tue, Sep 16, 2014 at 5:50 AM, Hal Finkel <hfinkel at anl.gov>
> >> wrote:
> >> > ----- Original Message -----
> >> >> From: "Andrea Di Biagio" <Andrea_DiBiagio at sn.scee.net>
> >> >> To: llvm-commits at cs.uiuc.edu
> >> >> Sent: Tuesday, September 2, 2014 12:22:50 PM
> >> >> Subject: [llvm] r216914 - Revert: [APFloat] Fixed a bug in
> >> >> method
> >> >>     'fusedMultiplyAdd'.
> >> >>
> >> >> Author: adibiagio
> >> >> Date: Tue Sep  2 12:22:49 2014
> >> >> New Revision: 216914
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=216914&view=rev
> >> >> Log:
> >> >> Revert: [APFloat] Fixed a bug in method 'fusedMultiplyAdd'.
> >> >>
> >> >> This reverts revision 216913; the new test added at revision
> >> >> 216913
> >> >> caused regression failures on a couple of buildbots.
> >> >
> >> > Did we ever figure this out?
> >> >
> >> >  -Hal
> >>
> >> Unfortunately not..
> >> Basically, only for five buildbots the fused-multiply-add from
> >> test
> >> case @PR20832 (added by test fold-builtin-fma.ll) was folded to a
> >> completely bogus number. Interestingly, each failing buildbot
> >> found a
> >> different number for the failing test case. That really confused
> >> me:
> >> in case of an error, I was expecting _all_ the buildbots to fail
> >> with
> >> the same error; instead, only a few builbots failed each time with
> >> different results (I see if I can find the blame emails..
> >> Hopefully I
> >> didn't delete them).
> >>
> >> Anyway, I tried to reproduce the regression failure on a different
> >> x86
> >> machine (as far as I remember, three of the failing builbots were
> >> bootstrapping clang on a x86_64 with both host and target
> >> x86_64-unknown-linux-gnu). I tried on two other x86 machines with
> >> the
> >> same configuration. Unfortunately I was never able to reproduce
> >> the
> >> failure (it always worked for me).
> >
> > Strange. Which bots failed?
> >
> >  -Hal
> >
> >>
> >> >
> >> >>
> >> >>
> >> >> Removed:
> >> >>     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=216914&r1=216913&r2=216914&view=diff

> >> >>
==============================================================================

> >> >> --- llvm/trunk/lib/Support/APFloat.cpp (original)
> >> >> +++ llvm/trunk/lib/Support/APFloat.cpp Tue Sep  2 12:22:49 2014
> >> >> @@ -1801,7 +1801,7 @@ APFloat::fusedMultiplyAdd(const APFloat
> >> >>       extended-precision calculation.  */
> >> >>    if (isFiniteNonZero() &&
> >> >>        multiplicand.isFiniteNonZero() &&
> >> >> -      addend.isFinite()) {
> >> >> +      addend.isFiniteNonZero()) {
> >> >>      lostFraction lost_fraction;
> >> >>
> >> >>      lost_fraction = multiplySignificand(multiplicand,
> >> >>      &addend);
> >> >>
> >> >> Removed:
> >> >> 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=216913&view=auto

> >> >>
==============================================================================

> >> >> --- llvm/trunk/test/Transforms/InstSimplify/fold-builtin-fma.ll
> >> >> (original)
> >> >> +++ llvm/trunk/test/Transforms/InstSimplify/fold-builtin-fma.ll
> >> >> (removed)
> >> >> @@ -1,119 +0,0 @@
> >> >> -; 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
> >> >> -
> >> >>
> >> >>
> >> >> _______________________________________________
> >> >> llvm-commits mailing list
> >> >> llvm-commits at cs.uiuc.edu
> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >>
> >> >
> >> > --
> >> > Hal Finkel
> >> > Assistant Computational Scientist
> >> > Leadership Computing Facility
> >> > Argonne National Laboratory
> >> > _______________________________________________
> >> > llvm-commits mailing list
> >> > llvm-commits at cs.uiuc.edu
> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
>

--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory







More information about the llvm-commits mailing list