Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[IR] LangRef: state explicitly that floats generally behave according to IEEE-754 #102140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IR] LangRef: state explicitly that floats generally behave according to IEEE-754 #102140
Changes from 3 commits
d643f2c
648d3ce
d107aa0
cd80e84
7d4deb8
29f5c14
f909f35
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the denormal exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the denormal exception? Is this about what happens when
denormal-fp-math
is set, but the default is to be IEEE-compatible?Given that IEEE says that denormals are not flushed and LLVM assumes the same by default, I don't think this is an exception from "IR float ops behave according to IEEE".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by "floating-point instruction" here? Is sqrt included?
I understand that the main point here is to say that without further IR constructs an instruction like fdiv is assumed to be correctly rounded. IEEE-754 also assumes this of sqrt. I believe the latest version specifies that other math functions should also return correctly rounded results. That's why I think it needs to be explicit here which ones you mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant all the operations that have an equivalent IEEE-754 operation. So yes that would include sqrt, though I was under the impression that it does not include transcendental functions.
I am not sure what is the best way to say that. Having a list seems awkward? Should each such operation have a comment, like "This corresponds to <op> in IEEE-754, so if the argument is an IEEE float format then the :ref:
floating-point semantics <floatsem>
guarantees apply."?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something that is hard to come up with a good term for. IEEE 754 has a core list of operations in section 5 which is a good starting point, but these omit the minimum/maximum operations (which are section 9.6). Section 9 is "recommended operations", and 9.2 is the main list of transcendental functions you're thinking of; IEEE 754 requires that they be correctly rounded, but C explicitly disclaims that requirement in Annex F. There's also a few functions in C that aren't in IEEE 754, notably ldexp and frexp.
(Note too that it was recently brought up in the Discourse forums that the libm intrinsics are meant to correspond to libm semantics, not IEEE 754 semantics.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minimum/maximum don't do any rounding, and already seem to unambiguously describe their semantics in the existing docs, making this clarification much less relevant. So maybe we should just say that this is about the core operations listed in section 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to specify "all machines that support IEEE-754 arithmetic"? I don't know if we support any targets that don't support IEEE-754, but it seems like there should be some provision for that. The C standard, for instance, talks about some transformations that are legal on "IEC 60559 machines."
Or are we saying that architectures that don't support IEEE-754 should indicate the differences in the IR or use a different type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, LLVM assumes that all backends implement IEEE-754 arithmetic, and will miscompile code if the backend doesn't do that. One example of a target that does not implement IEEE-754 arithmetic is x86 without SSE, and #89885 has examples of code that gets micompiled due to that.
The point of this PR is to make that more explicit. If instead the goal is to make LLVM work with backends and targets that do not implement IEEE-754 arithmetic, that will require changes to optimization passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're already at the point where we expect
float
et al to correspond to the IEEE 754 binary32 et al formats. (This is documented, although somewhat subtly, by the current LangRef). There is also agreement at this point that excess precision (à la x87) is not correct behavior for LLVM IR, although it's not (yet) explicitly documented in the LangRef.The only hardware deviation from IEEE 754 that we're prepared to accept at this point is denormal handling. I'm reluctant to offer too many guarantees on denormal handling because I'm not up to speed on the diversity of common FP hardware with respect to denormals, but I'm pretty sure there is hardware in use that mandates denormal flushing (e.g., the AVX512-BF16 stuff is unconditionally default RM+DAZ+FTZ, with changing MXCSR having no effect).
In short, we already require that hardware supporting LLVM be IEEE 754-ish; this is tightening up the definition in the LangRef to cover what we already agree to be the case. In the putative future that we start talking about cases where
float
et al are truly non-IEEE 754 types (say, Alpha machines, or perhaps posits will make it big), then we can talk about how to add support for them in LLVM IR (which, given the history of LLVM, probably means "add new types", not "float means something different depending on target triple").There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even there, the pass that causes trouble in #89885 would lead to miscompilations.
Analysis/ScalarEvolution
will assume that float ops that don't return NaNs produce a given bit pattern (including denormals), and if codegen later generates code that produces a different bit pattern, the result is a miscompilation. If we don't accept "always return the same bit-identical result on all machines", then this pass (and possibly others) has to be changed.So non-standard denormal handling is only supported with an explicit marker, which works very similar to the markers required for non-default FP exception handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add "This also means that backends are not allowed to implement floating-point instructions using larger floating-point types unless they take care to consistently narrow the results back to the original range without inducing double-rounding." or some similar text that makes it clear that mapping
fadd float
via just an x87FADD
instruction is not legal lowering.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO that is covered by "backends cannot change the precision of these operations". If we start listing all the consequences of that statement, we'll never be done...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is basically an exact duplicate of the second paragraph in the
floatenv
section, so I am inclined to remove it... but your draft did include such a sentence.The way I view it, the
floatsem
section is just about the IEEE float formats. This paragraph is true for all formats so it should be in thefloatenv
section.