Skip to content

[ValueTracking] Update Ordered when both operands are non-NaN. #143349

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

Merged
merged 5 commits into from
Jun 9, 2025

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Jun 9, 2025

When the original predicate is ordered and both operands are non-NaN, Ordered should be set to true. This variable still matters even if both operands are non-NaN because FMF only applies to select, not fcmp.

Closes #143123.

@llvmbot
Copy link
Member

llvmbot commented Jun 9, 2025

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

When the original predicate is ordered and both operands are non-NaN, Ordered should be set to true. If the original one is unordered, it is also safe to be converted into an ordered predicate. This variable still matters even if both operands are non-NaN because FMF only applies to select, not fcmp.

Closes #143123.


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

3 Files Affected:

  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+2)
  • (modified) llvm/test/Transforms/InstCombine/fcmp-select.ll (+16-3)
  • (modified) llvm/unittests/Analysis/ValueTrackingTest.cpp (+1-1)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index d6bb852e208f9..e65e34cda95ac 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -8660,6 +8660,8 @@ static SelectPatternResult matchSelectPattern(CmpInst::Predicate Pred,
     if (LHSSafe && RHSSafe) {
       // Both operands are known non-NaN.
       NaNBehavior = SPNB_RETURNS_ANY;
+      // It is always safe to convert the comparison to an ordered one.
+      Ordered = true;
     } else if (CmpInst::isOrdered(Pred)) {
       // An ordered comparison will return false when given a NaN, so it
       // returns the RHS.
diff --git a/llvm/test/Transforms/InstCombine/fcmp-select.ll b/llvm/test/Transforms/InstCombine/fcmp-select.ll
index 053b233cb5f04..9434f6da0b9b3 100644
--- a/llvm/test/Transforms/InstCombine/fcmp-select.ll
+++ b/llvm/test/Transforms/InstCombine/fcmp-select.ll
@@ -270,12 +270,25 @@ define i1 @test_fcmp_select_var_const_unordered(double %x, double %y) {
 }
 
 define i1 @test_fcmp_ord_select_fcmp_oeq_var_const(double %x) {
-; CHECK-LABEL:    @test_fcmp_ord_select_fcmp_oeq_var_const(
-; CHECK-NEXT:     [[CMP1:%.*]] = fcmp oeq double [[X:%.*]], 1.000000e+00
-; CHECK-NEXT:     ret i1 [[CMP1]]
+; CHECK-LABEL: @test_fcmp_ord_select_fcmp_oeq_var_const(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp oeq double [[X:%.*]], 1.000000e+00
+; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %cmp1 = fcmp ord double %x, 0.000000e+00
   %sel = select i1 %cmp1, double %x, double 0.000000e+00
   %cmp2 = fcmp oeq double %sel, 1.000000e+00
   ret i1 %cmp2
 }
+
+; Make sure that we recognize the SPF correctly.
+
+define float @test_select_nnan_nsz_fcmp_olt(float %x) {
+; CHECK-LABEL: @test_select_nnan_nsz_fcmp_olt(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp olt float [[X:%.*]], -0.000000e+00
+; CHECK-NEXT:    [[SEL1:%.*]] = select i1 [[TMP1]], float [[X]], float -0.000000e+00
+; CHECK-NEXT:    ret float [[SEL1]]
+;
+  %cmp = fcmp olt float %x, 0.000000e+00
+  %sel = select nnan nsz i1 %cmp, float %x, float -0.000000e+00
+  ret float %sel
+}
diff --git a/llvm/unittests/Analysis/ValueTrackingTest.cpp b/llvm/unittests/Analysis/ValueTrackingTest.cpp
index e23005b60891d..faf72bb17880b 100644
--- a/llvm/unittests/Analysis/ValueTrackingTest.cpp
+++ b/llvm/unittests/Analysis/ValueTrackingTest.cpp
@@ -187,7 +187,7 @@ TEST_F(MatchSelectPatternTest, FastFMin) {
       "  %A = select i1 %1, float %a, float 5.0\n"
       "  ret float %A\n"
       "}\n");
-  expectPattern({SPF_FMINNUM, SPNB_RETURNS_ANY, false});
+  expectPattern({SPF_FMINNUM, SPNB_RETURNS_ANY, true});
 }
 
 TEST_F(MatchSelectPatternTest, FMinConstantZero) {

@dtcxzyw dtcxzyw marked this pull request as draft June 9, 2025 06:14
@dtcxzyw dtcxzyw changed the title [ValueTracking] Set Ordered to true when both operands are non-NaN. [ValueTracking] Update Ordered when both operands are non-NaN. Jun 9, 2025
@dtcxzyw dtcxzyw marked this pull request as ready for review June 9, 2025 06:30
@dtcxzyw dtcxzyw merged commit 2f15637 into llvm:main Jun 9, 2025
9 checks passed
@dtcxzyw dtcxzyw deleted the fix-143123 branch June 9, 2025 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[InstCombine] fcmp is incorrectly inverted
3 participants