llvm dev - Sep 2014 - [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.

Thanks,
Bill
> 
> – Steve

----- 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 :(

 -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

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
> > 
>

This isn't purely a fast-math issue...ConstantFolding isn't using
enable-unsafe-fp-math to decide whether to emit the '0'. It's just
looking
for the llvm intrinsic rather than a call to sqrt:

  %0 = call double @llvm.sqrt.f64(double -2.000000e+00)

vs:

  %0 = call double @sqrt(double -2.000000e+00) #2

So how about a front-end fix:
If the parameter is a negative constant, instead of converting the sqrt
call into the intrinsic, just leave it as-is regardless of fast-math. I
think that change would provide internal and external consistency. And I
suspect this is the solution the other compilers have reached - they end up
generating a HW sqrt instruction even though they could have precomputed
the result.


On Fri, Sep 26, 2014 at 12:36 PM, Hal Finkel <hfinkel at anl.gov> 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 :(
>
>  -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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140926/ee49311f/attachment.html>