Skip to content

expandFMINIMUMNUM_FMAXIMUMNUM: Quiet is not needed for NaN vs NaN #139237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wzssyqa
Copy link
Contributor

@wzssyqa wzssyqa commented May 9, 2025

New LangRef doesn't requires quieting for NaN vs NaN, aka the result may be sNaN for sNaN vs NaN.
See: #139228

@wzssyqa wzssyqa requested a review from arsenm May 9, 2025 10:00
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label May 9, 2025
@llvmbot
Copy link
Member

llvmbot commented May 9, 2025

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: YunQiang Su (wzssyqa)

Changes

New LangRef doesn't requires quieting for NaN vs NaN, aka the result may be sNaN for sNaN vs NaN.
See: #139228


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (-5)
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index ba34c72156228..2c63c54fc03f7 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -8683,11 +8683,6 @@ SDValue TargetLowering::expandFMINIMUMNUM_FMAXIMUMNUM(SDNode *Node,
 
   SDValue MinMax =
       DAG.getSelectCC(DL, LHS, RHS, LHS, RHS, IsMax ? ISD::SETGT : ISD::SETLT);
-  // If MinMax is NaN, let's quiet it.
-  if (!Flags.hasNoNaNs() && !DAG.isKnownNeverNaN(LHS) &&
-      !DAG.isKnownNeverNaN(RHS)) {
-    MinMax = DAG.getNode(ISD::FCANONICALIZE, DL, VT, MinMax, Flags);
-  }
 
   // Fixup signed zero behavior.
   if (Options.NoSignedZerosFPMath || Flags.hasNoSignedZeros() ||

@arsenm arsenm added the floating-point Floating-point math label May 13, 2025
@arsenm
Copy link
Contributor

arsenm commented May 13, 2025

Missing test changes

wzssyqa added 2 commits May 14, 2025 10:23
New LangRef doesn't requires quieting for NaN vs NaN, aka
the result may be sNaN for sNaN vs NaN.
See: llvm#139228
@wzssyqa wzssyqa force-pushed the fix-phasing-minimumnum-maximumnum branch from 06f867c to 8003083 Compare May 14, 2025 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants