[ConstantRange] Expand makeAllowedICmpRegion to use samesign to give tighter range by Adar-Dagan · Pull Request #174355 · llvm/llvm-project

Adar-Dagan · 2026-01-05T04:49:58Z

After the addition of samesign then instcombine correctly transforms unsigned greater than to use it if it can prove that value is greater than zero and then removes the assume that allowed it to prove that. For example:

define i8 @remove_for_samesign(i8 %x) {
  %gt = icmp sgt i8 %x, 10
  call void @llvm.assume(i1 %gt)
  %gt.zero = icmp sge i8 %x, 0
  call void @llvm.assume(i1 %gt.zero)
  ret i8 %x
}

Is optimized to:

define i8 @remove_for_samesign(i8 %x) {
  %gt= icmp samesign ugt i8 [[X:%.*]], 10
  call void @llvm.assume(i1 %gt)
  ret i8 %x
}

This leads to a wrong range being returned from computeConstantRange because it doesn't look at samesign when constructing a range from assumes, this patch fixes that.

llvmbot · 2026-01-05T04:50:28Z

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-ir

Author: Adar Dagan (Adar-Dagan)

Changes

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

2 Files Affected:

(modified) llvm/include/llvm/IR/ConstantRange.h (+2-1)
(modified) llvm/lib/IR/ConstantRange.cpp (+6-1)

diff --git a/llvm/include/llvm/IR/ConstantRange.h b/llvm/include/llvm/IR/ConstantRange.h
index f4d4f1f555fa4..47b9b60b64b2e 100644
--- a/llvm/include/llvm/IR/ConstantRange.h
+++ b/llvm/include/llvm/IR/ConstantRange.h
@@ -41,6 +41,7 @@ namespace llvm {
 
 class MDNode;
 class raw_ostream;
+class CmpPredicate;
 struct KnownBits;
 
 /// This class represents a range of values.
@@ -106,7 +107,7 @@ class [[nodiscard]] ConstantRange {
   ///
   /// Example: Pred = ult and Other = i8 [2, 5) returns Result = [0, 4)
   LLVM_ABI static ConstantRange
-  makeAllowedICmpRegion(CmpInst::Predicate Pred, const ConstantRange &Other);
+  makeAllowedICmpRegion(CmpPredicate Pred, const ConstantRange &Other);
 
   /// Produce the largest range such that all values in the returned range
   /// satisfy the given predicate with all values contained within Other.
diff --git a/llvm/lib/IR/ConstantRange.cpp b/llvm/lib/IR/ConstantRange.cpp
index 9beaee60d0bc1..87d180f19470e 100644
--- a/llvm/lib/IR/ConstantRange.cpp
+++ b/llvm/lib/IR/ConstantRange.cpp
@@ -23,6 +23,7 @@
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/IR/CmpPredicate.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
@@ -106,7 +107,7 @@ std::pair<ConstantRange, ConstantRange> ConstantRange::splitPosNeg() const {
   return {intersectWith(PosFilter), intersectWith(NegFilter)};
 }
 
-ConstantRange ConstantRange::makeAllowedICmpRegion(CmpInst::Predicate Pred,
+ConstantRange ConstantRange::makeAllowedICmpRegion(CmpPredicate Pred,
                                                    const ConstantRange &CR) {
   if (CR.isEmptySet())
     return CR;
@@ -141,6 +142,8 @@ ConstantRange ConstantRange::makeAllowedICmpRegion(CmpInst::Predicate Pred,
     APInt UMin(CR.getUnsignedMin());
     if (UMin.isMaxValue())
       return getEmpty(W);
+    if (Pred.hasSameSign() && CR.isAllNonNegative())
+      return ConstantRange(std::move(UMin) + 1, APInt::getSignedMinValue(W));
     return ConstantRange(std::move(UMin) + 1, APInt::getZero(W));
   }
   case CmpInst::ICMP_SGT: {
@@ -150,6 +153,8 @@ ConstantRange ConstantRange::makeAllowedICmpRegion(CmpInst::Predicate Pred,
     return ConstantRange(std::move(SMin) + 1, APInt::getSignedMinValue(W));
   }
   case CmpInst::ICMP_UGE:
+    if (Pred.hasSameSign() && CR.isAllNonNegative())
+      return getNonEmpty(CR.getUnsignedMin(), APInt::getSignedMinValue(W));
     return getNonEmpty(CR.getUnsignedMin(), APInt::getZero(W));
   case CmpInst::ICMP_SGE:
     return getNonEmpty(CR.getSignedMin(), APInt::getSignedMinValue(W));

Adar-Dagan · 2026-01-11T04:02:09Z

@nikic Hey, I see you were added as a reviewer. Could you review this? or point me to who I should get a review from?

Adar-Dagan · 2026-01-14T08:59:09Z

@nikic ping?

nikic

Missing PR description, motivation and tests?

github-actions · 2026-01-15T06:47:28Z

🪟 Windows x64 Test Results

131364 tests passed
2950 tests skipped

✅ The build succeeded and all tests passed.

dtcxzyw · 2026-01-15T17:16:01Z

llvm/test/Transforms/InstCombine/assume.ll

  ret i1 %q
 }

+define i8 @remove_for_samesign(i8 %x) {


This test doesn't change with your patch: https://godbolt.org/z/84YPE5Kq4

Yes, I meant to show here that instcombine does an optimization that leads to loss of information in computeConstantRange without this patch. I'll remove it and add to the description instead

dtcxzyw · 2026-01-15T17:28:33Z

llvm/include/llvm/IR/ConstantRange.h

  /// Example: Pred = ult and Other = i8 [2, 5) returns Result = [0, 4)
  LLVM_ABI static ConstantRange
-  makeAllowedICmpRegion(CmpInst::Predicate Pred, const ConstantRange &Other);
+  makeAllowedICmpRegion(CmpPredicate Pred, const ConstantRange &Other);


I'd expect this to simplify isImpliedCondCommonOperandWithCR:

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 9456 to 9467 in d5a5678

if (auto Res = CRImpliesPred(ConstantRange::makeAllowedICmpRegion(LPred, LCR),

RPred))

return Res;

if (LPred.hasSameSign() ^ RPred.hasSameSign()) {

LPred = LPred.hasSameSign() ? ICmpInst::getFlippedSignednessPredicate(LPred)

: LPred.dropSameSign();

RPred = RPred.hasSameSign() ? ICmpInst::getFlippedSignednessPredicate(RPred)

: RPred.dropSameSign();

return CRImpliesPred(ConstantRange::makeAllowedICmpRegion(LPred, LCR),

RPred);

}

return std::nullopt;

Unfortunately no, we need to do the same to ConstantRange::icmp to simplify this

dtcxzyw · 2026-01-15T17:31:03Z

llvm/unittests/Analysis/ValueTrackingTest.cpp

    EXPECT_EQ(10, CR2.getUpper());
  }

+  {


Please also add an exhaustive test in ConstantRangeTest.cpp to make sure the result is optimal. I also doubt the correctness of Boolean constant ranges.

Nice catch, there is an edge case for booleans

Added tests to check my changes

For exhaustive tests, I mean you can use EnumerateInterestingConstantRanges to enumerate all possible ranges from i1 to i4. Then you use TestRange to check for correctness and optimality. If the optimality is infeasible, please make sure it is always smaller than the result without samesign.

I printed out the elements I create for each test to validate the correct elements are created, attached file with that print
res.txt

Adar-Dagan · 2026-01-28T09:05:37Z

@dtcxzyw I answered your comments, any followups or something else? or could you approve?

artagnon

Thanks, this is looking pretty good.

llvm/lib/IR/ConstantRange.cpp

llvm/unittests/Analysis/ValueTrackingTest.cpp

llvm/lib/IR/ConstantRange.cpp

github-actions · 2026-02-05T08:38:49Z

🐧 Linux x64 Test Results

191087 tests passed
4968 tests skipped

✅ The build succeeded and all tests passed.

dtcxzyw

LGTM. I guess there is still much room for further simplifying the implementation. But I don't have any ideas for now. Please wait for additional approval from other reviewers.

artagnon

Hm, there's quite a bit of subtle complexity here. I wonder why this has such a small impact on llvm-opt-benchmark?

llvm/lib/IR/ConstantRange.cpp

dtcxzyw · 2026-02-09T13:55:17Z

I wonder why this has such a small impact on llvm-opt-benchmark?

I guess it will become more useful after we let makeExactICmpRegion to leverage samesign as well.

Adar-Dagan · 2026-02-18T06:14:23Z

@artagnon Could you take another look?

artagnon

This LGTM. Pending sign-off from @nikic.

Adar-Dagan · 2026-02-24T05:41:16Z

@nikic ping

antoniofrighetto

I guess there is still much room for further simplifying the implementation.

Second this concern, implementation looks quite bespoke (and sort of getting harder to follow). Just wondering, don't intend to block this, would it be possible to leverage samesign semantics, thus splitting the input range in positive and negative ones, reuse existing SGT/SLT logic, and union the twos? Or would that still lead to a suboptimal range?

antoniofrighetto · 2026-02-25T08:56:01Z

llvm/lib/IR/ConstantRange.cpp

May be worth elaborating on the edge cases?

antoniofrighetto · 2026-02-25T08:56:30Z

llvm/lib/IR/ConstantRange.cpp

Maybe add a comment explaining that can never be satisfied for i1?

antoniofrighetto · 2026-02-25T08:58:10Z

llvm/lib/Analysis/ValueTracking.cpp

I imagine there could be other users in the codebase that may start benefiting from using the whole CmpPredicate, so may be worth postponing this change in a follow-up?

Adar-Dagan · 2026-02-26T08:17:36Z

I guess there is still much room for further simplifying the implementation.

Second this concern, implementation looks quite bespoke (and sort of getting harder to follow). Just wondering, don't intend to block this, would it be possible to leverage samesign semantics, thus splitting the input range in positive and negative ones, reuse existing SGT/SLT logic, and union the twos? Or would that still lead to a suboptimal range?

reusing the signed logic and using intersect works

dtcxzyw · 2026-02-27T02:43:08Z

I guess there is still much room for further simplifying the implementation.

Second this concern, implementation looks quite bespoke (and sort of getting harder to follow). Just wondering, don't intend to block this, would it be possible to leverage samesign semantics, thus splitting the input range in positive and negative ones, reuse existing SGT/SLT logic, and union the twos? Or would that still lead to a suboptimal range?

reusing the signed logic and using intersect works

Looks like this introduces significant compile-time overhead (+0.00349118% -> +0.13571089%) :(
Before: dtcxzyw/llvm-opt-benchmark#3514
After: dtcxzyw/llvm-opt-benchmark#3512

Adar-Dagan requested a review from nikic as a code owner January 5, 2026 04:49

llvmbot added the llvm:ir label Jan 5, 2026

Adar-Dagan force-pushed the MakeConstantRangeUseSameSign branch from 1599826 to 4154699 Compare January 12, 2026 09:46

llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Jan 12, 2026

nikic requested changes Jan 14, 2026

View reviewed changes

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Jan 15, 2026

Adar-Dagan force-pushed the MakeConstantRangeUseSameSign branch 2 times, most recently from 0f57fca to 74e72d5 Compare January 15, 2026 05:55

Adar Dagan added 2 commits January 15, 2026 10:24

init

dca9fb2

Add tests

94734c3

Adar-Dagan force-pushed the MakeConstantRangeUseSameSign branch from 74e72d5 to 94734c3 Compare January 15, 2026 08:24

dtcxzyw changed the title ~~Expand makeAllowedICmpRegion to use samesign to give tighter range~~ [ConstantRange] Expand makeAllowedICmpRegion to use samesign to give tighter range Jan 15, 2026

dtcxzyw reviewed Jan 15, 2026

View reviewed changes

dtcxzyw requested a review from artagnon January 15, 2026 17:31

Fix comments and add tests

a361009

artagnon reviewed Jan 28, 2026

View reviewed changes

llvm/lib/IR/ConstantRange.cpp Outdated Show resolved Hide resolved

llvm/unittests/Analysis/ValueTrackingTest.cpp Show resolved Hide resolved

llvm/lib/IR/ConstantRange.cpp Outdated Show resolved Hide resolved

nikic mentioned this pull request Jan 28, 2026

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

zyw-bot mentioned this pull request Jan 28, 2026

pre-commit: PR174355 dtcxzyw/llvm-opt-benchmark#3398

Closed

Adar Dagan added 2 commits January 28, 2026 16:52

Fix more comments

8f3757d

Use exhaustive test function and improve robustness

1c98459

dtcxzyw reviewed Feb 3, 2026

View reviewed changes

llvm/lib/IR/ConstantRange.cpp Outdated Show resolved Hide resolved

zyw-bot mentioned this pull request Feb 3, 2026

pre-commit: PR174355 dtcxzyw/llvm-opt-benchmark#3417

Closed

dtcxzyw mentioned this pull request Feb 6, 2026

Fuzz PR174355 dtcxzyw/llvm-fuzz-service#175

Closed

dtcxzyw approved these changes Feb 6, 2026

View reviewed changes

Add suggestion

004eb60

Adar-Dagan force-pushed the MakeConstantRangeUseSameSign branch from e94c036 to 004eb60 Compare February 8, 2026 04:13

artagnon reviewed Feb 9, 2026

View reviewed changes

llvm/lib/IR/ConstantRange.cpp Outdated Show resolved Hide resolved

llvm/lib/IR/ConstantRange.cpp Outdated Show resolved Hide resolved

llvm/lib/IR/ConstantRange.cpp Show resolved Hide resolved

Fix comments

210546d

artagnon reviewed Feb 18, 2026

View reviewed changes

antoniofrighetto reviewed Feb 25, 2026

View reviewed changes

Use fallthrough to get signed range

3727dd1

zyw-bot mentioned this pull request Feb 26, 2026

pre-commit: PR174355 dtcxzyw/llvm-opt-benchmark#3512

Closed

	if (auto Res = CRImpliesPred(ConstantRange::makeAllowedICmpRegion(LPred, LCR),
	RPred))
	return Res;
	if (LPred.hasSameSign() ^ RPred.hasSameSign()) {
	LPred = LPred.hasSameSign() ? ICmpInst::getFlippedSignednessPredicate(LPred)
	: LPred.dropSameSign();
	RPred = RPred.hasSameSign() ? ICmpInst::getFlippedSignednessPredicate(RPred)
	: RPred.dropSameSign();
	return CRImpliesPred(ConstantRange::makeAllowedICmpRegion(LPred, LCR),
	RPred);
	}
	return std::nullopt;

Conversation

Adar-Dagan commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Adar-Dagan commented Jan 11, 2026

Uh oh!

Adar-Dagan commented Jan 14, 2026

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows x64 Test Results

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!

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!

Adar-Dagan commented Jan 28, 2026

Uh oh!

artagnon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

artagnon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dtcxzyw commented Feb 9, 2026

Uh oh!

Adar-Dagan commented Feb 18, 2026

Uh oh!

artagnon left a comment

Choose a reason for hiding this comment

Uh oh!

Adar-Dagan commented Feb 24, 2026

Uh oh!

antoniofrighetto 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!

Adar-Dagan commented Feb 26, 2026

Uh oh!

dtcxzyw commented Feb 27, 2026

Uh oh!

Adar-Dagan commented Jan 5, 2026 •

edited

Loading

llvmbot commented Jan 5, 2026 •

edited

Loading

github-actions bot commented Jan 15, 2026 •

edited

Loading

github-actions bot commented Feb 5, 2026 •

edited

Loading