Skip to content

Commit d4627b9

Browse files
nikicnikic
committed
[InstCombine] Avoid modifying instructions in-place
As discussed on D73919, this replaces a few cases where we were modifying multiple operands of instructions in-place with the creation of a new instruction, which we generally prefer nowadays. This tends to be more readable and less prone to worklist management bugs. Test changes are only superficial (instruction naming and order).
1 parent 9d03b7d commit d4627b9

File tree

5 files changed

+32
-48
lines changed

5 files changed

+32
-48
lines changed

‎llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,33 +2742,24 @@ static Instruction *foldXorToXor(BinaryOperator &I,
27422742
// (A | B) ^ (A & B) -> A ^ B
27432743
// (A | B) ^ (B & A) -> A ^ B
27442744
if (match(&I, m_c_Xor(m_And(m_Value(A), m_Value(B)),
2745-
m_c_Or(m_Deferred(A), m_Deferred(B))))) {
2746-
I.setOperand(0, A);
2747-
I.setOperand(1, B);
2748-
return &I;
2749-
}
2745+
m_c_Or(m_Deferred(A), m_Deferred(B)))))
2746+
return BinaryOperator::CreateXor(A, B);
27502747

27512748
// (A | ~B) ^ (~A | B) -> A ^ B
27522749
// (~B | A) ^ (~A | B) -> A ^ B
27532750
// (~A | B) ^ (A | ~B) -> A ^ B
27542751
// (B | ~A) ^ (A | ~B) -> A ^ B
27552752
if (match(&I, m_Xor(m_c_Or(m_Value(A), m_Not(m_Value(B))),
2756-
m_c_Or(m_Not(m_Deferred(A)), m_Deferred(B))))) {
2757-
I.setOperand(0, A);
2758-
I.setOperand(1, B);
2759-
return &I;
2760-
}
2753+
m_c_Or(m_Not(m_Deferred(A)), m_Deferred(B)))))
2754+
return BinaryOperator::CreateXor(A, B);
27612755

27622756
// (A & ~B) ^ (~A & B) -> A ^ B
27632757
// (~B & A) ^ (~A & B) -> A ^ B
27642758
// (~A & B) ^ (A & ~B) -> A ^ B
27652759
// (B & ~A) ^ (A & ~B) -> A ^ B
27662760
if (match(&I, m_Xor(m_c_And(m_Value(A), m_Not(m_Value(B))),
2767-
m_c_And(m_Not(m_Deferred(A)), m_Deferred(B))))) {
2768-
I.setOperand(0, A);
2769-
I.setOperand(1, B);
2770-
return &I;
2771-
}
2761+
m_c_And(m_Not(m_Deferred(A)), m_Deferred(B)))))
2762+
return BinaryOperator::CreateXor(A, B);
27722763

27732764
// For the remaining cases we need to get rid of one of the operands.
27742765
if (!Op0->hasOneUse() && !Op1->hasOneUse())
@@ -3109,9 +3100,7 @@ Instruction *InstCombiner::visitXor(BinaryOperator &I) {
31093100
MaskedValueIsZero(X, *C, 0, &I)) {
31103101
Constant *NewC = ConstantInt::get(I.getType(), *C ^ *RHSC);
31113102
Worklist.push(cast<Instruction>(Op0));
3112-
I.setOperand(0, X);
3113-
I.setOperand(1, NewC);
3114-
return &I;
3103+
return BinaryOperator::CreateXor(X, NewC);
31153104
}
31163105
}
31173106
}

‎llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,12 +1680,11 @@ Instruction *InstCombiner::foldICmpAndShift(ICmpInst &Cmp, BinaryOperator *And,
16801680
if (Cmp.getPredicate() == ICmpInst::ICMP_NE)
16811681
return replaceInstUsesWith(Cmp, ConstantInt::getTrue(Cmp.getType()));
16821682
} else {
1683-
Cmp.setOperand(1, ConstantInt::get(And->getType(), NewCst));
16841683
APInt NewAndCst = IsShl ? C2.lshr(*C3) : C2.shl(*C3);
1685-
And->setOperand(1, ConstantInt::get(And->getType(), NewAndCst));
1686-
And->setOperand(0, Shift->getOperand(0));
1687-
Worklist.push(Shift); // Shift is dead.
1688-
return &Cmp;
1684+
Value *NewAnd = Builder.CreateAnd(
1685+
Shift->getOperand(0), ConstantInt::get(And->getType(), NewAndCst));
1686+
return new ICmpInst(Cmp.getPredicate(),
1687+
NewAnd, ConstantInt::get(And->getType(), NewCst));
16891688
}
16901689
}
16911690
}
@@ -4154,9 +4153,7 @@ Instruction *InstCombiner::foldICmpEquality(ICmpInst &I) {
41544153
if (X) { // Build (X^Y) & Z
41554154
Op1 = Builder.CreateXor(X, Y);
41564155
Op1 = Builder.CreateAnd(Op1, Z);
4157-
I.setOperand(0, Op1);
4158-
I.setOperand(1, Constant::getNullValue(Op1->getType()));
4159-
return &I;
4156+
return new ICmpInst(Pred, Op1, Constant::getNullValue(Op1->getType()));
41604157
}
41614158
}
41624159

‎llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1917,10 +1917,8 @@ Instruction *InstCombiner::visitShuffleVectorInst(ShuffleVectorInst &SVI) {
19171917
else
19181918
Elts.push_back(ConstantInt::get(Int32Ty, Mask[i] % LHSWidth));
19191919
}
1920-
SVI.setOperand(0, SVI.getOperand(1));
1921-
SVI.setOperand(1, UndefValue::get(RHS->getType()));
1922-
SVI.setOperand(2, ConstantVector::get(Elts));
1923-
return &SVI;
1920+
return new ShuffleVectorInst(LHS, UndefValue::get(RHS->getType()),
1921+
ConstantVector::get(Elts));
19241922
}
19251923

19261924
// shuffle undef, x, mask --> shuffle x, undef, mask'

‎llvm/test/Transforms/InstCombine/icmp-custom-dl.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,10 @@ define i1 @test62_as1(i8 addrspace(1)* %a) {
183183
; Variation of the above with an ashr
184184
define i1 @icmp_and_ashr_multiuse(i32 %X) {
185185
; CHECK-LABEL: @icmp_and_ashr_multiuse(
186-
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X:%.*]], 240
187-
; CHECK-NEXT: [[AND2:%.*]] = and i32 [[X]], 496
188-
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[AND]], 224
189-
; CHECK-NEXT: [[TOBOOL2:%.*]] = icmp ne i32 [[AND2]], 432
186+
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[X:%.*]], 240
187+
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[TMP1]], 224
188+
; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[X]], 496
189+
; CHECK-NEXT: [[TOBOOL2:%.*]] = icmp ne i32 [[TMP2]], 432
190190
; CHECK-NEXT: [[AND3:%.*]] = and i1 [[TOBOOL]], [[TOBOOL2]]
191191
; CHECK-NEXT: ret i1 [[AND3]]
192192
;

‎llvm/test/Transforms/InstCombine/icmp.ll

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ define <2 x i1> @test5_zero() {
9898

9999
define i32 @test6(i32 %a, i32 %b) {
100100
; CHECK-LABEL: @test6(
101-
; CHECK-NEXT: [[E:%.*]] = ashr i32 [[A:%.*]], 31
102-
; CHECK-NEXT: [[F:%.*]] = and i32 [[E]], [[B:%.*]]
101+
; CHECK-NEXT: [[A_LOBIT_NEG:%.*]] = ashr i32 [[A:%.*]], 31
102+
; CHECK-NEXT: [[F:%.*]] = and i32 [[A_LOBIT_NEG]], [[B:%.*]]
103103
; CHECK-NEXT: ret i32 [[F]]
104104
;
105105
%c = icmp sle i32 %a, -1
@@ -1775,8 +1775,8 @@ define i1 @icmp_and_shl_neg_eq_0(i32 %A, i32 %B) {
17751775

17761776
define i1 @icmp_add_and_shr_ne_0(i32 %X) {
17771777
; CHECK-LABEL: @icmp_add_and_shr_ne_0(
1778-
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X:%.*]], 240
1779-
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[AND]], 224
1778+
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[X:%.*]], 240
1779+
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[TMP1]], 224
17801780
; CHECK-NEXT: ret i1 [[TOBOOL]]
17811781
;
17821782
%shr = lshr i32 %X, 4
@@ -1788,8 +1788,8 @@ define i1 @icmp_add_and_shr_ne_0(i32 %X) {
17881788

17891789
define <2 x i1> @icmp_add_and_shr_ne_0_vec(<2 x i32> %X) {
17901790
; CHECK-LABEL: @icmp_add_and_shr_ne_0_vec(
1791-
; CHECK-NEXT: [[AND:%.*]] = and <2 x i32> [[X:%.*]], <i32 240, i32 240>
1792-
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne <2 x i32> [[AND]], <i32 224, i32 224>
1791+
; CHECK-NEXT: [[TMP1:%.*]] = and <2 x i32> [[X:%.*]], <i32 240, i32 240>
1792+
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne <2 x i32> [[TMP1]], <i32 224, i32 224>
17931793
; CHECK-NEXT: ret <2 x i1> [[TOBOOL]]
17941794
;
17951795
%shr = lshr <2 x i32> %X, <i32 4, i32 4>
@@ -1802,10 +1802,10 @@ define <2 x i1> @icmp_add_and_shr_ne_0_vec(<2 x i32> %X) {
18021802
; Variation of the above with an extra use of the shift
18031803
define i1 @icmp_and_shr_multiuse(i32 %X) {
18041804
; CHECK-LABEL: @icmp_and_shr_multiuse(
1805-
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X:%.*]], 240
1806-
; CHECK-NEXT: [[AND2:%.*]] = and i32 [[X]], 496
1807-
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[AND]], 224
1808-
; CHECK-NEXT: [[TOBOOL2:%.*]] = icmp ne i32 [[AND2]], 432
1805+
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[X:%.*]], 240
1806+
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[TMP1]], 224
1807+
; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[X]], 496
1808+
; CHECK-NEXT: [[TOBOOL2:%.*]] = icmp ne i32 [[TMP2]], 432
18091809
; CHECK-NEXT: [[AND3:%.*]] = and i1 [[TOBOOL]], [[TOBOOL2]]
18101810
; CHECK-NEXT: ret i1 [[AND3]]
18111811
;
@@ -1821,10 +1821,10 @@ define i1 @icmp_and_shr_multiuse(i32 %X) {
18211821
; Variation of the above with an ashr
18221822
define i1 @icmp_and_ashr_multiuse(i32 %X) {
18231823
; CHECK-LABEL: @icmp_and_ashr_multiuse(
1824-
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X:%.*]], 240
1825-
; CHECK-NEXT: [[AND2:%.*]] = and i32 [[X]], 496
1826-
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[AND]], 224
1827-
; CHECK-NEXT: [[TOBOOL2:%.*]] = icmp ne i32 [[AND2]], 432
1824+
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[X:%.*]], 240
1825+
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[TMP1]], 224
1826+
; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[X]], 496
1827+
; CHECK-NEXT: [[TOBOOL2:%.*]] = icmp ne i32 [[TMP2]], 432
18281828
; CHECK-NEXT: [[AND3:%.*]] = and i1 [[TOBOOL]], [[TOBOOL2]]
18291829
; CHECK-NEXT: ret i1 [[AND3]]
18301830
;

0 commit comments

Comments
 (0)