[llvm] [NFC][RemoveDIs] Switch ConstantExpr::getAsInstruction to not insert (PR #84737)
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 19 06:58:37 PDT 2024
https://github.com/SLTozer updated https://github.com/llvm/llvm-project/pull/84737
>From d6dbf782d157bc0d05c42d1b11e0eaee6f6e9c5e Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Mon, 11 Mar 2024 09:30:43 +0000
Subject: [PATCH 1/2] [NFC][RemoveDIs] Switch ConstantExpr::getAsInstruction to
not insert
Because the RemoveDIs work is putting a debug-info bit into
BasicBlock::iterator and iterators are needed for insertion, the
getAsInstruction method declaration would need to use a fully defined
instruction-iterator, which leads to a complicated header-inclusion-order
problem. Much simpler to instead just not insert, and make it the
callers problem to insert.
(This is proportionate because there are only four call-sites to
getAsInstruction).
---
llvm/include/llvm/IR/Constants.h | 5 ++---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 13 +++++++++----
llvm/lib/IR/Constants.cpp | 19 +++++++++----------
llvm/lib/IR/ReplaceConstant.cpp | 13 +++++++------
.../Target/XCore/XCoreLowerThreadLocal.cpp | 8 +++++---
5 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index c0ac9a4aa6750c..c5e1e19d649824 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -1289,14 +1289,13 @@ class ConstantExpr : public Constant {
Type *SrcTy = nullptr) const;
/// Returns an Instruction which implements the same operation as this
- /// ConstantExpr. If \p InsertBefore is not null, the new instruction is
- /// inserted before it, otherwise it is not inserted into any basic block.
+ /// ConstantExpr. It is not inserted into any basic block.
///
/// A better approach to this could be to have a constructor for Instruction
/// which would take a ConstantExpr parameter, but that would have spread
/// implementation details of ConstantExpr outside of Constants.cpp, which
/// would make it harder to remove ConstantExprs altogether.
- Instruction *getAsInstruction(Instruction *InsertBefore = nullptr) const;
+ Instruction *getAsInstruction() const;
/// Whether creating a constant expression for this binary operator is
/// desirable.
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index d65ed8c11d86cc..585e08f9c25615 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -5056,10 +5056,15 @@ FunctionCallee OpenMPIRBuilder::createDispatchFiniFunction(unsigned IVSize,
static void replaceConstatExprUsesInFuncWithInstr(ConstantExpr *ConstExpr,
Function *Func) {
- for (User *User : make_early_inc_range(ConstExpr->users()))
- if (auto *Instr = dyn_cast<Instruction>(User))
- if (Instr->getFunction() == Func)
- Instr->replaceUsesOfWith(ConstExpr, ConstExpr->getAsInstruction(Instr));
+ for (User *User : make_early_inc_range(ConstExpr->users())) {
+ if (auto *Instr = dyn_cast<Instruction>(User)) {
+ if (Instr->getFunction() == Func) {
+ Instruction *ConstInst = ConstExpr->getAsInstruction();
+ ConstInst->insertBefore(*Instr->getParent(), Instr->getIterator());
+ Instr->replaceUsesOfWith(ConstExpr, ConstInst);
+ }
+ }
+ }
}
static void replaceConstantValueUsesInFuncWithInstr(llvm::Value *Input,
diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index e6b92aad392f66..111f6bc54b6808 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -3303,7 +3303,7 @@ Value *ConstantExpr::handleOperandChangeImpl(Value *From, Value *ToV) {
NewOps, this, From, To, NumUpdated, OperandNo);
}
-Instruction *ConstantExpr::getAsInstruction(Instruction *InsertBefore) const {
+Instruction *ConstantExpr::getAsInstruction() const {
SmallVector<Value *, 4> ValueOperands(operands());
ArrayRef<Value*> Ops(ValueOperands);
@@ -3322,32 +3322,31 @@ Instruction *ConstantExpr::getAsInstruction(Instruction *InsertBefore) const {
case Instruction::BitCast:
case Instruction::AddrSpaceCast:
return CastInst::Create((Instruction::CastOps)getOpcode(), Ops[0],
- getType(), "", InsertBefore);
+ getType(), "");
case Instruction::InsertElement:
- return InsertElementInst::Create(Ops[0], Ops[1], Ops[2], "", InsertBefore);
+ return InsertElementInst::Create(Ops[0], Ops[1], Ops[2], "");
case Instruction::ExtractElement:
- return ExtractElementInst::Create(Ops[0], Ops[1], "", InsertBefore);
+ return ExtractElementInst::Create(Ops[0], Ops[1], "");
case Instruction::ShuffleVector:
- return new ShuffleVectorInst(Ops[0], Ops[1], getShuffleMask(), "",
- InsertBefore);
+ return new ShuffleVectorInst(Ops[0], Ops[1], getShuffleMask(), "");
case Instruction::GetElementPtr: {
const auto *GO = cast<GEPOperator>(this);
if (GO->isInBounds())
return GetElementPtrInst::CreateInBounds(
- GO->getSourceElementType(), Ops[0], Ops.slice(1), "", InsertBefore);
+ GO->getSourceElementType(), Ops[0], Ops.slice(1), "");
return GetElementPtrInst::Create(GO->getSourceElementType(), Ops[0],
- Ops.slice(1), "", InsertBefore);
+ Ops.slice(1), "");
}
case Instruction::ICmp:
case Instruction::FCmp:
return CmpInst::Create((Instruction::OtherOps)getOpcode(),
(CmpInst::Predicate)getPredicate(), Ops[0], Ops[1],
- "", InsertBefore);
+ "");
default:
assert(getNumOperands() == 2 && "Must be binary operator?");
BinaryOperator *BO = BinaryOperator::Create(
- (Instruction::BinaryOps)getOpcode(), Ops[0], Ops[1], "", InsertBefore);
+ (Instruction::BinaryOps)getOpcode(), Ops[0], Ops[1], "");
if (isa<OverflowingBinaryOperator>(BO)) {
BO->setHasNoUnsignedWrap(SubclassOptionalData &
OverflowingBinaryOperator::NoUnsignedWrap);
diff --git a/llvm/lib/IR/ReplaceConstant.cpp b/llvm/lib/IR/ReplaceConstant.cpp
index 42dec7c72328ea..9b07bd8040492a 100644
--- a/llvm/lib/IR/ReplaceConstant.cpp
+++ b/llvm/lib/IR/ReplaceConstant.cpp
@@ -22,11 +22,13 @@ static bool isExpandableUser(User *U) {
return isa<ConstantExpr>(U) || isa<ConstantAggregate>(U);
}
-static SmallVector<Instruction *, 4> expandUser(Instruction *InsertPt,
+static SmallVector<Instruction *, 4> expandUser(BasicBlock::iterator InsertPt,
Constant *C) {
SmallVector<Instruction *, 4> NewInsts;
if (auto *CE = dyn_cast<ConstantExpr>(C)) {
- NewInsts.push_back(CE->getAsInstruction(InsertPt));
+ Instruction *ConstInst = CE->getAsInstruction();
+ ConstInst->insertBefore(*InsertPt->getParent(), InsertPt);
+ NewInsts.push_back(ConstInst);
} else if (isa<ConstantStruct>(C) || isa<ConstantArray>(C)) {
Value *V = PoisonValue::get(C->getType());
for (auto [Idx, Op] : enumerate(C->operands())) {
@@ -80,12 +82,11 @@ bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
Instruction *I = InstructionWorklist.pop_back_val();
DebugLoc Loc = I->getDebugLoc();
for (Use &U : I->operands()) {
- auto *BI = I;
+ BasicBlock::iterator BI = I->getIterator();
if (auto *Phi = dyn_cast<PHINode>(I)) {
BasicBlock *BB = Phi->getIncomingBlock(U);
- BasicBlock::iterator It = BB->getFirstInsertionPt();
- assert(It != BB->end() && "Unexpected empty basic block");
- BI = &*It;
+ BI = BB->getFirstInsertionPt();
+ assert(BI != BB->end() && "Unexpected empty basic block");
}
if (auto *C = dyn_cast<Constant>(U.get())) {
diff --git a/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp b/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
index b5a683de33ab13..0386e3cb9c5c36 100644
--- a/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
+++ b/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
@@ -88,12 +88,14 @@ static bool replaceConstantExprOp(ConstantExpr *CE, Pass *P) {
BasicBlock *PredBB = PN->getIncomingBlock(I);
if (PredBB->getTerminator()->getNumSuccessors() > 1)
PredBB = SplitEdge(PredBB, PN->getParent());
- Instruction *InsertPos = PredBB->getTerminator();
- Instruction *NewInst = CE->getAsInstruction(InsertPos);
+ BasicBlock::iterator InsertPos = PredBB->getTerminator()->getIterator();
+ Instruction *NewInst = CE->getAsInstruction();
+ NewInst->insertBefore(*PredBB, InsertPos);
PN->setOperand(I, NewInst);
}
} else if (Instruction *Instr = dyn_cast<Instruction>(WU)) {
- Instruction *NewInst = CE->getAsInstruction(Instr);
+ Instruction *NewInst = CE->getAsInstruction();
+ NewInst->insertBefore(*Instr->getParent(), Instr->getIterator());
Instr->replaceUsesOfWith(CE, NewInst);
} else {
ConstantExpr *CExpr = dyn_cast<ConstantExpr>(WU);
>From 2692390fa4c71925215cb65f0227969055fb1957 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Tue, 19 Mar 2024 13:57:58 +0000
Subject: [PATCH 2/2] clang-format
---
llvm/lib/IR/Constants.cpp | 4 ++--
llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 111f6bc54b6808..07b5bced96dae9 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -3333,8 +3333,8 @@ Instruction *ConstantExpr::getAsInstruction() const {
case Instruction::GetElementPtr: {
const auto *GO = cast<GEPOperator>(this);
if (GO->isInBounds())
- return GetElementPtrInst::CreateInBounds(
- GO->getSourceElementType(), Ops[0], Ops.slice(1), "");
+ return GetElementPtrInst::CreateInBounds(GO->getSourceElementType(),
+ Ops[0], Ops.slice(1), "");
return GetElementPtrInst::Create(GO->getSourceElementType(), Ops[0],
Ops.slice(1), "");
}
diff --git a/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp b/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
index 0386e3cb9c5c36..5e91cce1068b46 100644
--- a/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
+++ b/llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
@@ -88,7 +88,8 @@ static bool replaceConstantExprOp(ConstantExpr *CE, Pass *P) {
BasicBlock *PredBB = PN->getIncomingBlock(I);
if (PredBB->getTerminator()->getNumSuccessors() > 1)
PredBB = SplitEdge(PredBB, PN->getParent());
- BasicBlock::iterator InsertPos = PredBB->getTerminator()->getIterator();
+ BasicBlock::iterator InsertPos =
+ PredBB->getTerminator()->getIterator();
Instruction *NewInst = CE->getAsInstruction();
NewInst->insertBefore(*PredBB, InsertPos);
PN->setOperand(I, NewInst);
More information about the llvm-commits
mailing list