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.

@dtcxzyw dtcxzyw requested a review from arsenm June 9, 2025 06:06
@dtcxzyw dtcxzyw requested a review from nikic as a code owner June 9, 2025 06:06
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Jun 9, 2025
@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
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
…m#143349)

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 llvm#143123.
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
…m#143349)

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 llvm#143123.
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
…m#143349)

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 llvm#143123.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[InstCombine] fcmp is incorrectly inverted
3 participants