Skip to content

Commit 060de41

Browse files
committed
Reapply [InstCombine] Simplify and/or of icmp eq with op replacement (#70335)
Relative to the first attempt, this contains two changes: First, we only handle the case where one side simplifies to true or false, instead of calling simplification recursively. The previous approach would return poison if one operand simplified to poison (under the equality assumption), which is incorrect. Second, we do not fold llvm.is.constant in simplifyWithOpReplaced(). We may be assuming that a value is constant, if the equality holds, but it may not actually be constant. This is nominally just a QoI issue, but the std::list implementation in libstdc++ relies on the precise behavior in a way that causes miscompiles. ----- and/or in logical (select) form benefit from generic simplifications via simplifyWithOpReplaced(). However, the corresponding fold for plain and/or currently does not exist. Similar to selects, there are two general cases for this fold (illustrated with `and`, but there are `or` conjugates). The basic case is something like `(a == b) & c`, where the replacement of a with b or b with a inside c allows it to fold to true or false. Then the whole operation will fold to either false or `a == b`. The second case is something like `(a != b) & c`, where the replacement inside c allows it to fold to false. In that case, the operand can be replaced with c, because in the case where a == b (and thus the icmp is false), c itself will already be false. As the test diffs show, this catches quite a lot of patterns in existing test coverage. This also obsoletes quite a few existing special-case and/or of icmp folds we have (e.g. simplifyAndOrOfICmpsWithLimitConst), but I haven't removed anything as part of this patch in the interest of risk mitigation. Fixes #69050. Fixes #69091.
1 parent 05d52a4 commit 060de41

14 files changed

+251
-587
lines changed

llvm/lib/Analysis/InstructionSimplify.cpp

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,6 +2026,58 @@ static Value *simplifyAndOrOfCmps(const SimplifyQuery &Q, Value *Op0,
20262026
return nullptr;
20272027
}
20282028

2029+
static Value *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
2030+
const SimplifyQuery &Q,
2031+
bool AllowRefinement,
2032+
SmallVectorImpl<Instruction *> *DropFlags,
2033+
unsigned MaxRecurse);
2034+
2035+
static Value *simplifyAndOrWithICmpEq(unsigned Opcode, Value *Op0, Value *Op1,
2036+
const SimplifyQuery &Q,
2037+
unsigned MaxRecurse) {
2038+
assert((Opcode == Instruction::And || Opcode == Instruction::Or) &&
2039+
"Must be and/or");
2040+
ICmpInst::Predicate Pred;
2041+
Value *A, *B;
2042+
if (!match(Op0, m_ICmp(Pred, m_Value(A), m_Value(B))) ||
2043+
!ICmpInst::isEquality(Pred))
2044+
return nullptr;
2045+
2046+
auto Simplify = [&](Value *Res) -> Value * {
2047+
Constant *Absorber = ConstantExpr::getBinOpAbsorber(Opcode, Res->getType());
2048+
2049+
// and (icmp eq a, b), x implies (a==b) inside x.
2050+
// or (icmp ne a, b), x implies (a==b) inside x.
2051+
// If x simplifies to true/false, we can simplify the and/or.
2052+
if (Pred ==
2053+
(Opcode == Instruction::And ? ICmpInst::ICMP_EQ : ICmpInst::ICMP_NE)) {
2054+
if (Res == Absorber)
2055+
return Absorber;
2056+
if (Res == ConstantExpr::getBinOpIdentity(Opcode, Res->getType()))
2057+
return Op0;
2058+
return nullptr;
2059+
}
2060+
2061+
// If we have and (icmp ne a, b), x and for a==b we can simplify x to false,
2062+
// then we can drop the icmp, as x will already be false in the case where
2063+
// the icmp is false. Similar for or and true.
2064+
if (Res == Absorber)
2065+
return Op1;
2066+
return nullptr;
2067+
};
2068+
2069+
if (Value *Res =
2070+
simplifyWithOpReplaced(Op1, A, B, Q, /* AllowRefinement */ true,
2071+
/* DropFlags */ nullptr, MaxRecurse))
2072+
return Simplify(Res);
2073+
if (Value *Res =
2074+
simplifyWithOpReplaced(Op1, B, A, Q, /* AllowRefinement */ true,
2075+
/* DropFlags */ nullptr, MaxRecurse))
2076+
return Simplify(Res);
2077+
2078+
return nullptr;
2079+
}
2080+
20292081
/// Given a bitwise logic op, check if the operands are add/sub with a common
20302082
/// source value and inverted constant (identity: C - X -> ~(X + ~C)).
20312083
static Value *simplifyLogicOfAddSub(Value *Op0, Value *Op1,
@@ -2160,6 +2212,13 @@ static Value *simplifyAndInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
21602212
isKnownToBeAPowerOfTwo(Op0, Q.DL, /*OrZero*/ true, 0, Q.AC, Q.CxtI, Q.DT))
21612213
return Constant::getNullValue(Op0->getType());
21622214

