[llvm] [NFC][RemoveDIs] Switch constant-hoisting to insert with iterators (PR #84738)
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 19 06:36:22 PDT 2024
https://github.com/SLTozer updated https://github.com/llvm/llvm-project/pull/84738
>From aaaf4583cca106efe86f23c7d6f4eb462624288c Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Mon, 11 Mar 2024 09:37:23 +0000
Subject: [PATCH 1/2] [NFC][RemoveDIs] Switch constant-hoisting to insert with
iterators
Seeing how constant-hoisting also happens to store an insertion-point in a
DenseMap, this means we have to install DenseMapInfo for hashing
BasicBlock::iterators and comparing them.
---
llvm/include/llvm/IR/BasicBlock.h | 25 +++++++++++
.../llvm/Transforms/Scalar/ConstantHoisting.h | 12 +++---
.../Transforms/Scalar/ConstantHoisting.cpp | 41 ++++++++++---------
3 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 179305e9260f91..1add4c2da9d438 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -14,6 +14,7 @@
#define LLVM_IR_BASICBLOCK_H
#include "llvm-c/Types.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/Twine.h"
#include "llvm/ADT/ilist.h"
#include "llvm/ADT/ilist_node.h"
@@ -774,6 +775,30 @@ BasicBlock::iterator skipDebugIntrinsics(BasicBlock::iterator It);
inline void BasicBlock::validateInstrOrdering() const {}
#endif
+// Specialize DenseMapInfo for iterators, so that ththey can be installed into
+// maps and sets. The iterator is made up of its node pointer, and the
+// debug-info "head" bit.
+template<>
+struct DenseMapInfo<BasicBlock::iterator> {
+ static inline BasicBlock::iterator getEmptyKey() {
+ return BasicBlock::iterator(nullptr);
+ }
+
+ static inline BasicBlock::iterator getTombstoneKey() {
+ BasicBlock::iterator It(nullptr);
+ It.setHeadBit(true);
+ return It;
+ }
+
+ static unsigned getHashValue(const BasicBlock::iterator &It) {
+ return DenseMapInfo<void *>::getHashValue(reinterpret_cast<void *>(It.getNodePtr())) ^ It.getHeadBit();
+ }
+
+ static bool isEqual(const BasicBlock::iterator &LHS, const BasicBlock::iterator &RHS) {
+ return LHS == RHS && LHS.getHeadBit() == RHS.getHeadBit();
+ }
+};
+
} // end namespace llvm
#endif // LLVM_IR_BASICBLOCK_H
diff --git a/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h b/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
index a4d1653fdf4bc6..e2c165365bedc0 100644
--- a/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
+++ b/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
@@ -172,11 +172,11 @@ class ConstantHoistingPass : public PassInfoMixin<ConstantHoistingPass> {
void collectMatInsertPts(
const consthoist::RebasedConstantListType &RebasedConstants,
- SmallVectorImpl<Instruction *> &MatInsertPts) const;
- Instruction *findMatInsertPt(Instruction *Inst, unsigned Idx = ~0U) const;
- SetVector<Instruction *>
+ SmallVectorImpl<BasicBlock::iterator> &MatInsertPts) const;
+ BasicBlock::iterator findMatInsertPt(Instruction *Inst, unsigned Idx = ~0U) const;
+ SetVector<BasicBlock::iterator>
findConstantInsertionPoint(const consthoist::ConstantInfo &ConstInfo,
- const ArrayRef<Instruction *> MatInsertPts) const;
+ const ArrayRef<BasicBlock::iterator> MatInsertPts) const;
void collectConstantCandidates(ConstCandMapType &ConstCandMap,
Instruction *Inst, unsigned Idx,
ConstantInt *ConstInt);
@@ -203,9 +203,9 @@ class ConstantHoistingPass : public PassInfoMixin<ConstantHoistingPass> {
struct UserAdjustment {
Constant *Offset;
Type *Ty;
- Instruction *MatInsertPt;
+ BasicBlock::iterator MatInsertPt;
const consthoist::ConstantUser User;
- UserAdjustment(Constant *O, Type *T, Instruction *I,
+ UserAdjustment(Constant *O, Type *T, BasicBlock::iterator I,
consthoist::ConstantUser U)
: Offset(O), Ty(T), MatInsertPt(I), User(U) {}
};
diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
index 49f8761a139232..6d1db436ecd9be 100644
--- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -162,14 +162,14 @@ bool ConstantHoistingLegacyPass::runOnFunction(Function &Fn) {
void ConstantHoistingPass::collectMatInsertPts(
const RebasedConstantListType &RebasedConstants,
- SmallVectorImpl<Instruction *> &MatInsertPts) const {
+ SmallVectorImpl<BasicBlock::iterator> &MatInsertPts) const {
for (const RebasedConstantInfo &RCI : RebasedConstants)
for (const ConstantUser &U : RCI.Uses)
MatInsertPts.emplace_back(findMatInsertPt(U.Inst, U.OpndIdx));
}
/// Find the constant materialization insertion point.
-Instruction *ConstantHoistingPass::findMatInsertPt(Instruction *Inst,
+BasicBlock::iterator ConstantHoistingPass::findMatInsertPt(Instruction *Inst,
unsigned Idx) const {
// If the operand is a cast instruction, then we have to materialize the
// constant before the cast instruction.
@@ -177,12 +177,12 @@ Instruction *ConstantHoistingPass::findMatInsertPt(Instruction *Inst,
Value *Opnd = Inst->getOperand(Idx);
if (auto CastInst = dyn_cast<Instruction>(Opnd))
if (CastInst->isCast())
- return CastInst;
+ return CastInst->getIterator();
}
// The simple and common case. This also includes constant expressions.
if (!isa<PHINode>(Inst) && !Inst->isEHPad())
- return Inst;
+ return Inst->getIterator();
// We can't insert directly before a phi node or an eh pad. Insert before
// the terminator of the incoming or dominating block.
@@ -191,7 +191,7 @@ Instruction *ConstantHoistingPass::findMatInsertPt(Instruction *Inst,
if (Idx != ~0U && isa<PHINode>(Inst)) {
InsertionBlock = cast<PHINode>(Inst)->getIncomingBlock(Idx);
if (!InsertionBlock->isEHPad()) {
- return InsertionBlock->getTerminator();
+ return InsertionBlock->getTerminator()->getIterator();
}
} else {
InsertionBlock = Inst->getParent();
@@ -206,7 +206,7 @@ Instruction *ConstantHoistingPass::findMatInsertPt(Instruction *Inst,
IDom = IDom->getIDom();
}
- return IDom->getBlock()->getTerminator();
+ return IDom->getBlock()->getTerminator()->getIterator();
}
/// Given \p BBs as input, find another set of BBs which collectively
@@ -314,26 +314,26 @@ static void findBestInsertionSet(DominatorTree &DT, BlockFrequencyInfo &BFI,
}
/// Find an insertion point that dominates all uses.
-SetVector<Instruction *> ConstantHoistingPass::findConstantInsertionPoint(
+SetVector<BasicBlock::iterator> ConstantHoistingPass::findConstantInsertionPoint(
const ConstantInfo &ConstInfo,
- const ArrayRef<Instruction *> MatInsertPts) const {
+ const ArrayRef<BasicBlock::iterator> MatInsertPts) const {
assert(!ConstInfo.RebasedConstants.empty() && "Invalid constant info entry.");
// Collect all basic blocks.
SetVector<BasicBlock *> BBs;
- SetVector<Instruction *> InsertPts;
+ SetVector<BasicBlock::iterator> InsertPts;
- for (Instruction *MatInsertPt : MatInsertPts)
+ for (BasicBlock::iterator MatInsertPt : MatInsertPts)
BBs.insert(MatInsertPt->getParent());
if (BBs.count(Entry)) {
- InsertPts.insert(&Entry->front());
+ InsertPts.insert(Entry->begin());
return InsertPts;
}
if (BFI) {
findBestInsertionSet(*DT, *BFI, Entry, BBs);
for (BasicBlock *BB : BBs)
- InsertPts.insert(&*BB->getFirstInsertionPt());
+ InsertPts.insert(BB->getFirstInsertionPt());
return InsertPts;
}
@@ -343,7 +343,7 @@ SetVector<Instruction *> ConstantHoistingPass::findConstantInsertionPoint(
BB2 = BBs.pop_back_val();
BB = DT->findNearestCommonDominator(BB1, BB2);
if (BB == Entry) {
- InsertPts.insert(&Entry->front());
+ InsertPts.insert(Entry->begin());
return InsertPts;
}
BBs.insert(BB);
@@ -761,11 +761,11 @@ void ConstantHoistingPass::emitBaseConstants(Instruction *Base,
Mat = GetElementPtrInst::Create(Type::getInt8Ty(*Ctx), Base, Adj->Offset,
"mat_gep", Adj->MatInsertPt);
// Hide it behind a bitcast.
- Mat = new BitCastInst(Mat, Adj->Ty, "mat_bitcast", Adj->MatInsertPt);
+ Mat = new BitCastInst(Mat, Adj->Ty, "mat_bitcast", Adj->MatInsertPt->getIterator());
} else
// Constant being rebased is a ConstantInt.
Mat = BinaryOperator::Create(Instruction::Add, Base, Adj->Offset,
- "const_mat", Adj->MatInsertPt);
+ "const_mat", Adj->MatInsertPt->getIterator());
LLVM_DEBUG(dbgs() << "Materialize constant (" << *Base->getOperand(0)
<< " + " << *Adj->Offset << ") in BB "
@@ -816,7 +816,8 @@ void ConstantHoistingPass::emitBaseConstants(Instruction *Base,
// Aside from constant GEPs, only constant cast expressions are collected.
assert(ConstExpr->isCast() && "ConstExpr should be a cast");
- Instruction *ConstExprInst = ConstExpr->getAsInstruction(Adj->MatInsertPt);
+ Instruction *ConstExprInst = ConstExpr->getAsInstruction();
+ ConstExprInst->insertBefore(Adj->MatInsertPt);
ConstExprInst->setOperand(0, Mat);
// Use the same debug location as the instruction we are about to update.
@@ -842,9 +843,9 @@ bool ConstantHoistingPass::emitBaseConstants(GlobalVariable *BaseGV) {
SmallVectorImpl<consthoist::ConstantInfo> &ConstInfoVec =
BaseGV ? ConstGEPInfoMap[BaseGV] : ConstIntInfoVec;
for (const consthoist::ConstantInfo &ConstInfo : ConstInfoVec) {
- SmallVector<Instruction *, 4> MatInsertPts;
+ SmallVector<BasicBlock::iterator, 4> MatInsertPts;
collectMatInsertPts(ConstInfo.RebasedConstants, MatInsertPts);
- SetVector<Instruction *> IPSet =
+ SetVector<BasicBlock::iterator> IPSet =
findConstantInsertionPoint(ConstInfo, MatInsertPts);
// We can have an empty set if the function contains unreachable blocks.
if (IPSet.empty())
@@ -853,7 +854,7 @@ bool ConstantHoistingPass::emitBaseConstants(GlobalVariable *BaseGV) {
unsigned UsesNum = 0;
unsigned ReBasesNum = 0;
unsigned NotRebasedNum = 0;
- for (Instruction *IP : IPSet) {
+ for (const BasicBlock::iterator &IP : IPSet) {
// First, collect constants depending on this IP of the base.
UsesNum = 0;
SmallVector<UserAdjustment, 4> ToBeRebased;
@@ -861,7 +862,7 @@ bool ConstantHoistingPass::emitBaseConstants(GlobalVariable *BaseGV) {
for (auto const &RCI : ConstInfo.RebasedConstants) {
UsesNum += RCI.Uses.size();
for (auto const &U : RCI.Uses) {
- Instruction *MatInsertPt = MatInsertPts[MatCtr++];
+ const BasicBlock::iterator &MatInsertPt = MatInsertPts[MatCtr++];
BasicBlock *OrigMatInsertBB = MatInsertPt->getParent();
// If Base constant is to be inserted in multiple places,
// generate rebase for U using the Base dominating U.
>From 40e706e7d078af0e2346aeb20fd5b7120ac6bdcc Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Tue, 19 Mar 2024 13:36:03 +0000
Subject: [PATCH 2/2] clang-format
---
llvm/include/llvm/IR/BasicBlock.h | 11 ++++++-----
.../llvm/Transforms/Scalar/ConstantHoisting.h | 9 +++++----
llvm/lib/Transforms/Scalar/ConstantHoisting.cpp | 13 ++++++++-----
3 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 1add4c2da9d438..86d026126316d0 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -22,7 +22,6 @@
#include "llvm/ADT/iterator_range.h"
#include "llvm/IR/DebugProgramInstruction.h"
#include "llvm/IR/Instruction.h"
-#include "llvm/IR/DebugProgramInstruction.h"
#include "llvm/IR/SymbolTableListTraits.h"
#include "llvm/IR/Value.h"
#include <cassert>
@@ -778,8 +777,7 @@ inline void BasicBlock::validateInstrOrdering() const {}
// Specialize DenseMapInfo for iterators, so that ththey can be installed into
// maps and sets. The iterator is made up of its node pointer, and the
// debug-info "head" bit.
-template<>
-struct DenseMapInfo<BasicBlock::iterator> {
+template <> struct DenseMapInfo<BasicBlock::iterator> {
static inline BasicBlock::iterator getEmptyKey() {
return BasicBlock::iterator(nullptr);
}
@@ -791,10 +789,13 @@ struct DenseMapInfo<BasicBlock::iterator> {
}
static unsigned getHashValue(const BasicBlock::iterator &It) {
- return DenseMapInfo<void *>::getHashValue(reinterpret_cast<void *>(It.getNodePtr())) ^ It.getHeadBit();
+ return DenseMapInfo<void *>::getHashValue(
+ reinterpret_cast<void *>(It.getNodePtr())) ^
+ It.getHeadBit();
}
- static bool isEqual(const BasicBlock::iterator &LHS, const BasicBlock::iterator &RHS) {
+ static bool isEqual(const BasicBlock::iterator &LHS,
+ const BasicBlock::iterator &RHS) {
return LHS == RHS && LHS.getHeadBit() == RHS.getHeadBit();
}
};
diff --git a/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h b/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
index e2c165365bedc0..48f8421c1813c8 100644
--- a/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
+++ b/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
@@ -173,10 +173,11 @@ class ConstantHoistingPass : public PassInfoMixin<ConstantHoistingPass> {
void collectMatInsertPts(
const consthoist::RebasedConstantListType &RebasedConstants,
SmallVectorImpl<BasicBlock::iterator> &MatInsertPts) const;
- BasicBlock::iterator findMatInsertPt(Instruction *Inst, unsigned Idx = ~0U) const;
- SetVector<BasicBlock::iterator>
- findConstantInsertionPoint(const consthoist::ConstantInfo &ConstInfo,
- const ArrayRef<BasicBlock::iterator> MatInsertPts) const;
+ BasicBlock::iterator findMatInsertPt(Instruction *Inst,
+ unsigned Idx = ~0U) const;
+ SetVector<BasicBlock::iterator> findConstantInsertionPoint(
+ const consthoist::ConstantInfo &ConstInfo,
+ const ArrayRef<BasicBlock::iterator> MatInsertPts) const;
void collectConstantCandidates(ConstCandMapType &ConstCandMap,
Instruction *Inst, unsigned Idx,
ConstantInt *ConstInt);
diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
index 6d1db436ecd9be..4f9d9c84fe9303 100644
--- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -170,7 +170,7 @@ void ConstantHoistingPass::collectMatInsertPts(
/// Find the constant materialization insertion point.
BasicBlock::iterator ConstantHoistingPass::findMatInsertPt(Instruction *Inst,
- unsigned Idx) const {
+ unsigned Idx) const {
// If the operand is a cast instruction, then we have to materialize the
// constant before the cast instruction.
if (Idx != ~0U) {
@@ -314,7 +314,8 @@ static void findBestInsertionSet(DominatorTree &DT, BlockFrequencyInfo &BFI,
}
/// Find an insertion point that dominates all uses.
-SetVector<BasicBlock::iterator> ConstantHoistingPass::findConstantInsertionPoint(
+SetVector<BasicBlock::iterator>
+ConstantHoistingPass::findConstantInsertionPoint(
const ConstantInfo &ConstInfo,
const ArrayRef<BasicBlock::iterator> MatInsertPts) const {
assert(!ConstInfo.RebasedConstants.empty() && "Invalid constant info entry.");
@@ -761,11 +762,13 @@ void ConstantHoistingPass::emitBaseConstants(Instruction *Base,
Mat = GetElementPtrInst::Create(Type::getInt8Ty(*Ctx), Base, Adj->Offset,
"mat_gep", Adj->MatInsertPt);
// Hide it behind a bitcast.
- Mat = new BitCastInst(Mat, Adj->Ty, "mat_bitcast", Adj->MatInsertPt->getIterator());
+ Mat = new BitCastInst(Mat, Adj->Ty, "mat_bitcast",
+ Adj->MatInsertPt->getIterator());
} else
// Constant being rebased is a ConstantInt.
- Mat = BinaryOperator::Create(Instruction::Add, Base, Adj->Offset,
- "const_mat", Adj->MatInsertPt->getIterator());
+ Mat =
+ BinaryOperator::Create(Instruction::Add, Base, Adj->Offset,
+ "const_mat", Adj->MatInsertPt->getIterator());
LLVM_DEBUG(dbgs() << "Materialize constant (" << *Base->getOperand(0)
<< " + " << *Adj->Offset << ") in BB "
More information about the llvm-commits
mailing list