[llvm] Revert "LangRef: Clarify llvm.minnum and llvm.maxnum about sNaN and signed zero (#112852)" (PR #138451)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Sun May 4 07:36:40 PDT 2025
nikic wrote:
> The operation is fundamentally broken.
The operation as specified by IEEE-754-2008 is indeed fundamentally broken, which is why we did not use that definition...
> We are not going to make forward progress unless we fix the definition. Signaling nans do not matter in the slightest in the real world. I do not consider any signaling validation failure to be an emergency worth reverting over
This seems like an argument to do the revert and then leave it alone entirely? The whole change is exclusively about the behavior with signaling NaNs -- but unfortunately also affects the general optimization properties of the intrinsic (such as associativity). The impact on existing middle-end optimizations was not properly analyzed when the change was made, let alone actually implemented (in the multiple months since the LangRef patch landed).
As seen in recent PRs and issues, the current state where LangRef and the middle-end do not agree is causing unnecessary confusion, which is why I think it is important to revert it now, before the situation becomes worse. We can still do the change, but it needs to be implemented in a more coordinated way, just like all other changes to IR semantics.
(An alternative I would put up for consideration is to remove maxnum entirely, auto-upgrading the intrinsic to maximumnum nsz to match the previous semantics. Not sure anyone actually wants llvm.maxnum with the new semantics?)
https://github.com/llvm/llvm-project/pull/138451
More information about the llvm-commits
mailing list