[llvm] r216914 - Revert: [APFloat] Fixed a bug in method 'fusedMultiplyAdd'.
Hal Finkel
hfinkel at anl.gov
Tue Oct 14 12:34:46 PDT 2014
Hi Uli,
Thanks very much for looking into this! I've reapplied this patch in r219708, with a fix for the issue you've detailed (and which Valgrind confirmed).
-Hal
----- Original Message -----
> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Andrea Di Biagio" <andrea.dibiagio at gmail.com>, "Andrea Di Biagio" <Andrea_DiBiagio at sn.scee.net>, "Gabor
> Ballabas" <gaborb at inf.u-szeged.hu>, "Dmitri Gribenko" <gribozavr at gmail.com>, "llvm-commits at cs.uiuc.edu for LLVM"
> <llvm-commits at cs.uiuc.edu>, "Nick Lewycky" <nicholas at mxc.ca>
> Sent: Monday, October 13, 2014 7:41:57 AM
> Subject: Re: [llvm] r216914 - Revert: [APFloat] Fixed a bug in method 'fusedMultiplyAdd'.
>
> 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
>
>
>
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list