Skip to content

Conversation

@wzssyqa
Copy link
Contributor

@wzssyqa wzssyqa commented Apr 25, 2025

ISD::FMAXNUM and ISD::FMINNUM treat +0.0>-0.0 now, so let's set MinMaxMustRespectOrderedZero for it.

@wzssyqa wzssyqa marked this pull request as draft April 25, 2025 17:19
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Apr 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 25, 2025

@llvm/pr-subscribers-llvm-selectiondag

Author: YunQiang Su (wzssyqa)

Changes

ISD::FMAXNUM and ISD::FMINNUM treat +0.0>-0.0 now, so let's set MinMaxMustRespectOrderedZero for it.


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+1-2)
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 6930b54ddb14a..7baed2d591514 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -8573,8 +8573,6 @@ SDValue TargetLowering::expandFMINIMUM_FMAXIMUM(SDNode *N,
   unsigned CompOpcIeee = IsMax ? ISD::FMAXNUM_IEEE : ISD::FMINNUM_IEEE;
   unsigned CompOpc = IsMax ? ISD::FMAXNUM : ISD::FMINNUM;
 
-  // FIXME: We should probably define fminnum/fmaxnum variants with correct
-  // signed zero behavior.
   bool MinMaxMustRespectOrderedZero = false;
 
   if (isOperationLegalOrCustom(CompOpcIeee, VT)) {
@@ -8582,6 +8580,7 @@ SDValue TargetLowering::expandFMINIMUM_FMAXIMUM(SDNode *N,
     MinMaxMustRespectOrderedZero = true;
   } else if (isOperationLegalOrCustom(CompOpc, VT)) {
     MinMax = DAG.getNode(CompOpc, DL, VT, LHS, RHS, Flags);
+    MinMaxMustRespectOrderedZero = true;
   } else {
     if (VT.isVector() && !isOperationLegalOrCustom(ISD::VSELECT, VT))
       return DAG.UnrollVectorOp(N);

wzssyqa and others added 2 commits April 27, 2025 08:52
ISD::FMAXNUM and ISD::FMINNUM treat +0.0>-0.0 now,
so let's set MinMaxMustRespectOrderedZero for it.
@wzssyqa wzssyqa force-pushed the expandFMINIMUM_NO_IEEE branch from cce0512 to c153bb8 Compare April 27, 2025 00:52
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Dec 2, 2025

See:#168838

@wzssyqa wzssyqa closed this Dec 2, 2025
@arsenm arsenm reopened this Dec 2, 2025
@arsenm
Copy link
Contributor

arsenm commented Dec 2, 2025

See:#168838

This was reverted, and the contentious part is signaling nan handling. One of the reasons simply reverting this is wrong is the signed zero behavior should remain regardless of that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants