[llvm] LangRef: Clarify llvm.minnum and llvm.maxnum about sNaN (PR #112852)

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 20 08:16:03 PDT 2024


arsenm wrote:

> I'm not convinced this change is a good idea, as opposed to fixing the backends that implement it differently.

I think we definitely need to do this. Taking the name of the IEEE operations and not implementing the same behavior is not an acceptable place to be. We will simplify the code, and save instructions by doing this. We now have minimumnum/maximumnum to provide the ignore-signalingness behavior.


> LLVM has a general principle that sNaNs are treated more or less equivalent to qNaN, and this is the main case where sNaN inputs do very different things from qNaN inputs. 

Yes, the IEEE behavior absurdly inverts the behavior for signaling nans so we do not have the luxury of simply ignoring them like in every other operation.

> The original text is pretty explicit that targets are supposed to use `llvm.canonicalize` to handle sNaN quieting if they're mapping to hardware that does distinguish between sNaN/qNaN, and I don't see any justification in this PR yet for why doing that is problematic.

This was me working around this absolute nonsense situation. The *only* reason it is this way is because of the OpenCL conformance test checked for signaling nans treated as quiet nans, and gnu libm's fmin/fmax had the ignore signaling behavior at the time. libm has been fixed, and OpenCL has a footnote on fmin/fmax that signaling nans "may" behave as quiet nans. 

The codegen for these was never consistent across targets. We never considered the host library versions for the buggy libm, and until recently I believe AMDGPU was the only target bothering with the canonicalize + ieee variant lowering. We have been wasting cycles on this for years, for no real reason. Signaling nans are not used in the real world. We should be able to just directly select this to the instruction that implements the IEEE 2008 behavior.



https://github.com/llvm/llvm-project/pull/112852


More information about the llvm-commits mailing list