2215+
if (Value *V =
2216+
simplifyAndOrWithICmpEq(Instruction::And, Op0, Op1, Q, MaxRecurse))
2217+
return V;
2218+
if (Value *V =
2219+
simplifyAndOrWithICmpEq(Instruction::And, Op1, Op0, Q, MaxRecurse))
2220+
return V;
2221+
21632222
if (Value *V = simplifyAndOrOfCmps(Q, Op0, Op1, true))
21642223
return V;
21652224

@@ -2436,6 +2495,13 @@ static Value *simplifyOrInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
24362495
match(Op0, m_LShr(m_Specific(X), m_Specific(Y))))
24372496
return Op1;
24382497

2498+
if (Value *V =
2499+
simplifyAndOrWithICmpEq(Instruction::Or, Op0, Op1, Q, MaxRecurse))
2500+
return V;
2501+
if (Value *V =
2502+
simplifyAndOrWithICmpEq(Instruction::Or, Op1, Op0, Q, MaxRecurse))
2503+
return V;
2504+
24392505
if (Value *V = simplifyAndOrOfCmps(Q, Op0, Op1, false))
24402506
return V;
24412507

@@ -4337,6 +4403,10 @@ static Value *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
43374403
return nullptr;
43384404
}
43394405

4406+
// Don't fold away llvm.is.constant checks based on assumptions.
4407+
if (match(I, m_Intrinsic<Intrinsic::is_constant>()))
4408+
return nullptr;
4409+
43404410
// Replace Op with RepOp in instruction operands.
43414411
SmallVector<Value *, 8> NewOps;
43424412
bool AnyReplaced = false;

llvm/test/CodeGen/PowerPC/pr45448.ll

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,16 @@ define hidden void @julia_tryparse_internal_45896() #0 {
2020
; CHECK-NEXT: .LBB0_6: # %fail194
2121
; CHECK-NEXT: .LBB0_7: # %L670
2222
; CHECK-NEXT: li r5, -3
23-
; CHECK-NEXT: cmpdi r3, 0
2423
; CHECK-NEXT: sradi r4, r3, 63
2524
; CHECK-NEXT: rldic r5, r5, 4, 32
26-
; CHECK-NEXT: crnot 4*cr5+lt, eq
2725
; CHECK-NEXT: mulhdu r3, r3, r5
2826
; CHECK-NEXT: maddld r6, r4, r5, r3
2927
; CHECK-NEXT: cmpld cr1, r6, r3
3028
; CHECK-NEXT: mulhdu. r3, r4, r5
31-
; CHECK-NEXT: bc 4, 4*cr5+lt, .LBB0_10
32-
; CHECK-NEXT: # %bb.8: # %L670
3329
; CHECK-NEXT: crorc 4*cr5+lt, 4*cr1+lt, eq
34-
; CHECK-NEXT: bc 4, 4*cr5+lt, .LBB0_10
35-
; CHECK-NEXT: # %bb.9: # %L917
36-
; CHECK-NEXT: .LBB0_10: # %L994
30+
; CHECK-NEXT: bc 4, 4*cr5+lt, .LBB0_9
31+
; CHECK-NEXT: # %bb.8: # %L917
32+
; CHECK-NEXT: .LBB0_9: # %L994
3733
top:
3834
%0 = load i64, ptr undef, align 8
3935
%1 = icmp ne i64 %0, 0

llvm/test/Transforms/InstCombine/div-by-0-guard-before-smul_ov.ll

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,7 @@ define i1 @n2_wrong_size(i4 %size0, i4 %size1, i4 %nmemb) {
4747

4848
define i1 @n3_wrong_pred(i4 %size, i4 %nmemb) {
4949
; CHECK-LABEL: @n3_wrong_pred(
50-
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i4 [[SIZE:%.*]], 0
51-
; CHECK-NEXT: [[SMUL:%.*]] = tail call { i4, i1 } @llvm.smul.with.overflow.i4(i4 [[SIZE]], i4 [[NMEMB:%.*]])
52-
; CHECK-NEXT: [[SMUL_OV:%.*]] = extractvalue { i4, i1 } [[SMUL]], 1
53-
; CHECK-NEXT: [[AND:%.*]] = and i1 [[SMUL_OV]], [[CMP]]
54-
; CHECK-NEXT: ret i1 [[AND]]
50+
; CHECK-NEXT: ret i1 false
5551
;
5652
%cmp = icmp eq i4 %size, 0 ; not 'ne'
5753
%smul = tail call { i4, i1 } @llvm.smul.with.overflow.i4(i4 %size, i4 %nmemb)
@@ -63,10 +59,7 @@ define i1 @n3_wrong_pred(i4 %size, i4 %nmemb) {
6359
define i1 @n4_not_and(i4 %size, i4 %nmemb) {
6460
; CHECK-LABEL: @n4_not_and(
6561
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i4 [[SIZE:%.*]], 0
66-
; CHECK-NEXT: [[SMUL:%.*]] = tail call { i4, i1 } @llvm.smul.with.overflow.i4(i4 [[SIZE]], i4 [[NMEMB:%.*]])
67-
; CHECK-NEXT: [[SMUL_OV:%.*]] = extractvalue { i4, i1 } [[SMUL]], 1
68-
; CHECK-NEXT: [[AND:%.*]] = or i1 [[SMUL_OV]], [[CMP]]
69-
; CHECK-NEXT: ret i1 [[AND]]
62+
; CHECK-NEXT: ret i1 [[CMP]]
7063
;
7164
%cmp = icmp ne i4 %size, 0
7265
%smul = tail call { i4, i1 } @llvm.smul.with.overflow.i4(i4 %size, i4 %nmemb)

llvm/test/Transforms/InstCombine/div-by-0-guard-before-umul_ov.ll

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,7 @@ define i1 @n2_wrong_size(i4 %size0, i4 %size1, i4 %nmemb) {
4747

4848
define i1 @n3_wrong_pred(i4 %size, i4 %nmemb) {
4949
; CHECK-LABEL: @n3_wrong_pred(
50-
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i4 [[SIZE:%.*]], 0
51-
; CHECK-NEXT: [[UMUL:%.*]] = tail call { i4, i1 } @llvm.umul.with.overflow.i4(i4 [[SIZE]], i4 [[NMEMB:%.*]])
52-
; CHECK-NEXT: [[UMUL_OV:%.*]] = extractvalue { i4, i1 } [[UMUL]], 1
53-
; CHECK-NEXT: [[AND:%.*]] = and i1 [[UMUL_OV]], [[CMP]]
54-
; CHECK-NEXT: ret i1 [[AND]]
50+
; CHECK-NEXT: ret i1 false
5551
;
5652
%cmp = icmp eq i4 %size, 0 ; not 'ne'
5753
%umul = tail call { i4, i1 } @llvm.umul.with.overflow.i4(i4 %size, i4 %nmemb)
@@ -63,10 +59,7 @@ define i1 @n3_wrong_pred(i4 %size, i4 %nmemb) {
6359
define i1 @n4_not_and(i4 %size, i4 %nmemb) {
6460
; CHECK-LABEL: @n4_not_and(
6561
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i4 [[SIZE:%.*]], 0
66-
; CHECK-NEXT: [[UMUL:%.*]] = tail call { i4, i1 } @llvm.umul.with.overflow.i4(i4 [[SIZE]], i4 [[NMEMB:%.*]])
67-
; CHECK-NEXT: [[UMUL_OV:%.*]] = extractvalue { i4, i1 } [[UMUL]], 1
68-
; CHECK-NEXT: [[AND:%.*]] = or i1 [[UMUL_OV]], [[CMP]]
69-
; CHECK-NEXT: ret i1 [[AND]]
62+
; CHECK-NEXT: ret i1 [[CMP]]
7063
;
7164
%cmp = icmp ne i4 %size, 0
7265
%umul = tail call { i4, i1 } @llvm.umul.with.overflow.i4(i4 %size, i4 %nmemb)

llvm/test/Transforms/InstCombine/ispow2.ll

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,7 @@ define i1 @is_pow2_ctpop_wrong_pred1(i32 %x) {
392392
; CHECK-LABEL: @is_pow2_ctpop_wrong_pred1(
393393
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
394394
; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 [[T0]], 2
395-
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
396-
; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
397-
; CHECK-NEXT: ret i1 [[R]]
395+
; CHECK-NEXT: ret i1 [[CMP]]
398396
;
399397
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
400398
%cmp = icmp ugt i32 %t0, 2
@@ -946,9 +944,7 @@ define i1 @is_pow2or0_ctpop_wrong_pred1(i32 %x) {
946944
; CHECK-LABEL: @is_pow2or0_ctpop_wrong_pred1(
947945
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
948946
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[T0]], 1
949-
; CHECK-NEXT: [[ISZERO:%.*]] = icmp eq i32 [[X]], 0
950-
; CHECK-NEXT: [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
951-
; CHECK-NEXT: ret i1 [[R]]
947+
; CHECK-NEXT: ret i1 [[CMP]]
952948
;
953949
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
954950
%cmp = icmp ne i32 %t0, 1
@@ -959,11 +955,7 @@ define i1 @is_pow2or0_ctpop_wrong_pred1(i32 %x) {
959955

960956
define i1 @is_pow2or0_ctpop_wrong_pred2(i32 %x) {
961957
; CHECK-LABEL: @is_pow2or0_ctpop_wrong_pred2(
962-
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
963-
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[T0]], 1
964-
; CHECK-NEXT: [[ISZERO:%.*]] = icmp ne i32 [[X]], 0
965-
; CHECK-NEXT: [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
966-
; CHECK-NEXT: ret i1 [[R]]
958+
; CHECK-NEXT: ret i1 true
967959
;
968960
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
969961
%cmp = icmp ne i32 %t0, 1
@@ -1149,9 +1141,7 @@ define i1 @isnot_pow2nor0_ctpop_wrong_pred1(i32 %x) {
11491141
; CHECK-LABEL: @isnot_pow2nor0_ctpop_wrong_pred1(
11501142
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
11511143
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[T0]], 1
1152-
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
1153-
; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
1154-
; CHECK-NEXT: ret i1 [[R]]
1144+
; CHECK-NEXT: ret i1 [[CMP]]
11551145
;
11561146
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
11571147
%cmp = icmp eq i32 %t0, 1
@@ -1162,11 +1152,7 @@ define i1 @isnot_pow2nor0_ctpop_wrong_pred1(i32 %x) {
11621152

11631153
define i1 @isnot_pow2nor0_ctpop_wrong_pred2(i32 %x) {
11641154
; CHECK-LABEL: @isnot_pow2nor0_ctpop_wrong_pred2(
1165-
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
1166-
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[T0]], 1
1167-
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp eq i32 [[X]], 0
1168-
; CHECK-NEXT: [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
1169-
; CHECK-NEXT: ret i1 [[R]]
1155+
; CHECK-NEXT: ret i1 false
11701156
;
11711157
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
11721158
%cmp = icmp eq i32 %t0, 1

llvm/test/Transforms/InstSimplify/and-or-icmp-ctpop.ll

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,7 @@ define <2 x i1> @eq_or_non_0_commute(<2 x i32> %x) {
4040

4141
define i1 @eq_or_non_0_wrong_pred1(i32 %x) {
4242
; CHECK-LABEL: @eq_or_non_0_wrong_pred1(
43-
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]])
44-
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[T0]], 10
45-
; CHECK-NEXT: [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
46-
; CHECK-NEXT: [[R:%.*]] = or i1 [[NOTZERO]], [[CMP]]
47-
; CHECK-NEXT: ret i1 [[R]]
43+
; CHECK-NEXT: ret i1 true
4844
;
4945
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
5046
%cmp = icmp ne i32 %t0, 10
@@ -90,9 +86,7 @@ define i1 @ne_and_is_0_wrong_pred1(i32 %x) {
9086
; CHECK-LABEL: @ne_and_is_0_wrong_pred1(
9187
; CHECK-NEXT: [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]])
9288
; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[T0]], 10
93-
; CHECK-NEXT: [[ISZERO:%.*]] = icmp eq i32 [[X]], 0
94-
; CHECK-NEXT: [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
95-
; CHECK-NEXT: ret i1 [[R]]
89+
; CHECK-NEXT: ret i1 [[CMP]]
9690
;
9791
%t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
9892
%cmp = icmp ne i32 %t0, 10

0 commit comments

Comments
 (0)