[llvm] 5b59b3a - [NFC][RemoveDIs] Switch constant-hoisting to insert with iterators (#84738)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 19 06:39:51 PDT 2024
Author: Jeremy Morse
Date: 2024-03-19T13:39:46Z
New Revision: 5b59b3afe1cfe52c3c2cb9c9817ca21e589928a0
URL: https://github.com/llvm/llvm-project/commit/5b59b3afe1cfe52c3c2cb9c9817ca21e589928a0
DIFF: https://github.com/llvm/llvm-project/commit/5b59b3afe1cfe52c3c2cb9c9817ca21e589928a0.diff
LOG: [NFC][RemoveDIs] Switch constant-hoisting to insert with iterators (#84738)
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. I'm not really sure where to
put the DenseMapInfo declarations as BasicBlock.h seems most logical,
but that then means including DenseMap.h into pretty much all of LLVM.
I've sent this up to the compile time tracker to see whether there's a
major cost from this.
---------
Merged by: Stephen Tozer <stephen.tozer at sony.com>
Added:
Modified:
llvm/include/llvm/IR/BasicBlock.h
llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 71c1a8394896b7..88e341d7ad6f8c 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"
@@ -21,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>
@@ -774,6 +774,32 @@ 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..48f8421c1813c8 100644
--- a/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
+++ b/llvm/include/llvm/Transforms/Scalar/ConstantHoisting.h
@@ -172,11 +172,12 @@ 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 *>
- findConstantInsertionPoint(const consthoist::ConstantInfo &ConstInfo,
- const ArrayRef<Instruction *> MatInsertPts) const;
+ 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;
void collectConstantCandidates(ConstCandMapType &ConstCandMap,
Instruction *Inst, unsigned Idx,
ConstantInt *ConstInt);
@@ -203,9 +204,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 c12dc281b0e3c4..68774cf31ce9b3 100644
--- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -162,27 +162,27 @@ 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,
- unsigned Idx) const {
+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.
if (Idx != ~0U) {
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,27 @@ 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 +344,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);
@@ -764,11 +765,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);
+ 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);
+ 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 "
@@ -819,7 +822,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.
@@ -845,9 +849,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())
@@ -856,7 +860,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;
@@ -864,7 +868,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.
More information about the llvm-commits
mailing list