SelectionDAG: ExpandFMINNUM_FMAXNUM: FCANONICALIZE is not needed for FMINNUM_IEEE

[wzssyqa]

In fact we define FMINNUM_IEEE with the same behavioir as FMINNUM.

Fixes: #138303

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

Author: YunQiang Su (wzssyqa)

Changes

In fact we define FMINNUM_IEEE with the same behavioir as FMINNUM.

Fixes: #138303

---

Full diff: https://github.com/llvm/llvm-project/pull/139007.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+3-17)

diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index ba34c72156228..321ae1d508b6c 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -8511,23 +8511,9 @@ SDValue TargetLowering::expandFMINNUM_FMAXNUM(SDNode *Node,
       Node->getOpcode() == ISD::FMINNUM ? ISD::FMINNUM_IEEE : ISD::FMAXNUM_IEEE;
 
   if (isOperationLegalOrCustom(NewOp, VT)) {
-    SDValue Quiet0 = Node->getOperand(0);
-    SDValue Quiet1 = Node->getOperand(1);
-
-    if (!Node->getFlags().hasNoNaNs()) {
-      // Insert canonicalizes if it's possible we need to quiet to get correct
-      // sNaN behavior.
-      if (!DAG.isKnownNeverSNaN(Quiet0)) {
-        Quiet0 = DAG.getNode(ISD::FCANONICALIZE, dl, VT, Quiet0,
-                             Node->getFlags());
-      }
-      if (!DAG.isKnownNeverSNaN(Quiet1)) {
-        Quiet1 = DAG.getNode(ISD::FCANONICALIZE, dl, VT, Quiet1,
-                             Node->getFlags());
-      }
-    }
-
-    return DAG.getNode(NewOp, dl, VT, Quiet0, Quiet1, Node->getFlags());
+    SDValue N0 = Node->getOperand(0);
+    SDValue N1 = Node->getOperand(1);
+    return DAG.getNode(NewOp, dl, VT, N0, N1, Node->getFlags());
   }
 
   // If the target has FMINIMUM/FMAXIMUM but not FMINNUM/FMAXNUM use that

[nikic]

Fixes: #138303

This change doesn't have any relation to this issue at all.

[arsenm]

This isn't the right sequencing. The IEEE version only exists because of the old behavior. The way to do this is to convert each target to making fminnum/fmaxnum directly legal and then deleting this expansion

[wzssyqa]

This isn't the right sequencing. The IEEE version only exists because of the old behavior. The way to do this is to convert each target to making fminnum/fmaxnum directly legal and then deleting this expansion

Yes. I am working on it
#139009
#139010
#135739

I noticed that AMDGPU also has defined FMAXNUM_IEEE. Can you take care about it since I have feel knowlege about the ISA of AMDGPU.

[wzssyqa]

I will improve expandFMINNUM_FMAXNUM if we are ready to do so.