[SCEV] Better preserve wrapping info in SimplifyICmpOperands for UGE. #144404

fhahn · 2025-06-16T17:55:53Z

Update SimplifyICmpOperands to only try subtracting 1 from RHS first, if RHS is an op we can fold the subtract directly into. Otherwise try adding to LHS first, as we can preserve NUW flags.

This improves results in a few cases, including the modified test case from berkeley-abc and new code to be added in
#128061.

Note that there are more cases where the results can be improved by better ordering here which I'll try to investigate as follow-up.

llvmbot · 2025-06-16T17:56:27Z

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Update SimplifyICmpOperands to only try subtracting 1 from RHS first, if RHS is an op we can fold the subtract directly into. Otherwise try adding to LHS first, as we can preserve NUW flags.

This improves results in a few cases, including the modified test case from berkeley-abc and new code to be added in
#128061.

Note that there are more cases where the results can be improved by better ordering here which I'll try to investigate as follow-up.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ScalarEvolution.cpp (+9-1)
(modified) llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll (+3-6)

diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 2dfe625eb0dcc..ea7c7ece5632f 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -10892,7 +10892,11 @@ bool ScalarEvolution::SimplifyICmpOperands(CmpPredicate &Pred, const SCEV *&LHS,
     }
     break;
   case ICmpInst::ICMP_UGE:
-    if (!getUnsignedRangeMin(RHS).isMinValue()) {
+    // If RHS is an op we can fold the -1, try that first.
+    if ((isa<SCEVConstant>(RHS) ||
+         match(RHS,
+               m_scev_AffineAddRec(m_SCEVConstant(), m_SCEV(), m_Loop()))) &&
+        !getUnsignedRangeMin(RHS).isMinValue()) {
       RHS = getAddExpr(getConstant(RHS->getType(), (uint64_t)-1, true), RHS);
       Pred = ICmpInst::ICMP_UGT;
       Changed = true;
@@ -10901,6 +10905,10 @@ bool ScalarEvolution::SimplifyICmpOperands(CmpPredicate &Pred, const SCEV *&LHS,
                        SCEV::FlagNUW);
       Pred = ICmpInst::ICMP_UGT;
       Changed = true;
+    } else if (!getUnsignedRangeMin(RHS).isMinValue()) {
+      RHS = getAddExpr(getConstant(RHS->getType(), (uint64_t)-1, true), RHS);
+      Pred = ICmpInst::ICMP_UGT;
+      Changed = true;
     }
     break;
   default:
diff --git a/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll b/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll
index b0dbbd5eaedf4..fb2fdb116f904 100644
--- a/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll
+++ b/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll
@@ -53,15 +53,12 @@ loop.latch:
 
 define void @test_simplifycompare_rhs_not_constant1() {
 ; CHECK-LABEL: define void @test_simplifycompare_rhs_not_constant1() {
-; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:  [[ENTRY:.*:]]
 ; CHECK-NEXT:    [[P:%.*]] = alloca i64, align 8
 ; CHECK-NEXT:    br label %[[LOOP:.*]]
 ; CHECK:       [[LOOP]]:
-; CHECK-NEXT:    [[PTR_IV:%.*]] = phi ptr [ [[P]], %[[ENTRY]] ], [ [[PTR_IV_NEXT:%.*]], %[[LOOP]] ]
-; CHECK-NEXT:    [[PTR_IV_NEXT]] = getelementptr i8, ptr [[PTR_IV]], i64 -8
-; CHECK-NEXT:    call void @use(ptr [[PTR_IV]])
-; CHECK-NEXT:    [[EC:%.*]] = icmp ult ptr [[PTR_IV_NEXT]], [[P]]
-; CHECK-NEXT:    br i1 [[EC]], label %[[EXIT:.*]], label %[[LOOP]]
+; CHECK-NEXT:    call void @use(ptr [[P]])
+; CHECK-NEXT:    br i1 true, label %[[EXIT:.*]], label %[[LOOP]]
 ; CHECK:       [[EXIT]]:
 ; CHECK-NEXT:    ret void
 ;

llvmbot · 2025-06-16T17:56:27Z

@llvm/pr-subscribers-llvm-analysis

Author: Florian Hahn (fhahn)

Changes

Update SimplifyICmpOperands to only try subtracting 1 from RHS first, if RHS is an op we can fold the subtract directly into. Otherwise try adding to LHS first, as we can preserve NUW flags.

This improves results in a few cases, including the modified test case from berkeley-abc and new code to be added in
#128061.

Note that there are more cases where the results can be improved by better ordering here which I'll try to investigate as follow-up.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ScalarEvolution.cpp (+9-1)
(modified) llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll (+3-6)

diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 2dfe625eb0dcc..ea7c7ece5632f 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -10892,7 +10892,11 @@ bool ScalarEvolution::SimplifyICmpOperands(CmpPredicate &Pred, const SCEV *&LHS,
     }
     break;
   case ICmpInst::ICMP_UGE:
-    if (!getUnsignedRangeMin(RHS).isMinValue()) {
+    // If RHS is an op we can fold the -1, try that first.
+    if ((isa<SCEVConstant>(RHS) ||
+         match(RHS,
+               m_scev_AffineAddRec(m_SCEVConstant(), m_SCEV(), m_Loop()))) &&
+        !getUnsignedRangeMin(RHS).isMinValue()) {
       RHS = getAddExpr(getConstant(RHS->getType(), (uint64_t)-1, true), RHS);
       Pred = ICmpInst::ICMP_UGT;
       Changed = true;
@@ -10901,6 +10905,10 @@ bool ScalarEvolution::SimplifyICmpOperands(CmpPredicate &Pred, const SCEV *&LHS,
                        SCEV::FlagNUW);
       Pred = ICmpInst::ICMP_UGT;
       Changed = true;
+    } else if (!getUnsignedRangeMin(RHS).isMinValue()) {
+      RHS = getAddExpr(getConstant(RHS->getType(), (uint64_t)-1, true), RHS);
+      Pred = ICmpInst::ICMP_UGT;
+      Changed = true;
     }
     break;
   default:
diff --git a/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll b/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll
index b0dbbd5eaedf4..fb2fdb116f904 100644
--- a/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll
+++ b/llvm/test/Transforms/IndVarSimplify/simplify-icmp-operands-order.ll
@@ -53,15 +53,12 @@ loop.latch:
 
 define void @test_simplifycompare_rhs_not_constant1() {
 ; CHECK-LABEL: define void @test_simplifycompare_rhs_not_constant1() {
-; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:  [[ENTRY:.*:]]
 ; CHECK-NEXT:    [[P:%.*]] = alloca i64, align 8
 ; CHECK-NEXT:    br label %[[LOOP:.*]]
 ; CHECK:       [[LOOP]]:
-; CHECK-NEXT:    [[PTR_IV:%.*]] = phi ptr [ [[P]], %[[ENTRY]] ], [ [[PTR_IV_NEXT:%.*]], %[[LOOP]] ]
-; CHECK-NEXT:    [[PTR_IV_NEXT]] = getelementptr i8, ptr [[PTR_IV]], i64 -8
-; CHECK-NEXT:    call void @use(ptr [[PTR_IV]])
-; CHECK-NEXT:    [[EC:%.*]] = icmp ult ptr [[PTR_IV_NEXT]], [[P]]
-; CHECK-NEXT:    br i1 [[EC]], label %[[EXIT:.*]], label %[[LOOP]]
+; CHECK-NEXT:    call void @use(ptr [[P]])
+; CHECK-NEXT:    br i1 true, label %[[EXIT:.*]], label %[[LOOP]]
 ; CHECK:       [[EXIT]]:
 ; CHECK-NEXT:    ret void
 ;

nikic · 2025-06-16T20:07:20Z

llvm/lib/Analysis/ScalarEvolution.cpp

+    } else if (!getUnsignedRangeMin(RHS).isMinValue()) {
+      RHS = getAddExpr(getConstant(RHS->getType(), (uint64_t)-1, true), RHS);
+      Pred = ICmpInst::ICMP_UGT;
+      Changed = true;


Does unconditionally swapping the cases cause regressions?

Yep, we would loose flags in test_simplifycompare_rhs_constant and test_simplifycompare_rhs_addrec, but gain some in test_simplifycompare_rhs_not_constant2

The current version doesn't have any regression on a large test-set of C/C++ workloads.

llvm-opt-benchmark results are available here: https://github.com/dtcxzyw/llvm-opt-benchmark/pull/2452/files

nikic

LGTM

nikic · 2025-06-17T07:55:28Z

llvm/lib/Analysis/ScalarEvolution.cpp

+    // If RHS is an op we can fold the -1, try that first.
+    if ((isa<SCEVConstant>(RHS) ||
+         match(RHS,
+               m_scev_AffineAddRec(m_SCEVConstant(), m_SCEV(), m_Loop()))) &&


This doesn't need an affine addrec, and we could also handle add with constant first operand.

Added to check for SCEVAddExpr/SCEVAddRecExpr and check their first operand. Didn't cause any changes on my test set.

nikic · 2025-06-17T07:57:55Z

llvm/lib/Analysis/ScalarEvolution.cpp

@@ -10892,7 +10892,11 @@ bool ScalarEvolution::SimplifyICmpOperands(CmpPredicate &Pred, const SCEV *&LHS,
    }
    break;
  case ICmpInst::ICMP_UGE:
-    if (!getUnsignedRangeMin(RHS).isMinValue()) {
+    // If RHS is an op we can fold the -1, try that first.


Suggested change

// If RHS is an op we can fold the -1, try that first.

// If RHS is an op we can fold the -1, try that first.

// Otherwise prefer LHS to preserve the nuw flag.

Added thanks

nikic · 2025-06-17T07:59:10Z

llvm/lib/Analysis/ScalarEvolution.cpp

@@ -10901,6 +10905,10 @@ bool ScalarEvolution::SimplifyICmpOperands(CmpPredicate &Pred, const SCEV *&LHS,
                       SCEV::FlagNUW);
      Pred = ICmpInst::ICMP_UGT;
      Changed = true;
+    } else if (!getUnsignedRangeMin(RHS).isMinValue()) {


I really don't like the code structure here, but I don't have a good suggestion on improving it either...

Same here, but I also didn't find anything better :(

@nikic Not sure if this could be an acceptable alternative: https://gist.github.com/antoniofrighetto/83c188d5ad0a4988eec8d4c30684dafe. It started relatively well, it didn't really end up as good as originally envisioned (NFCI, as RHS is checked first for UGE, might not want this as we probably should preserve the flag).

Update SimplifyICmpOperands to only try subtracting 1 from RHS first, if RHS is an op we can fold the subtract directly into. Otherwise try adding to LHS first, as we can preserve NUW flags. This improves results in a few cases, including the modified test case from berkeley-abc and new code to be added in llvm#128061. Note that there are more cases where the results can be improved by better ordering here which I'll try to investigate as follow-up.

llvm-ci · 2025-06-17T14:32:48Z

LLVM Buildbot has detected a new failure on builder clang-hip-vega20 running on hip-vega20-0 while building llvm at step 2 "update-annotated-scripts".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/21504

Here is the relevant piece of the build log for the reference

Step 2 (update-annotated-scripts) failure: update (failure)
git version 2.34.1
fatal: unable to access 'https://github.com/llvm/llvm-zorg.git/': Could not resolve host: github.com
fatal: unable to access 'https://github.com/llvm/llvm-zorg.git/': Could not resolve host: github.com

…ds for UGE. (#144404) Update SimplifyICmpOperands to only try subtracting 1 from RHS first, if RHS is an op we can fold the subtract directly into. Otherwise try adding to LHS first, as we can preserve NUW flags. This improves results in a few cases, including the modified test case from berkeley-abc and new code to be added in llvm/llvm-project#128061. Note that there are more cases where the results can be improved by better ordering here which I'll try to investigate as follow-up. PR: llvm/llvm-project#144404

…llvm#144404) Update SimplifyICmpOperands to only try subtracting 1 from RHS first, if RHS is an op we can fold the subtract directly into. Otherwise try adding to LHS first, as we can preserve NUW flags. This improves results in a few cases, including the modified test case from berkeley-abc and new code to be added in llvm#128061. Note that there are more cases where the results can be improved by better ordering here which I'll try to investigate as follow-up. PR: llvm#144404

fhahn requested review from preames and efriedma-quic June 16, 2025 17:55

fhahn requested a review from nikic as a code owner June 16, 2025 17:55

llvmbot added llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Jun 16, 2025

nikic reviewed Jun 16, 2025

View reviewed changes

fhahn mentioned this pull request Jun 16, 2025

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

dtcxzyw mentioned this pull request Jun 16, 2025

pre-commit: PR144404 dtcxzyw/llvm-opt-benchmark#2452

Closed

nikic approved these changes Jun 17, 2025

View reviewed changes

fhahn added 2 commits June 17, 2025 13:03

!fixup address comments, thanks

3baa310

fhahn force-pushed the scev-simplifyicmp-operands branch from cfcc8f3 to 3baa310 Compare June 17, 2025 13:19

fhahn merged commit 8f79754 into llvm:main Jun 17, 2025
6 of 7 checks passed

fhahn deleted the scev-simplifyicmp-operands branch June 17, 2025 14:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SCEV] Better preserve wrapping info in SimplifyICmpOperands for UGE. #144404

[SCEV] Better preserve wrapping info in SimplifyICmpOperands for UGE. #144404

Uh oh!

fhahn commented Jun 16, 2025

Uh oh!

llvmbot commented Jun 16, 2025

Uh oh!

llvmbot commented Jun 16, 2025

Uh oh!

nikic Jun 16, 2025

Uh oh!

fhahn Jun 16, 2025

Uh oh!

fhahn Jun 16, 2025

Uh oh!

nikic left a comment

Uh oh!

nikic Jun 17, 2025

Uh oh!

fhahn Jun 17, 2025

Uh oh!

nikic Jun 17, 2025

Uh oh!

fhahn Jun 17, 2025

Uh oh!

nikic Jun 17, 2025

Uh oh!

fhahn Jun 17, 2025

Uh oh!

antoniofrighetto Jun 17, 2025 •

edited

Loading

Uh oh!

Uh oh!

llvm-ci commented Jun 17, 2025

Uh oh!

Uh oh!

	// If RHS is an op we can fold the -1, try that first.
	// If RHS is an op we can fold the -1, try that first.
	// Otherwise prefer LHS to preserve the nuw flag.

[SCEV] Better preserve wrapping info in SimplifyICmpOperands for UGE. #144404

[SCEV] Better preserve wrapping info in SimplifyICmpOperands for UGE. #144404

Uh oh!

Conversation

fhahn commented Jun 16, 2025

Uh oh!

llvmbot commented Jun 16, 2025

Uh oh!

llvmbot commented Jun 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antoniofrighetto Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Jun 17, 2025

Uh oh!

Uh oh!

antoniofrighetto Jun 17, 2025 •

edited

Loading