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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: scanon.
spatel added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4154
+  if (isa<UndefValue>(Op0) || isa<UndefValue>(Op1))
+    return ConstantFP::getNaN(Op0->getType());
+
----------------
mike.dvoretsky wrote:
> Why are we emitting NaNs from an undef operand here?
See the recent discussion on llvm-dev:
http://lists.llvm.org/pipermail/llvm-dev/2018-March/121481.html


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4157
+  if (match(Op0, m_NaN()) || match(Op1, m_NaN()))
+    return ConstantFP::getNaN(Op0->getType());
+
----------------
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.


https://reviews.llvm.org/D44521