Skip to content

[InstCombine] Generalize foldAndOrOfICmpsUsingRanges to handle more cases. #158498

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.

Sign up for GitHub

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 4 commits into from
Sep 18, 2025
Merged

[InstCombine] Generalize foldAndOrOfICmpsUsingRanges to handle more cases. #158498

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9630974
[InstCombine] Add pre-commit tests. NFC.
dtcxzyw Sep 14, 2025
c9a4bc8
[InstCombine] Generalize `foldAndOrOfICmpsUsingRanges` to handle more…
dtcxzyw Sep 14, 2025
a8517e0
[InstCombine] Address review comments. NFC.
dtcxzyw Sep 16, 2025
d616102
[InstCombine] Address review comments. NFC.
dtcxzyw Sep 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 38 additions & 63 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1320,74 +1320,49 @@ static Value *foldAndOrOfICmpsWithConstEq(ICmpInst *Cmp0, ICmpInst *Cmp1,
Value *InstCombinerImpl::foldAndOrOfICmpsUsingRanges(ICmpInst *ICmp1,
ICmpInst *ICmp2,
bool IsAnd) {
CmpPredicate Pred1, Pred2;
Value *V1, *V2;
const APInt *C1, *C2;
if (!match(ICmp1, m_ICmp(Pred1, m_Value(V1), m_APInt(C1))) ||
!match(ICmp2, m_ICmp(Pred2, m_Value(V2), m_APInt(C2))))
return nullptr;

// Look through add of a constant offset on V1, V2, or both operands. This
// allows us to interpret the V + C' < C'' range idiom into a proper range.
const APInt *Offset1 = nullptr, *Offset2 = nullptr;
if (V1 != V2) {
Value *X;
if (match(V1, m_Add(m_Value(X), m_APInt(Offset1))))
V1 = X;
if (match(V2, m_Add(m_Value(X), m_APInt(Offset2))))
V2 = X;
}

// Look through and with a negative power of 2 mask on V1 or V2. This
// detects idioms of the form `(x == A) || ((x & Mask) == A + 1)` where A + 1
// is aligned to the mask and A + 1 >= |Mask|. This pattern corresponds to a
// contiguous range check, which can be folded into an addition and compare.
// The same applies for `(x != A) && ((x & Mask) != A + 1)`.
auto AreContiguousRangePredicates = [](CmpPredicate Pred1, CmpPredicate Pred2,
bool IsAnd) {
if (IsAnd)
return Pred1 == ICmpInst::ICMP_NE && Pred2 == ICmpInst::ICMP_NE;
return Pred1 == ICmpInst::ICMP_EQ && Pred2 == ICmpInst::ICMP_EQ;
};
const APInt *Mask1 = nullptr, *Mask2 = nullptr;
bool MatchedAnd1 = false, MatchedAnd2 = false;
if (V1 != V2 && AreContiguousRangePredicates(Pred1, Pred2, IsAnd)) {
auto MatchRangeCheck =
Copy link
Contributor

Choose a reason for hiding this comment

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

Could add a comment for the lambda function? How about s/MatchRangeCheck/MaybeSatisfyingRangeForPred/ instead (or something along these lines)?

Copy link
Member Author

Choose a reason for hiding this comment

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

MaybeSatisfyingRangeForPred

The range is exact (ignoring the poison-generating flags). I think "MaybeSatisfying" is more confusing than the original one...

[](ICmpInst *ICmp) -> std::optional<std::pair<Value *, ConstantRange>> {
const APInt *C;
if (!match(ICmp->getOperand(1), m_APInt(C)))
return std::nullopt;
Value *LHS = ICmp->getOperand(0);
CmpPredicate Pred = ICmp->getPredicate();
Value *X;
if (match(V1, m_OneUse(m_And(m_Value(X), m_NegatedPower2(Mask1)))) &&
C1->getBitWidth() == C2->getBitWidth() && *C1 == *C2 + 1 &&
C1->uge(Mask1->abs()) && C1->isPowerOf2()) {
MatchedAnd1 = true;
V1 = X;
}
if (match(V2, m_OneUse(m_And(m_Value(X), m_NegatedPower2(Mask2)))) &&
C1->getBitWidth() == C2->getBitWidth() && *C2 == *C1 + 1 &&
C2->uge(Mask2->abs()) && C2->isPowerOf2()) {
MatchedAnd2 = true;
V2 = X;
// Match (x & NegPow2) ==/!= C
const APInt *Mask;
if (ICmpInst::isEquality(Pred) &&
match(LHS, m_OneUse(m_And(m_Value(X), m_NegatedPower2(Mask)))) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the one use check here? there is non for the add below

Copy link
Member Author

Choose a reason for hiding this comment

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

It was added in #153842. Note that this fold may insert two instructions for the new range check. I think the one-use check here is needed. We can investigate this (and m_AddLike below) in the future. But for now, I want to keep the diff small.

C->countr_zero() >= Mask->countr_zero()) {
ConstantRange CR{*C, *C - *Mask};
if (Pred == ICmpInst::ICMP_NE)
CR = CR.inverse();
return std::make_pair(X, CR);
}
}
ConstantRange CR = ConstantRange::makeExactICmpRegion(Pred, *C);
// Match (add X, C1) pred C
const APInt *C1;
if (match(LHS, m_AddLike(m_Value(X), m_APInt(C1))))
return std::make_pair(X, CR.subtract(*C1));
return std::make_pair(LHS, CR);
};

auto RC1 = MatchRangeCheck(ICmp1);
if (!RC1)
return nullptr;

if (V1 != V2)
auto RC2 = MatchRangeCheck(ICmp2);
if (!RC2)
return nullptr;

ConstantRange CR1 =
MatchedAnd1
? ConstantRange(*C1, *C1 - *Mask1)
: ConstantRange::makeExactICmpRegion(
IsAnd ? ICmpInst::getInverseCmpPredicate(Pred1) : Pred1, *C1);
if (Offset1)
CR1 = CR1.subtract(*Offset1);

ConstantRange CR2 =
MatchedAnd2
? ConstantRange(*C2, *C2 - *Mask2)
: ConstantRange::makeExactICmpRegion(
IsAnd ? ICmpInst::getInverseCmpPredicate(Pred2) : Pred2, *C2);
if (Offset2)
CR2 = CR2.subtract(*Offset2);

Type *Ty = V1->getType();
Value *NewV = V1;
if (RC1->first != RC2->first)
return nullptr;

Value *V = RC1->first;
auto CR1 = IsAnd ? RC1->second.inverse() : RC1->second;
auto CR2 = IsAnd ? RC2->second.inverse() : RC2->second;

Type *Ty = V->getType();
Value *NewV = V;
std::optional<ConstantRange> CR = CR1.exactUnionWith(CR2);
if (!CR) {
if (!(ICmp1->hasOneUse() && ICmp2->hasOneUse()) || CR1.isWrappedSet() ||
Expand Down
49 changes: 42 additions & 7 deletions llvm/test/Transforms/InstCombine/and-or-icmps.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3672,17 +3672,52 @@ define i1 @neg_or_icmp_eq_double_and_pow2(i32 %x) {
ret i1 %ret
}

define i1 @neg_select_icmp_eq_and_pow2(i32 %x) {
; CHECK-LABEL: @neg_select_icmp_eq_and_pow2(
; CHECK-NEXT: [[ICMP1:%.*]] = icmp sgt i32 [[X:%.*]], 127
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X]], -32
; CHECK-NEXT: [[ICMP2:%.*]] = icmp eq i32 [[AND]], 128
; CHECK-NEXT: [[TMP1:%.*]] = and i1 [[ICMP1]], [[ICMP2]]
; CHECK-NEXT: ret i1 [[TMP1]]
define i1 @implied_select_icmp_eq_and_pow2(i32 %x) {
; CHECK-LABEL: @implied_select_icmp_eq_and_pow2(
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[X:%.*]], -32
; CHECK-NEXT: [[TMP2:%.*]] = icmp eq i32 [[TMP1]], 128
; CHECK-NEXT: ret i1 [[TMP2]]
;
%icmp1 = icmp sgt i32 %x, 127
%and = and i32 %x, -32
%icmp2 = icmp eq i32 %and, 128
%1 = select i1 %icmp1, i1 %icmp2, i1 false
ret i1 %1
}

define i1 @implied_range_check(i8 %a) {
; CHECK-LABEL: @implied_range_check(
; CHECK-NEXT: [[MASKED:%.*]] = and i8 [[A:%.*]], -2
; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i8 [[MASKED]], 2
; CHECK-NEXT: ret i1 [[CMP2]]
;
%cmp1 = icmp ult i8 %a, 5
%masked = and i8 %a, -2
%cmp2 = icmp eq i8 %masked, 2
%and = and i1 %cmp1, %cmp2
ret i1 %and
}

define i1 @merge_range_check_and(i8 %a) {
; CHECK-LABEL: @merge_range_check_and(
; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i8 [[MASKED:%.*]], 2
; CHECK-NEXT: ret i1 [[CMP2]]
;
%cmp1 = icmp ult i8 %a, 3
%masked = and i8 %a, -2
%cmp2 = icmp eq i8 %masked, 2
%and = and i1 %cmp1, %cmp2
ret i1 %and
}

define i1 @merge_range_check_or(i8 %a) {
; CHECK-LABEL: @merge_range_check_or(
; CHECK-NEXT: [[AND:%.*]] = icmp ult i8 [[A:%.*]], 4
; CHECK-NEXT: ret i1 [[AND]]
;
%cmp1 = icmp ult i8 %a, 3
%masked = and i8 %a, -2
%cmp2 = icmp eq i8 %masked, 2
%and = or i1 %cmp1, %cmp2
ret i1 %and
}
5 changes: 1 addition & 4 deletions llvm/test/Transforms/InstCombine/icmp-range.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1678,10 +1678,7 @@ define i1 @icmp_slt_sext_ne_otherwise_nofold(i32 %a) {
; tests from PR59555
define i1 @isFloat(i64 %0) {
; CHECK-LABEL: @isFloat(
; CHECK-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP0:%.*]], 281474976710655
; CHECK-NEXT: [[TMP3:%.*]] = and i64 [[TMP0]], -281474976710656
; CHECK-NEXT: [[TMP4:%.*]] = icmp ne i64 [[TMP3]], 281474976710656
; CHECK-NEXT: [[TMP5:%.*]] = and i1 [[TMP2]], [[TMP4]]
; CHECK-NEXT: [[TMP5:%.*]] = icmp ugt i64 [[TMP0:%.*]], 562949953421311
; CHECK-NEXT: ret i1 [[TMP5]]
;
%2 = icmp ugt i64 %0, 281474976710655
Expand Down
Loading