llvm dev - Nov 2013 - [LLVMdev] [RFC] How to fix sqrt vs llvm.sqrt optimization asymmetry

----- Original Message -----
> 
> On Nov 11, 2013, at 9:30 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> 
> > ----- Original Message -----
> >> Hi Hal, all.
> >> 
> >> I'm not sure why llvm.sqrt is 'special'.  Maybe
because there is a
> >> SSE
> >> packed sqrt instruction (SQRTPS) but not e.g. a packed sin
> >> instruction
> >> AFAIK.
> > 
> > This seems relevant:
> > http://lists.cs.uiuc.edu/pipermail/llvmdev/2007-August/010248.html
> > 
> > Chris, et al., does the decision on how to treat sqrt predate our
> > current way of handling errno?
> 
> The intention of llvm.sqrt is that it is the same as sqrt(), but
> doesn't affect errno.  If it helps, I think it would be completely
> reasonable to change this in langref.html:
> 
> "Unlike sqrt in libm, however, llvm.sqrt has undefined behavior for
> ..."
> 
> to "... produces an undefined value", with a link back to
> ##undefined-values.
I'm not sure that helps, because it will prevents sqrt + -fno-math-errno (a
readnone sqrt) -> llvm.sqrt -- and thus still prevents the use of llvm.sqrt
to vectorize sqrt. I think what will help is just saying something like this:

  "Unlike sqrt in libm, after calling llvm.sqrt with a negative argument
(except for -0.0), the state of errno, and any other portions of the
floating-point environment that are used to record errors, is undefined."

FWIW, we may want to make a similar change for the other llvm.<libm
function> intrinsics, at least for the vector versions (as vector ops on many
ISAs don't affect the FP state bits like the scalar ops do).
> 
> Transformations that apply to sqrt() should generally apply to
> llvm.sqrt as well.
And the intent is for this to be true for all of the other llvm.<libm
function> intrinsics as well, right?

Thanks again,
Hal
> 
> -Chris
> 
-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory

----- Original Message -----
> ----- Original Message -----
> > 
> > On Nov 11, 2013, at 9:30 PM, Hal Finkel <hfinkel at anl.gov>
wrote:
> > 
> > > ----- Original Message -----
> > >> Hi Hal, all.
> > >> 
> > >> I'm not sure why llvm.sqrt is 'special'.  Maybe
because there is
> > >> a
> > >> SSE
> > >> packed sqrt instruction (SQRTPS) but not e.g. a packed sin
> > >> instruction
> > >> AFAIK.
> > > 
> > > This seems relevant:
> > >
http://lists.cs.uiuc.edu/pipermail/llvmdev/2007-August/010248.html
> > > 
> > > Chris, et al., does the decision on how to treat sqrt predate our
> > > current way of handling errno?
> > 
> > The intention of llvm.sqrt is that it is the same as sqrt(), but
> > doesn't affect errno.  If it helps, I think it would be completely
> > reasonable to change this in langref.html:
> > 
> > "Unlike sqrt in libm, however, llvm.sqrt has undefined behavior
for
> > ..."
> > 
> > to "... produces an undefined value", with a link back to
> > ##undefined-values.
> 
> I'm not sure that helps, because it will prevents sqrt +
> -fno-math-errno (a readnone sqrt) -> llvm.sqrt -- and thus still
> prevents the use of llvm.sqrt to vectorize sqrt. I think what will
> help is just saying something like this:
> 
>   "Unlike sqrt in libm, after calling llvm.sqrt with a negative
>   argument (except for -0.0), the state of errno, and any other
>   portions of the floating-point environment that are used to record
>   errors, is undefined."
(to use the correct terminology): "used to record errors" ->
"used to record exceptions".

 -Hal
> 
> FWIW, we may want to make a similar change for the other llvm.<libm
> function> intrinsics, at least for the vector versions (as vector
> ops on many ISAs don't affect the FP state bits like the scalar ops
> do).
> 
> > 
> > Transformations that apply to sqrt() should generally apply to
> > llvm.sqrt as well.
> 
> And the intent is for this to be true for all of the other llvm.<libm
> function> intrinsics as well, right?
> 
> Thanks again,
> Hal
> 
> > 
> > -Chris
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> 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 Nov 11, 2013, at 10:44 PM, Hal Finkel <hfinkel at anl.gov>
wrote:
>> "Unlike sqrt in libm, however, llvm.sqrt has undefined behavior
for
>> ..."
>> 
>> to "... produces an undefined value", with a link back to
>> ##undefined-values.
> 
> I'm not sure that helps, because it will prevents sqrt +
-fno-math-errno (a readnone sqrt) -> llvm.sqrt -- and thus still prevents the
use of llvm.sqrt to vectorize sqrt. I think what will help is just saying
something like this:
> 
>  "Unlike sqrt in libm, after calling llvm.sqrt with a negative
argument (except for -0.0), the state of errno, and any other portions of the
floating-point environment that are used to record errors, is undefined.”
Makes sense to me.
> 
> FWIW, we may want to make a similar change for the other llvm.<libm
function> intrinsics, at least for the vector versions (as vector ops on many
ISAs don't affect the FP state bits like the scalar ops do).
Yep.
> 
>> 
>> Transformations that apply to sqrt() should generally apply to
>> llvm.sqrt as well.
> 
> And the intent is for this to be true for all of the other llvm.<libm
function> intrinsics as well, right?
Yep!

-Chris

----- Original Message -----
> On Nov 11, 2013, at 10:44 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> >> "Unlike sqrt in libm, however, llvm.sqrt has undefined
behavior
> >> for
> >> ..."
> >> 
> >> to "... produces an undefined value", with a link back
to
> >> ##undefined-values.
> > 
> > I'm not sure that helps, because it will prevents sqrt +
> > -fno-math-errno (a readnone sqrt) -> llvm.sqrt -- and thus still
> > prevents the use of llvm.sqrt to vectorize sqrt. I think what will
> > help is just saying something like this:
> > 
> >  "Unlike sqrt in libm, after calling llvm.sqrt with a negative
> >  argument (except for -0.0), the state of errno, and any other
> >  portions of the floating-point environment that are used to
> >  record errors, is undefined.”
> 
> Makes sense to me.
> 
> > 
> > FWIW, we may want to make a similar change for the other llvm.<libm
> > function> intrinsics, at least for the vector versions (as vector
> > ops on many ISAs don't affect the FP state bits like the scalar
> > ops do).
> 
> Yep.
Okay, good. I'll construct a documentation patch for review shortly.
> 
> > 
> >> 
> >> Transformations that apply to sqrt() should generally apply to
> >> llvm.sqrt as well.
> > 
> > And the intent is for this to be true for all of the other
> > llvm.<libm function> intrinsics as well, right?
> 
> Yep!
Okay; I'll work on refactoring the existing optimizations to apply to both.

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