[PATCH] D88572: AMDGPU/SelectionDAG Check for NaN, DX10Clamp and IEEE in fmed3 combine

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

I think it's a futile to expect correct behavior with IEEE=0. No other operation behaves correctly with respect to snan quieting, and we're not going to implement all of the necessary manual quieting (i.e. isKnownNeverSNaN is always wrong since the operations that quiet don't really)



================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9765
+  if (K1->isExactlyValue(1.0) && K0->isExactlyValue(0.0)) {
+    // If we're told NaNs won't happen on inner node make clamp and keep flags.
+    if (Op0->getFlags().hasNoNaNs() || DAG.getTarget().Options.NoNaNsFPMath)
----------------
Missing the


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9771-9772
+    // min(QNaN, 1.0) = 1.0 and we can't use clamp.
+    bool XKnownNeverSNaN = DAG.isKnownNeverSNaN(Var);
+    if (DAG.isKnownNeverNaN(Var) ||
+        (Info->getMode().DX10Clamp &&
----------------
Should not need to do both of these checks


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9774
+        (Info->getMode().DX10Clamp &&
+         (XKnownNeverSNaN || (!XKnownNeverSNaN && !Info->getMode().IEEE))))
+      return DAG.getNode(AMDGPUISD::CLAMP, SL, VT, Var);
----------------
Should short circuit expensive check based on IEEE mode


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88572/new/

https://reviews.llvm.org/D88572