[llvm] 878cb38 - [InstCombine] Add replaceOperand() helper
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 3 10:04:27 PST 2020
Author: Nikita Popov
Date: 2020-02-03T19:00:17+01:00
New Revision: 878cb38a5c4272005f65554da0ec090c7c1c1f6a
URL: https://github.com/llvm/llvm-project/commit/878cb38a5c4272005f65554da0ec090c7c1c1f6a
DIFF: https://github.com/llvm/llvm-project/commit/878cb38a5c4272005f65554da0ec090c7c1c1f6a.diff
LOG: [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
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
llvm/lib/Transforms/InstCombine/InstCombineInternal.h
llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index cd7745ada9db..bc788f6b5ce5 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -143,8 +143,7 @@ Instruction *InstCombiner::OptAndOp(BinaryOperator *Op,
// 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 a44955ebc13c..64dca01e1b21 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1204,19 +1204,15 @@ static Instruction *foldCttzCtlz(IntrinsicInst &II, InstCombiner &IC) {
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 @@ static Instruction *foldCttzCtlz(IntrinsicInst &II, InstCombiner &IC) {
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 @@ static Instruction *foldCtpop(IntrinsicInst &II, InstCombiner &IC) {
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 @@ Instruction *InstCombiner::visitCallInst(CallInst &CI) {
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 @@ Instruction *InstCombiner::visitCallInst(CallInst &CI) {
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 @@ Instruction *InstCombiner::visitCallInst(CallInst &CI) {
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 5aba2f974b42..3d53d2a39a1e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -1821,9 +1821,7 @@ Instruction *InstCombiner::commonPointerCastTransforms(CastInst &CI) {
// 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 ac2bd5408d3c..e0c90b240418 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -1575,11 +1575,8 @@ Instruction *InstCombiner::foldICmpXorConstant(ICmpInst &Cmp,
// 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 @@ Instruction *InstCombiner::foldICmpAndShift(ICmpInst &Cmp, BinaryOperator *And,
// 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 @@ Instruction *InstCombiner::foldICmpAndConstConst(ICmpInst &Cmp,
}
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 859df58d478b..215183d9e109 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -683,6 +683,13 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner
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 a70065fa7e36..a2be01585b05 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -187,9 +187,7 @@ static Instruction *simplifyAllocaArraySize(InstCombiner &IC, AllocaInst &AI) {
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 @@ static Instruction *simplifyAllocaArraySize(InstCombiner &IC, AllocaInst &AI) {
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 @@ Instruction *InstCombiner::visitAllocaInst(AllocaInst &AI) {
// 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 @@ Instruction *InstCombiner::visitLoadInst(LoadInst &LI) {
// 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 @@ Instruction *InstCombiner::visitStoreInst(StoreInst &SI) {
// 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 43b95f06c0f2..5b825581a32f 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -1417,11 +1417,8 @@ Instruction *InstCombiner::visitSRem(BinaryOperator &I) {
{
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 @@ Instruction *InstCombiner::visitSRem(BinaryOperator &I) {
}
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 584c8aa21a8d..516da231bb28 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
@@ -399,11 +399,8 @@ Instruction *InstCombiner::visitExtractElementInst(ExtractElementInst &EI) {
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 32dc390f3d66..f6bc83347e05 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2740,11 +2740,9 @@ Instruction *InstCombiner::visitReturnInst(ReturnInst &RI) {
// 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;
}
More information about the llvm-commits
mailing list