Page MenuHomePhabricator

[LangRef] make it clear that FP instructions do not have side effects
ClosedPublic

Authored by spatel on Mar 7 2018, 9:39 AM.

Details

Summary

There may still be some ambiguity about the output with undef and special constants, but I think we have consensus that the LLVM FP opcodes operate in an environment with no trapping, so let's make that explicit and fix the bogus example.

The existing text for the constrained intrinsics says:
"By default, LLVM optimization passes assume that the rounding mode is round-to-nearest and that floating point exceptions will not be monitored. Constrained FP intrinsics are used to support non-default rounding modes and accurately preserve exception behavior without compromising LLVM’s ability to optimize FP code when the default behavior is used."

Diff Detail

Repository
rL LLVM

Event Timeline

spatel created this revision.Mar 7 2018, 9:39 AM
spatel edited the summary of this revision. (Show Details)Mar 7 2018, 9:42 AM

Thanks for taking the initiative on this!

docs/LangRef.rst
3047 ↗(On Diff #137414)

I don't like the wording, "There is no possibility of trapping...." Strictly speaking, if the user code has unmasked FP exceptions (in spite of not telling us via FENV_ACCESS that they intended to) it is possible that these will trap. More to the point is that the optimizer assumes no trapping.

6400 ↗(On Diff #137414)

Again, I'd suggest "can not trap or have any other side effects" -> "is assumed not to trap or have any other side effects"

spatel updated this revision to Diff 137428.Mar 7 2018, 10:28 AM
spatel marked 2 inline comments as done.

Patch updated:
Improve/soften the wording - we make assumptions about the state of FP affairs.

efriedma added inline comments.Mar 7 2018, 11:37 AM
docs/LangRef.rst
3041 ↗(On Diff #137428)

I would rather just rewrite this example so it doesn't use fdiv.

You can make the point this paragraph is trying to make much more clearly and unambiguously using integer sdiv rather than floating-point operations.

spatel added inline comments.Mar 7 2018, 11:49 AM
docs/LangRef.rst
3041 ↗(On Diff #137428)

Yes, good point. Do you think there's value in also having the FP examples here to explicitly show the difference between int and FP, or are the additions to the opcode definitions enough?

efriedma added inline comments.Mar 7 2018, 11:59 AM
docs/LangRef.rst
3041 ↗(On Diff #137428)

The additions to the opcode definitions are probably good enough.

6497 ↗(On Diff #137428)

You might also want to mention we assume the default rounding mode, or maybe just the default floating-point environment. And maybe specifically call out that we assume the user won't read the floating-point exception state.

spatel updated this revision to Diff 137484.Mar 7 2018, 2:37 PM

Patch updated:

  1. Change the undef vs. UB example to use sdiv rather than fdiv.
  2. Add more details about the assumed FP execution env and behavior of the FP instructions.
lattner accepted this revision.Mar 8 2018, 6:21 PM

These changes look great, but I'd suggest explicitly mentioning that SNaN behavior somewhere, the relation of the instructions to the intrinsics etc. Perhaps a section before Fast Math Flags?

This revision is now accepted and ready to land.Mar 8 2018, 6:21 PM
spatel added a comment.Mar 9 2018, 7:17 AM

These changes look great, but I'd suggest explicitly mentioning that SNaN behavior somewhere, the relation of the instructions to the intrinsics etc. Perhaps a section before Fast Math Flags?

Sure, I'll follow-up. I'm intentionally making minimal patches because that makes it less likely that I'll veer into the unknown. Also, any help with the phraseology is appreciated; I've only played a lawyer on TV. :)

This revision was automatically updated to reflect the committed changes.