On Fri, 2014-09-26 at 13:36 -0500, Hal Finkel wrote:> ----- Original Message ----- > > From: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com> > > To: "Stephen Canon" <scanon at apple.com> > > Cc: spatel+llvm at rotateright.com, "Will Schmidt" <will_schmidt at vnet.ibm.com>, "LLVM Developers Mailing List" > > <llvmdev at cs.uiuc.edu> > > Sent: Friday, September 26, 2014 1:23:16 PM > > Subject: Re: [LLVMdev] Optimization of sqrt() with invalid argument > > > > On Fri, 2014-09-26 at 12:09 -0400, Stephen Canon wrote: > > > > On Sep 26, 2014, at 11:59 AM, Tim Northover > > > > <t.p.northover at gmail.com> wrote: > > > > > > > >> I know it's part of test-suite/external, but this constant fold > > > >> code has > > > >> been around 5+ years. Was the bug lying dormant all this time, > > > >> only visible > > > >> on PPC, or something else? > > > > > > > > My guess would be that people don't tend to run with -ffast-math > > > > (it's > > > > got a reputation for breaking code, even ignoring this particular > > > > issue). Without that, from what I can see the issue won't arise. > > > > > > If this can really only happen under fast-math, I take back what I > > > said entirely. IIRC, NaNs are explicitly not supported in that > > > mode, so all bets are off. Zero is a perfectly reasonable result. > > > > The result of this is that we return 0 only under -ffast-math. In > > all > > other cases, the sqrt call will remain and we will return NaN. So > > that > > seems like a bothersome discrepancy given that the Posix standard > > wording tends to favor NaN (though an implementation-defined value is > > allowable, regardless of -ffast-math). > > > > As mentioned earlier, a number of other compilers also generate NaN > > with > > -ffast-math or its equivalent. I believe this is done to comply with > > the Posix wording. > > > > Regardless of feelings about playing benchmark games (with which I > > have > > sympathy), people tend to compile many of the SPECfp benchmarks with > > -ffast-math so they can, e.g., use FMA instructions in their > > publishes. > > But this is a side issue, and I'm rather sorry I mentioned SPEC at > > all. > > This is really an issue of internal and external consistency. > > Yes, but Steve is right: -ffast-math implies -ffinite-math-only, which means that we assume no NaNs. I understand that people use -ffast-math to get vectorized reductions (just getting aggressive FMAs only requires -ffp-contract=fast), but this, unfortunately, is also a matter of internal consistency :(Hi Hal, Sure. Well, if the consensus is that we don't want to change this, then so be it -- but I expect I won't be the last person to raise the issue. At the least, we should constant-fold to NaN for the non fast-math sqrt calls...there's no point in going through the libcall overhead when we know the answer in advance. I can prepare a patch for that if everyone agrees. Thanks, Bill> > -Hal > > > > > Thanks, > > Bill > > > > > > > > – Steve > > > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > >
Tim Northover
2014-Sep-26 19:06 UTC
[LLVMdev] Optimization of sqrt() with invalid argument
> At the least, we should constant-fold to NaN for the non fast-math sqrt > calls...there's no point in going through the libcall overhead when we > know the answer in advance. I can prepare a patch for that if everyone > agrees.I thought we already did that, but apparently not. The only thing to beware of there is make sure it only triggers when -fno-math-errno has been passed. In the IR you'll be looking for a call to @sqrt (or variant) with the "readnone" attribute. Cheers. Tim.
Joerg Sonnenberger
2014-Sep-26 22:14 UTC
[LLVMdev] Optimization of sqrt() with invalid argument
On Fri, Sep 26, 2014 at 01:52:19PM -0500, Bill Schmidt wrote:> At the least, we should constant-fold to NaN for the non fast-math sqrt > calls...there's no point in going through the libcall overhead when we > know the answer in advance. I can prepare a patch for that if everyone > agrees.The crux is that sqrt(-1) can set errno, can't it? Joerg
Reid Kleckner
2014-Sep-26 22:23 UTC
[LLVMdev] Optimization of sqrt() with invalid argument
On Fri, Sep 26, 2014 at 3:14 PM, Joerg Sonnenberger <joerg at britannica.bec.de> wrote:> On Fri, Sep 26, 2014 at 01:52:19PM -0500, Bill Schmidt wrote: > > At the least, we should constant-fold to NaN for the non fast-math sqrt > > calls...there's no point in going through the libcall overhead when we > > know the answer in advance. I can prepare a patch for that if everyone > > agrees. > > The crux is that sqrt(-1) can set errno, can't it? >I'm pretty sure -ffast-math means you don't care about that. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140926/f5d6be5e/attachment.html>
----- Original Message -----> From: "Joerg Sonnenberger" <joerg at britannica.bec.de> > To: llvmdev at cs.uiuc.edu > Sent: Friday, September 26, 2014 5:14:54 PM > Subject: Re: [LLVMdev] Optimization of sqrt() with invalid argument > > On Fri, Sep 26, 2014 at 01:52:19PM -0500, Bill Schmidt wrote: > > At the least, we should constant-fold to NaN for the non fast-math > > sqrt > > calls...there's no point in going through the libcall overhead when > > we > > know the answer in advance. I can prepare a patch for that if > > everyone > > agrees. > > The crux is that sqrt(-1) can set errno, can't it?-ffast-math implies -fno-math-errno, but otherwise the default is target specific. On Linux, math calls can set errno by default, for example. On Darwin, they don't. On systems that can set errno, we indeed cannot constant fold the result unless we can prove the errno modification is not observable in a well-defined way. However, because we set different attributes on the functions depending on whether errno can be set or not, I think we already handle this correctly. -Hal> > Joerg > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory
----- Original Message -----> From: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com> > To: "Hal Finkel" <hfinkel at anl.gov> > Cc: spatel+llvm at rotateright.com, "Will Schmidt" <will_schmidt at vnet.ibm.com>, "LLVM Developers Mailing List" > <llvmdev at cs.uiuc.edu>, "Stephen Canon" <scanon at apple.com> > Sent: Friday, September 26, 2014 1:52:19 PM > Subject: Re: [LLVMdev] Optimization of sqrt() with invalid argument > > On Fri, 2014-09-26 at 13:36 -0500, Hal Finkel wrote: > > ----- Original Message ----- > > > From: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com> > > > To: "Stephen Canon" <scanon at apple.com> > > > Cc: spatel+llvm at rotateright.com, "Will Schmidt" > > > <will_schmidt at vnet.ibm.com>, "LLVM Developers Mailing List" > > > <llvmdev at cs.uiuc.edu> > > > Sent: Friday, September 26, 2014 1:23:16 PM > > > Subject: Re: [LLVMdev] Optimization of sqrt() with invalid > > > argument > > > > > > On Fri, 2014-09-26 at 12:09 -0400, Stephen Canon wrote: > > > > > On Sep 26, 2014, at 11:59 AM, Tim Northover > > > > > <t.p.northover at gmail.com> wrote: > > > > > > > > > >> I know it's part of test-suite/external, but this constant > > > > >> fold > > > > >> code has > > > > >> been around 5+ years. Was the bug lying dormant all this > > > > >> time, > > > > >> only visible > > > > >> on PPC, or something else? > > > > > > > > > > My guess would be that people don't tend to run with > > > > > -ffast-math > > > > > (it's > > > > > got a reputation for breaking code, even ignoring this > > > > > particular > > > > > issue). Without that, from what I can see the issue won't > > > > > arise. > > > > > > > > If this can really only happen under fast-math, I take back > > > > what I > > > > said entirely. IIRC, NaNs are explicitly not supported in that > > > > mode, so all bets are off. Zero is a perfectly reasonable > > > > result. > > > > > > The result of this is that we return 0 only under -ffast-math. > > > In > > > all > > > other cases, the sqrt call will remain and we will return NaN. > > > So > > > that > > > seems like a bothersome discrepancy given that the Posix standard > > > wording tends to favor NaN (though an implementation-defined > > > value is > > > allowable, regardless of -ffast-math). > > > > > > As mentioned earlier, a number of other compilers also generate > > > NaN > > > with > > > -ffast-math or its equivalent. I believe this is done to comply > > > with > > > the Posix wording. > > > > > > Regardless of feelings about playing benchmark games (with which > > > I > > > have > > > sympathy), people tend to compile many of the SPECfp benchmarks > > > with > > > -ffast-math so they can, e.g., use FMA instructions in their > > > publishes. > > > But this is a side issue, and I'm rather sorry I mentioned SPEC > > > at > > > all. > > > This is really an issue of internal and external consistency. > > > > Yes, but Steve is right: -ffast-math implies -ffinite-math-only, > > which means that we assume no NaNs. I understand that people use > > -ffast-math to get vectorized reductions (just getting aggressive > > FMAs only requires -ffp-contract=fast), but this, unfortunately, > > is also a matter of internal consistency :( > > Hi Hal, > > Sure. Well, if the consensus is that we don't want to change this, > then > so be it -- but I expect I won't be the last person to raise the > issue.I'm sure that's true ;) -- and I actually don't have a strong opinion here. The user already violated the contract when they wrote sqrt(-2) and compiled with -ffast-math.> > At the least, we should constant-fold to NaN for the non fast-math > sqrt > calls...there's no point in going through the libcall overhead when > we > know the answer in advance. I can prepare a patch for that if > everyone > agrees.Yes, I think that's fine. We can only do this when the function is marked as not having side effects anyway (so when -fno-math-errno is in effect). -Hal> > Thanks, > Bill > > > > > -Hal > > > > > > > > Thanks, > > > Bill > > > > > > > > > > > – Steve > > > > > > > > > _______________________________________________ > > > LLVM Developers mailing list > > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > > > > >-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory