Skip to content

Conversation

@wzssyqa
Copy link
Contributor

@wzssyqa wzssyqa commented Sep 11, 2024

If we are sure that it is not sNaN, even it may be qNaN, we can use it directly.

@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Sep 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 11, 2024

@llvm/pr-subscribers-llvm-selectiondag

Author: YunQiang Su (wzssyqa)

Changes

If we are sure that it is not sNaN, even it may be qNaN, we can use it directly.


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+2-2)
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 03010c1df00145..042661af1c8f96 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -8614,8 +8614,8 @@ 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)) {
+  if (!Flags.hasNoNaNs() && !DAG.isKnownNeverSNaN(LHS) &&
+      !DAG.isKnownNeverSNaN(RHS)) {
     MinMax = DAG.getNode(ISD::FCANONICALIZE, DL, VT, MinMax, Flags);
   }
 

@RKSimon RKSimon requested a review from arsenm September 11, 2024 10:02
@arsenm arsenm added the floating-point Floating-point math label Sep 11, 2024
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs test changes

@wzssyqa wzssyqa force-pushed the fminimumnum_fcanon_snan branch from 7672436 to 31dcdac Compare September 11, 2024 13:17
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Sep 11, 2024

Needs test changes

I only see that we have nnan option while there is no similiar one for sNaN.
https://llvm.org/docs/LangRef.html

How to mark a variable to never SNaN while may qNaN?

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Sep 12, 2024

I guess t hat I am confused by nofpclass in https://llvm.org/docs/LangRef.html#id1932.
Can it used to mark an argument that it will never be a type?

I have a try, while it won't as DAG.isKnownNeverNaN won't return true in this case.
Maybe I misunderstand its meaning.

@wzssyqa wzssyqa marked this pull request as draft September 12, 2024 09:14
@wzssyqa wzssyqa marked this pull request as draft September 12, 2024 09:14
@wzssyqa wzssyqa marked this pull request as draft September 12, 2024 09:14
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Sep 12, 2024

Needs test changes
Once #108350 is merged, I will fix SelectionDAG::isKnownNeverNaN with sNaN, and add test case for it.

@arsenm
Copy link
Contributor

arsenm commented Sep 15, 2024

I guess t hat I am confused by nofpclass in https://llvm.org/docs/LangRef.html#id1932. Can it used to mark an argument that it will never be a type?

I have a try, while it won't as DAG.isKnownNeverNaN won't return true in this case. Maybe I misunderstand its meaning.

isKnownNeverNaN is quite underdeveloped. You can most easily test the no-nan operand case by just having a filler fadd nnan instruction. It would be simpler to do that than pulling in a dependency on some more general nofpclass handling

wzssyqa and others added 2 commits March 6, 2025 18:24
SelectionDAGISel::LowerArguments: Pass NoNaN Flags to InVals.

`nofpclass` support values nan, snan, qnan, where nan=snan|qnan.
So let's use NoSNaNs and NoQNaNs in SDNodeFlags.

Thus, we can use it in isKnownNeverNaN.
…nly for sNaN

If we are sure that it is not sNaN, even it may be qNaN, we can use
it directly.
@wzssyqa wzssyqa force-pushed the fminimumnum_fcanon_snan branch from 44833e8 to d65a025 Compare March 6, 2025 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

floating-point Floating-point math llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants