[InstCombine] Add replaceOperand() helper
Adds a replaceOperand() helper, which is like Instruction.setOperand()
but adds the old operand to the worklist. This reduces the amount of
missing or incorrect worklist management.
This only applies the helper to a relatively small subset of
setOperand() calls in InstCombine, namely those of the pattern
`I.setOperand(); return &I;`, where it is most obviously applicable.
Differential Revision: https://reviews.llvm.org/D73803
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index cd7745a..bc788f6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -143,8 +143,7 @@
// the XOR is to toggle the bit. If it is clear, then the ADD has
// no effect.
if ((AddRHS & AndRHSV).isNullValue()) { // Bit is not set, noop
- TheAnd.setOperand(0, X);
- return &TheAnd;
+ return replaceOperand(TheAnd, 0, X);
} else {
// Pull the XOR out of the AND.
Value *NewAnd = Builder.CreateAnd(X, AndRHS);
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index a44955e..64dca01 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1204,19 +1204,15 @@
if (IsTZ) {
// cttz(-x) -> cttz(x)
- if (match(Op0, m_Neg(m_Value(X)))) {
- II.setOperand(0, X);
- return ⅈ
- }
+ if (match(Op0, m_Neg(m_Value(X))))
+ return IC.replaceOperand(II, 0, X);
// cttz(abs(x)) -> cttz(x)
// cttz(nabs(x)) -> cttz(x)
Value *Y;
SelectPatternFlavor SPF = matchSelectPattern(Op0, X, Y).Flavor;
- if (SPF == SPF_ABS || SPF == SPF_NABS) {
- II.setOperand(0, X);
- return ⅈ
- }
+ if (SPF == SPF_ABS || SPF == SPF_NABS)
+ return IC.replaceOperand(II, 0, X);
}
KnownBits Known = IC.computeKnownBits(Op0, 0, &II);
@@ -1242,10 +1238,8 @@
if (!Known.One.isNullValue() ||
isKnownNonZero(Op0, IC.getDataLayout(), 0, &IC.getAssumptionCache(), &II,
&IC.getDominatorTree())) {
- if (!match(II.getArgOperand(1), m_One())) {
- II.setOperand(1, IC.Builder.getTrue());
- return ⅈ
- }
+ if (!match(II.getArgOperand(1), m_One()))
+ return IC.replaceOperand(II, 1, IC.Builder.getTrue());
}
// Add range metadata since known bits can't completely reflect what we know.
@@ -1270,10 +1264,8 @@
Value *X;
// ctpop(bitreverse(x)) -> ctpop(x)
// ctpop(bswap(x)) -> ctpop(x)
- if (match(Op0, m_BitReverse(m_Value(X))) || match(Op0, m_BSwap(m_Value(X)))) {
- II.setOperand(0, X);
- return ⅈ
- }
+ if (match(Op0, m_BitReverse(m_Value(X))) || match(Op0, m_BSwap(m_Value(X))))
+ return IC.replaceOperand(II, 0, X);
// FIXME: Try to simplify vectors of integers.
auto *IT = dyn_cast<IntegerType>(Op0->getType());
@@ -3959,8 +3951,7 @@
break;
// If bound_ctrl = 1, row mask = bank mask = 0xf we can omit old value.
- II->setOperand(0, UndefValue::get(Old->getType()));
- return II;
+ return replaceOperand(*II, 0, UndefValue::get(Old->getType()));
}
case Intrinsic::amdgcn_permlane16:
case Intrinsic::amdgcn_permlanex16: {
@@ -3974,8 +3965,7 @@
if (!FetchInvalid->getZExtValue() && !BoundCtrl->getZExtValue())
break;
- II->setArgOperand(0, UndefValue::get(VDstIn->getType()));
- return II;
+ return replaceOperand(*II, 0, UndefValue::get(VDstIn->getType()));
}
case Intrinsic::amdgcn_readfirstlane:
case Intrinsic::amdgcn_readlane: {
@@ -4135,8 +4125,8 @@
if (GCR.getBasePtr() == GCR.getDerivedPtr() &&
GCR.getBasePtrIndex() != GCR.getDerivedPtrIndex()) {
auto *OpIntTy = GCR.getOperand(2)->getType();
- II->setOperand(2, ConstantInt::get(OpIntTy, GCR.getBasePtrIndex()));
- return II;
+ return replaceOperand(*II, 2,
+ ConstantInt::get(OpIntTy, GCR.getBasePtrIndex()));
}
// Translate facts known about a pointer before relocating into
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index 5aba2f9..3d53d2a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -1821,9 +1821,7 @@
// Changing the cast operand is usually not a good idea but it is safe
// here because the pointer operand is being replaced with another
// pointer operand so the opcode doesn't need to change.
- Worklist.push(GEP);
- CI.setOperand(0, GEP->getOperand(0));
- return &CI;
+ return replaceOperand(CI, 0, GEP->getOperand(0));
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index ac2bd54..e0c90b2 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -1575,11 +1575,8 @@
// If the sign bit of the XorCst is not set, there is no change to
// the operation, just stop using the Xor.
- if (!XorC->isNegative()) {
- Cmp.setOperand(0, X);
- Worklist.push(Xor);
- return &Cmp;
- }
+ if (!XorC->isNegative())
+ return replaceOperand(Cmp, 0, X);
// Emit the opposite comparison.
if (TrueIfSigned)
@@ -1705,8 +1702,7 @@
// Compute X & (C2 << Y).
Value *NewAnd = Builder.CreateAnd(Shift->getOperand(0), NewShift);
- Cmp.setOperand(0, NewAnd);
- return &Cmp;
+ return replaceOperand(Cmp, 0, NewAnd);
}
return nullptr;
@@ -1812,8 +1808,7 @@
}
if (NewOr) {
Value *NewAnd = Builder.CreateAnd(A, NewOr, And->getName());
- Cmp.setOperand(0, NewAnd);
- return &Cmp;
+ return replaceOperand(Cmp, 0, NewAnd);
}
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 859df58..215183d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -683,6 +683,13 @@
return &I;
}
+ /// Replace operand of instruction and add old operand to the worklist.
+ Instruction *replaceOperand(Instruction &I, unsigned OpNum, Value *V) {
+ Worklist.addValue(I.getOperand(OpNum));
+ I.setOperand(OpNum, V);
+ return &I;
+ }
+
/// Creates a result tuple for an overflow intrinsic \p II with a given
/// \p Result and a constant \p Overflow value.
Instruction *CreateOverflowTuple(IntrinsicInst *II, Value *Result,
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index a70065f..a2be015 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -187,9 +187,7 @@
return nullptr;
// Canonicalize it.
- Value *V = IC.Builder.getInt32(1);
- AI.setOperand(0, V);
- return &AI;
+ return IC.replaceOperand(AI, 0, IC.Builder.getInt32(1));
}
// Convert: alloca Ty, C - where C is a constant != 1 into: alloca [C x Ty], 1
@@ -230,8 +228,7 @@
Type *IntPtrTy = IC.getDataLayout().getIntPtrType(AI.getType());
if (AI.getArraySize()->getType() != IntPtrTy) {
Value *V = IC.Builder.CreateIntCast(AI.getArraySize(), IntPtrTy, false);
- AI.setOperand(0, V);
- return &AI;
+ return IC.replaceOperand(AI, 0, V);
}
return nullptr;
@@ -355,10 +352,9 @@
// For a zero sized alloca there is no point in doing an array allocation.
// This is helpful if the array size is a complicated expression not used
// elsewhere.
- if (AI.isArrayAllocation()) {
- AI.setOperand(0, ConstantInt::get(AI.getArraySize()->getType(), 1));
- return &AI;
- }
+ if (AI.isArrayAllocation())
+ return replaceOperand(AI, 0,
+ ConstantInt::get(AI.getArraySize()->getType(), 1));
// Get the first instruction in the entry block.
BasicBlock &EntryBlock = AI.getParent()->getParent()->getEntryBlock();
@@ -1048,18 +1044,14 @@
// load (select (cond, null, P)) -> load P
if (isa<ConstantPointerNull>(SI->getOperand(1)) &&
!NullPointerIsDefined(SI->getFunction(),
- LI.getPointerAddressSpace())) {
- LI.setOperand(0, SI->getOperand(2));
- return &LI;
- }
+ LI.getPointerAddressSpace()))
+ return replaceOperand(LI, 0, SI->getOperand(2));
// load (select (cond, P, null)) -> load P
if (isa<ConstantPointerNull>(SI->getOperand(2)) &&
!NullPointerIsDefined(SI->getFunction(),
- LI.getPointerAddressSpace())) {
- LI.setOperand(0, SI->getOperand(1));
- return &LI;
- }
+ LI.getPointerAddressSpace()))
+ return replaceOperand(LI, 0, SI->getOperand(1));
}
}
return nullptr;
@@ -1463,11 +1455,8 @@
// store X, null -> turns into 'unreachable' in SimplifyCFG
// store X, GEP(null, Y) -> turns into 'unreachable' in SimplifyCFG
if (canSimplifyNullStoreOrGEP(SI)) {
- if (!isa<UndefValue>(Val)) {
- SI.setOperand(0, UndefValue::get(Val->getType()));
- if (Instruction *U = dyn_cast<Instruction>(Val))
- Worklist.push(U); // Dropped a use.
- }
+ if (!isa<UndefValue>(Val))
+ return replaceOperand(SI, 0, UndefValue::get(Val->getType()));
return nullptr; // Do not modify these!
}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index 43b95f0..5b82558 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -1417,11 +1417,8 @@
{
const APInt *Y;
// X % -Y -> X % Y
- if (match(Op1, m_Negative(Y)) && !Y->isMinSignedValue()) {
- Worklist.pushValue(I.getOperand(1));
- I.setOperand(1, ConstantInt::get(I.getType(), -*Y));
- return &I;
- }
+ if (match(Op1, m_Negative(Y)) && !Y->isMinSignedValue())
+ return replaceOperand(I, 1, ConstantInt::get(I.getType(), -*Y));
}
// -X srem Y --> -(X srem Y)
@@ -1468,11 +1465,8 @@
}
Constant *NewRHSV = ConstantVector::get(Elts);
- if (NewRHSV != C) { // Don't loop on -MININT
- Worklist.pushValue(I.getOperand(1));
- I.setOperand(1, NewRHSV);
- return &I;
- }
+ if (NewRHSV != C) // Don't loop on -MININT
+ return replaceOperand(I, 1, NewRHSV);
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
index 584c8aa2..516da23 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
@@ -399,11 +399,8 @@
return replaceInstUsesWith(EI, IE->getOperand(1));
// If the inserted and extracted elements are constants, they must not
// be the same value, extract from the pre-inserted value instead.
- if (isa<Constant>(IE->getOperand(2)) && IndexC) {
- Worklist.pushValue(SrcVec);
- EI.setOperand(0, IE->getOperand(0));
- return &EI;
- }
+ if (isa<Constant>(IE->getOperand(2)) && IndexC)
+ return replaceOperand(EI, 0, IE->getOperand(0));
} else if (auto *SVI = dyn_cast<ShuffleVectorInst>(I)) {
// If this is extracting an element from a shufflevector, figure out where
// it came from and extract from the appropriate input element instead.
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 32dc390..f6bc833 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2740,11 +2740,9 @@
// There might be assume intrinsics dominating this return that completely
// determine the value. If so, constant fold it.
KnownBits Known = computeKnownBits(ResultOp, 0, &RI);
- if (Known.isConstant()) {
- Worklist.pushValue(ResultOp);
- RI.setOperand(0, Constant::getIntegerValue(VTy, Known.getConstant()));
- return &RI;
- }
+ if (Known.isConstant())
+ return replaceOperand(RI, 0,
+ Constant::getIntegerValue(VTy, Known.getConstant()));
return nullptr;
}