llvm dev - Jan 2014 - [LLVMdev] Removing ReadOnly from math intrinsics

On Fri, Jan 24, 2014 at 4:17 PM, Philip Reames <listmail at
philipreames.com>wrote:
> On 1/24/14 3:52 PM, Raul Silvera wrote:
>
>  In include/llvm/IR/Intrinsics.td there is code to mark sqrt and several
> other math intrinsics as "ReadOnly", even though they do not read
memory.
>
>  According to the comments this was done as an attempt to model changes
> to the FP rounding mode. This is too conservative, and unnecessarily blocks
> transformations such as commoning and vectorization.
>
>  I have heard from others that FP environment changes are not well
> modeled on LLVM anyway, so perhaps it is appropriate to just change these
> from ReadOnly to ReadNone. Any opinions on this? If there are no objections
> I'll prepare a patch.
>
> While our current modeling isn't quite right (e.g. we don't model
writes
> to errno and related state), I'm very reluctant to see us move in the
> direction you propose.  I'm leary of having intrinsics which are
modeled
> incorrectly and relying on the optimizers not to exploit that fact and
> yield incorrect code.  This seems like a recipe for disaster long term.

errno is a totally separate issue. I started to confuse these when talking
with Raul, and sorry for that.

The intrinsics should (IMO) be modeled as *not* fiddling with errno at all.
That is already the case today, and we don't transform library calls into
intrinsic calls unless compiling under '-fno-math-errno' to explicitly
say
this is allowed.

The interesting question is whether the floating point intrinsics
"read"
the hardware rounding mode. I would very much like to say "no" because
LLVM
has essentially no support for modeling hardware rounding modes in any
other context. For example, we don't model it for multiply and divide, so
modeling it for sqrt seems pointless and in impedes a huge number of
optimizations.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140124/175e37f5/attachment.html>

Right. Like Chandler is saying, these intrinsics are only used under
fno-math-errno and are already properly modeled as not modifying errno.

The change being proposed only affects the dynamic change of hardware fp
rounding modes, which is not currently supported by llvm (#pragma STDC
FENV_ACCESS ON). We currently appear to be at the worst of both worlds as
we do not have support for this environment but are always being
constrained by it.

On Fri, Jan 24, 2014 at 4:17 PM, Philip Reames <listmail at
philipreames.com>wrote:
> On 1/24/14 3:52 PM, Raul Silvera wrote:
>
>  In include/llvm/IR/Intrinsics.td there is code to mark sqrt and several
> other math intrinsics as "ReadOnly", even though they do not read
memory.
>
>  According to the comments this was done as an attempt to model changes
> to the FP rounding mode. This is too conservative, and unnecessarily blocks
> transformations such as commoning and vectorization.
>
>  I have heard from others that FP environment changes are not well
> modeled on LLVM anyway, so perhaps it is appropriate to just change these
> from ReadOnly to ReadNone. Any opinions on this? If there are no objections
> I'll prepare a patch.
>
> While our current modeling isn't quite right (e.g. we don't model
writes
> to errno and related state), I'm very reluctant to see us move in the
> direction you propose.  I'm leary of having intrinsics which are
modeled
> incorrectly and relying on the optimizers not to exploit that fact and
> yield incorrect code.  This seems like a recipe for disaster long term.

errno is a totally separate issue. I started to confuse these when talking
with Raul, and sorry for that.

The intrinsics should (IMO) be modeled as *not* fiddling with errno at all.
That is already the case today, and we don't transform library calls into
intrinsic calls unless compiling under '-fno-math-errno' to explicitly
say
this is allowed.

The interesting question is whether the floating point intrinsics
"read"
the hardware rounding mode. I would very much like to say "no" because
LLVM
has essentially no support for modeling hardware rounding modes in any
other context. For example, we don't model it for multiply and divide, so
modeling it for sqrt seems pointless and in impedes a huge number of
optimizations.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140124/39da6a22/attachment.html>

I would be perfectly fine with Raul & Chandler's proposal, provide that 
clear documentation is added.  The strong distinction between standard 
library calls and intrinsics is an important point for front end 
authors.  The deliberate ignorance of floating point environment flags 
is entirely defensible, but needs to be documented clearly.  We should 
also document which rounding mode (and related bits of FP env state) 
that our optimizations assume.

Do we currently model flag writes for floating point?  If so, does this 
effect the discussion?

Philip

On 1/24/14 5:19 PM, Raul Silvera wrote:
>
> Right. Like Chandler is saying, these intrinsics are only used under 
> fno-math-errno and are already properly modeled as not modifying errno.
>
> The change being proposed only affects the dynamic change of hardware 
> fp rounding modes, which is not currently supported by llvm (#pragma 
> STDC FENV_ACCESS ON). We currently appear to be at the worst of both 
> worlds as we do not have support for this environment but are always 
> being constrained by it.
>
>
> On Fri, Jan 24, 2014 at 4:17 PM, Philip Reames 
> <listmail at philipreames.com <mailto:listmail at
philipreames.com>> wrote:
>
>     On 1/24/14 3:52 PM, Raul Silvera wrote:
>>     In include/llvm/IR/Intrinsics.td there is code to mark sqrt and
>>     several other math intrinsics as "ReadOnly", even though
they do
>>     not read memory.
>>
>>     According to the comments this was done as an attempt to model
>>     changes to the FP rounding mode. This is too conservative, and
>>     unnecessarily blocks transformations such as commoning and
>>     vectorization.
>>
>>     I have heard from others that FP environment changes are not well
>>     modeled on LLVM anyway, so perhaps it is appropriate to just
>>     change these from ReadOnly to ReadNone. Any opinions on this? If
>>     there are no objections I'll prepare a patch.
>     While our current modeling isn't quite right (e.g. we don't
model
>     writes to errno and related state), I'm very reluctant to see us
>     move in the direction you propose. I'm leary of having intrinsics
>     which are modeled incorrectly and relying on the optimizers not to
>     exploit that fact and yield incorrect code.  This seems like a
>     recipe for disaster long term.
>
>
> errno is a totally separate issue. I started to confuse these when 
> talking with Raul, and sorry for that.
>
> The intrinsics should (IMO) be modeled as *not* fiddling with errno at 
> all. That is already the case today, and we don't transform library 
> calls into intrinsic calls unless compiling under '-fno-math-errno'
to
> explicitly say this is allowed.
>
> The interesting question is whether the floating point intrinsics 
> "read" the hardware rounding mode. I would very much like to say
"no"
> because LLVM has essentially no support for modeling hardware rounding 
> modes in any other context. For example, we don't model it for 
> multiply and divide, so modeling it for sqrt seems pointless and in 
> impedes a huge number of optimizations.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140127/69ba9e72/attachment.html>