[llvm] LangRef: Clarify llvm.minnum and llvm.maxnum about sNaN (PR #112852)
Joshua Cranmer via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 29 12:03:39 PDT 2024
https://github.com/jcranmer-intel requested changes to this pull request.
This PR as it stands has numerous issues that need to be addressed.
At this point, we are now changing the behavior of the intrinsic with respect to signed zero handling, and changing the behavior with respect to sNaN. This means that the commit message is no longer accurate and needs to be adjusted.
The deeper problem is the text. The edge case behavior of these intrinsics is confusing, and it's not helped by the fact that the source materials tend to bury the behavior in the important edge cases so that you need to collate normative text from several different places to even understand what IEEE 754:2008's rules for minNum or C23's rules for fmin are. Because of that, the semantics really need to be upfront and clear about what the edge-case behavior is, and the history of changes so far have served to bury those details rather than illuminate them.
An example opening paragraph that would work better is this:
> Follows the IEEE-754 semantics for minNum, except that -0.0 < +0.0 for the purposes of this intrinsic. As for signaling NaNs, per the IEEE-754 semantics, if either operand is an sNaN, the result is always a qNaN. This matches the *recommended* behavior for the libm function `fmin`, although not all implementations have implemented these recommended behaviors.
After that opening paragraph, I think it's useful to keep the original second paragraph, except modified slightly to account for the changes to sNaN and -0.0/+0.0 semantics. And only after that does it make sense to start to discuss the interactions with `nsz`, LLVM's general sNaN-quieting rules, and the caution about implementing it properly.
(It also might be worth adding notes to the minnum/maxnum intrinsics that mininum/minimumnum intrinsics should probably be preferred for their non-distinction of sNaN/qNaN handling).
https://github.com/llvm/llvm-project/pull/112852
More information about the llvm-commits
mailing list