[PATCH] D44521: [InstSimplify] fp_binop X, NaN --> NaN

[Steve Canon via Phabricator via llvm-commits]

scanon added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4157
+  if (match(Op0, m_NaN()) || match(Op1, m_NaN()))
+    return ConstantFP::getNaN(Op0->getType());
+
----------------
spatel wrote:
> mike.dvoretsky wrote:
> > According to IEEE 754, we should preserve the payload bits of the NaN operand here (either one if both are NaN). This code creates a NaN with all payload bits unset instead.
> As I understand it, preserving the payload is not required (cc @scanon ) - but we can do that if preferred. And maybe that will preserve the AMDGPU test behavior.
Preserving the payload is not required. It is *recommended* that the payload of the result be the payload of one of the operands, and if we can do that without additional complexity, let's do it. There is no defined rule for choosing which one, but ideally it should be commutative and associative.

Actual hardware implementations off the top of my head, in cases where both are NaN, either return the signaling NaN's payload if one is signaling, or return the "first" operand's payload ("first" in some machine-code assembly operand order), or return a default NaN (arm with DN bit set in FPCR, for example). Other behaviors are possible.


https://reviews.llvm.org/D44521