[PredicateInfo] Use BumpPtrAllocator for predicates (NFC) #145692
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Currently predicates are allocated on the heap and tracked with an intrusive list. Use a bump pointer allocator instead, which is more efficient. The list is no longer needed, as we don't have to track predicates for freeing. The bump pointer allocator is provided as a parameter for PredicateInfo to allow reusing the same allocator for all functions during IPSCCP.
Member
| @llvm/pr-subscribers-function-specialization Author: Nikita Popov (nikic) ChangesCurrently predicates are allocated on the heap and tracked with an intrusive list. Use a bump pointer allocator instead, which is more efficient. The list is no longer needed, as we don't have to track predicates for freeing. The bump pointer allocator is provided as a parameter for PredicateInfo to allow reusing the same allocator for all functions during IPSCCP. Full diff: https://github.com/llvm/llvm-project/pull/145692.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/PredicateInfo.h b/llvm/include/llvm/Transforms/Utils/PredicateInfo.h index 1619fa31fb6f4..c243e236901d5 100644 --- a/llvm/include/llvm/Transforms/Utils/PredicateInfo.h +++ b/llvm/include/llvm/Transforms/Utils/PredicateInfo.h @@ -52,11 +52,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallSet.h" -#include "llvm/ADT/ilist.h" -#include "llvm/ADT/ilist_node.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/PassManager.h" #include "llvm/IR/ValueHandle.h" +#include "llvm/Support/Allocator.h" #include "llvm/Support/Compiler.h" namespace llvm { @@ -79,7 +78,7 @@ struct PredicateConstraint { // Base class for all predicate information we provide. // All of our predicate information has at least a comparison. -class PredicateBase : public ilist_node<PredicateBase> { +class PredicateBase { public: PredicateType Type; // The original operand before we renamed it. @@ -96,7 +95,6 @@ class PredicateBase : public ilist_node<PredicateBase> { PredicateBase(const PredicateBase &) = delete; PredicateBase &operator=(const PredicateBase &) = delete; PredicateBase() = delete; - virtual ~PredicateBase() = default; static bool classof(const PredicateBase *PB) { return PB->Type == PT_Assume || PB->Type == PT_Branch || PB->Type == PT_Switch; @@ -177,7 +175,8 @@ class PredicateSwitch : public PredicateWithEdge { /// accesses. class PredicateInfo { public: - LLVM_ABI PredicateInfo(Function &, DominatorTree &, AssumptionCache &); + LLVM_ABI PredicateInfo(Function &, DominatorTree &, AssumptionCache &, + BumpPtrAllocator &); LLVM_ABI ~PredicateInfo(); LLVM_ABI void verifyPredicateInfo() const; @@ -197,9 +196,6 @@ class PredicateInfo { private: Function &F; - // This owns the all the predicate infos in the function, placed or not. - iplist<PredicateBase> AllInfos; - // This maps from copy operands to Predicate Info. Note that it does not own // the Predicate Info, they belong to the ValueInfo structs in the ValueInfos // vector. diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp index 584cb0e89ef66..7eeaaa0d99602 100644 --- a/llvm/lib/Transforms/Scalar/NewGVN.cpp +++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp @@ -505,13 +505,14 @@ class NewGVN { MemorySSAWalker *MSSAWalker = nullptr; AssumptionCache *AC = nullptr; const DataLayout &DL; - std::unique_ptr<PredicateInfo> PredInfo; // These are the only two things the create* functions should have // side-effects on due to allocating memory. mutable BumpPtrAllocator ExpressionAllocator; mutable ArrayRecycler<Value *> ArgRecycler; mutable TarjanSCC SCCFinder; + + std::unique_ptr<PredicateInfo> PredInfo; const SimplifyQuery SQ; // Number of function arguments, used by ranking @@ -673,7 +674,9 @@ class NewGVN { TargetLibraryInfo *TLI, AliasAnalysis *AA, MemorySSA *MSSA, const DataLayout &DL) : F(F), DT(DT), TLI(TLI), AA(AA), MSSA(MSSA), AC(AC), DL(DL), - PredInfo(std::make_unique<PredicateInfo>(F, *DT, *AC)), + // Reuse ExpressionAllocator for PredicateInfo as well. + PredInfo( + std::make_unique<PredicateInfo>(F, *DT, *AC, ExpressionAllocator)), SQ(DL, TLI, DT, AC, /*CtxI=*/nullptr, /*UseInstrInfo=*/false, /*CanUseUndef=*/false) {} diff --git a/llvm/lib/Transforms/Utils/PredicateInfo.cpp b/llvm/lib/Transforms/Utils/PredicateInfo.cpp index d26707a1361b3..ac413c9b58152 100644 --- a/llvm/lib/Transforms/Utils/PredicateInfo.cpp +++ b/llvm/lib/Transforms/Utils/PredicateInfo.cpp @@ -214,6 +214,8 @@ class PredicateInfoBuilder { // whether it returned a valid result. DenseMap<Value *, unsigned int> ValueInfoNums; + BumpPtrAllocator &Allocator; + ValueInfo &getOrCreateValueInfo(Value *); const ValueInfo &getValueInfo(Value *) const; @@ -242,8 +244,8 @@ class PredicateInfoBuilder { public: PredicateInfoBuilder(PredicateInfo &PI, Function &F, DominatorTree &DT, - AssumptionCache &AC) - : PI(PI), F(F), DT(DT), AC(AC) { + AssumptionCache &AC, BumpPtrAllocator &Allocator) + : PI(PI), F(F), DT(DT), AC(AC), Allocator(Allocator) { // Push an empty operand info so that we can detect 0 as not finding one ValueInfos.resize(1); } @@ -341,7 +343,6 @@ void PredicateInfoBuilder::addInfoFor(SmallVectorImpl<Value *> &OpsToRename, auto &OperandInfo = getOrCreateValueInfo(Op); if (OperandInfo.Infos.empty()) OpsToRename.push_back(Op); - PI.AllInfos.push_back(PB); OperandInfo.Infos.push_back(PB); } @@ -373,7 +374,7 @@ void PredicateInfoBuilder::processAssume( for (Value *V : Values) { if (shouldRename(V)) { - auto *PA = new PredicateAssume(V, II, Cond); + auto *PA = new (Allocator) PredicateAssume(V, II, Cond); addInfoFor(OpsToRename, V, PA); } } @@ -419,8 +420,8 @@ void PredicateInfoBuilder::processBranch( for (Value *V : Values) { if (shouldRename(V)) { - PredicateBase *PB = - new PredicateBranch(V, BranchBB, Succ, Cond, TakenEdge); + PredicateBase *PB = new (Allocator) + PredicateBranch(V, BranchBB, Succ, Cond, TakenEdge); addInfoFor(OpsToRename, V, PB); } } @@ -445,7 +446,7 @@ void PredicateInfoBuilder::processSwitch( for (auto C : SI->cases()) { BasicBlock *TargetBlock = C.getCaseSuccessor(); if (SwitchEdges.lookup(TargetBlock) == 1) { - PredicateSwitch *PS = new PredicateSwitch( + PredicateSwitch *PS = new (Allocator) PredicateSwitch( Op, SI->getParent(), TargetBlock, C.getCaseValue(), SI); addInfoFor(OpsToRename, Op, PS); } @@ -704,9 +705,9 @@ PredicateInfoBuilder::getValueInfo(Value *Operand) const { } PredicateInfo::PredicateInfo(Function &F, DominatorTree &DT, - AssumptionCache &AC) + AssumptionCache &AC, BumpPtrAllocator &Allocator) : F(F) { - PredicateInfoBuilder Builder(*this, F, DT, AC); + PredicateInfoBuilder Builder(*this, F, DT, AC, Allocator); Builder.buildPredicateInfo(); } @@ -797,7 +798,8 @@ PreservedAnalyses PredicateInfoPrinterPass::run(Function &F, auto &DT = AM.getResult<DominatorTreeAnalysis>(F); auto &AC = AM.getResult<AssumptionAnalysis>(F); OS << "PredicateInfo for function: " << F.getName() << "\n"; - auto PredInfo = std::make_unique<PredicateInfo>(F, DT, AC); + BumpPtrAllocator Allocator; + auto PredInfo = std::make_unique<PredicateInfo>(F, DT, AC, Allocator); PredInfo->print(OS); replaceCreatedSSACopys(*PredInfo, F); @@ -859,7 +861,8 @@ PreservedAnalyses PredicateInfoVerifierPass::run(Function &F, FunctionAnalysisManager &AM) { auto &DT = AM.getResult<DominatorTreeAnalysis>(F); auto &AC = AM.getResult<AssumptionAnalysis>(F); - std::make_unique<PredicateInfo>(F, DT, AC)->verifyPredicateInfo(); + BumpPtrAllocator Allocator; + std::make_unique<PredicateInfo>(F, DT, AC, Allocator)->verifyPredicateInfo(); return PreservedAnalyses::all(); } diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp index e1b1cf68cd9f0..cb6aafa279b5f 100644 --- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp +++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp @@ -490,6 +490,8 @@ class SCCPInstVisitor : public InstVisitor<SCCPInstVisitor> { LLVMContext &Ctx; + BumpPtrAllocator PredicateInfoAllocator; + private: ConstantInt *getConstantInt(const ValueLatticeElement &IV, Type *Ty) const { return dyn_cast_or_null<ConstantInt>(getConstant(IV, Ty)); @@ -761,7 +763,8 @@ class SCCPInstVisitor : public InstVisitor<SCCPInstVisitor> { public: void addPredicateInfo(Function &F, DominatorTree &DT, AssumptionCache &AC) { - FnPredicateInfo.insert({&F, std::make_unique<PredicateInfo>(F, DT, AC)}); + FnPredicateInfo.insert({&F, std::make_unique<PredicateInfo>( + F, DT, AC, PredicateInfoAllocator)}); } void removeSSACopies(Function &F) { |
Member
| @llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesCurrently predicates are allocated on the heap and tracked with an intrusive list. Use a bump pointer allocator instead, which is more efficient. The list is no longer needed, as we don't have to track predicates for freeing. The bump pointer allocator is provided as a parameter for PredicateInfo to allow reusing the same allocator for all functions during IPSCCP. Full diff: https://github.com/llvm/llvm-project/pull/145692.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/PredicateInfo.h b/llvm/include/llvm/Transforms/Utils/PredicateInfo.h index 1619fa31fb6f4..c243e236901d5 100644 --- a/llvm/include/llvm/Transforms/Utils/PredicateInfo.h +++ b/llvm/include/llvm/Transforms/Utils/PredicateInfo.h @@ -52,11 +52,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallSet.h" -#include "llvm/ADT/ilist.h" -#include "llvm/ADT/ilist_node.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/PassManager.h" #include "llvm/IR/ValueHandle.h" +#include "llvm/Support/Allocator.h" #include "llvm/Support/Compiler.h" namespace llvm { @@ -79,7 +78,7 @@ struct PredicateConstraint { // Base class for all predicate information we provide. // All of our predicate information has at least a comparison. -class PredicateBase : public ilist_node<PredicateBase> { +class PredicateBase { public: PredicateType Type; // The original operand before we renamed it. @@ -96,7 +95,6 @@ class PredicateBase : public ilist_node<PredicateBase> { PredicateBase(const PredicateBase &) = delete; PredicateBase &operator=(const PredicateBase &) = delete; PredicateBase() = delete; - virtual ~PredicateBase() = default; static bool classof(const PredicateBase *PB) { return PB->Type == PT_Assume || PB->Type == PT_Branch || PB->Type == PT_Switch; @@ -177,7 +175,8 @@ class PredicateSwitch : public PredicateWithEdge { /// accesses. class PredicateInfo { public: - LLVM_ABI PredicateInfo(Function &, DominatorTree &, AssumptionCache &); + LLVM_ABI PredicateInfo(Function &, DominatorTree &, AssumptionCache &, + BumpPtrAllocator &); LLVM_ABI ~PredicateInfo(); LLVM_ABI void verifyPredicateInfo() const; @@ -197,9 +196,6 @@ class PredicateInfo { private: Function &F; - // This owns the all the predicate infos in the function, placed or not. - iplist<PredicateBase> AllInfos; - // This maps from copy operands to Predicate Info. Note that it does not own // the Predicate Info, they belong to the ValueInfo structs in the ValueInfos // vector. diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp index 584cb0e89ef66..7eeaaa0d99602 100644 --- a/llvm/lib/Transforms/Scalar/NewGVN.cpp +++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp @@ -505,13 +505,14 @@ class NewGVN { MemorySSAWalker *MSSAWalker = nullptr; AssumptionCache *AC = nullptr; const DataLayout &DL; - std::unique_ptr<PredicateInfo> PredInfo; // These are the only two things the create* functions should have // side-effects on due to allocating memory. mutable BumpPtrAllocator ExpressionAllocator; mutable ArrayRecycler<Value *> ArgRecycler; mutable TarjanSCC SCCFinder; + + std::unique_ptr<PredicateInfo> PredInfo; const SimplifyQuery SQ; // Number of function arguments, used by ranking @@ -673,7 +674,9 @@ class NewGVN { TargetLibraryInfo *TLI, AliasAnalysis *AA, MemorySSA *MSSA, const DataLayout &DL) : F(F), DT(DT), TLI(TLI), AA(AA), MSSA(MSSA), AC(AC), DL(DL), - PredInfo(std::make_unique<PredicateInfo>(F, *DT, *AC)), + // Reuse ExpressionAllocator for PredicateInfo as well. + PredInfo( + std::make_unique<PredicateInfo>(F, *DT, *AC, ExpressionAllocator)), SQ(DL, TLI, DT, AC, /*CtxI=*/nullptr, /*UseInstrInfo=*/false, /*CanUseUndef=*/false) {} diff --git a/llvm/lib/Transforms/Utils/PredicateInfo.cpp b/llvm/lib/Transforms/Utils/PredicateInfo.cpp index d26707a1361b3..ac413c9b58152 100644 --- a/llvm/lib/Transforms/Utils/PredicateInfo.cpp +++ b/llvm/lib/Transforms/Utils/PredicateInfo.cpp @@ -214,6 +214,8 @@ class PredicateInfoBuilder { // whether it returned a valid result. DenseMap<Value *, unsigned int> ValueInfoNums; + BumpPtrAllocator &Allocator; + ValueInfo &getOrCreateValueInfo(Value *); const ValueInfo &getValueInfo(Value *) const; @@ -242,8 +244,8 @@ class PredicateInfoBuilder { public: PredicateInfoBuilder(PredicateInfo &PI, Function &F, DominatorTree &DT, - AssumptionCache &AC) - : PI(PI), F(F), DT(DT), AC(AC) { + AssumptionCache &AC, BumpPtrAllocator &Allocator) + : PI(PI), F(F), DT(DT), AC(AC), Allocator(Allocator) { // Push an empty operand info so that we can detect 0 as not finding one ValueInfos.resize(1); } @@ -341,7 +343,6 @@ void PredicateInfoBuilder::addInfoFor(SmallVectorImpl<Value *> &OpsToRename, auto &OperandInfo = getOrCreateValueInfo(Op); if (OperandInfo.Infos.empty()) OpsToRename.push_back(Op); - PI.AllInfos.push_back(PB); OperandInfo.Infos.push_back(PB); } @@ -373,7 +374,7 @@ void PredicateInfoBuilder::processAssume( for (Value *V : Values) { if (shouldRename(V)) { - auto *PA = new PredicateAssume(V, II, Cond); + auto *PA = new (Allocator) PredicateAssume(V, II, Cond); addInfoFor(OpsToRename, V, PA); } } @@ -419,8 +420,8 @@ void PredicateInfoBuilder::processBranch( for (Value *V : Values) { if (shouldRename(V)) { - PredicateBase *PB = - new PredicateBranch(V, BranchBB, Succ, Cond, TakenEdge); + PredicateBase *PB = new (Allocator) + PredicateBranch(V, BranchBB, Succ, Cond, TakenEdge); addInfoFor(OpsToRename, V, PB); } } @@ -445,7 +446,7 @@ void PredicateInfoBuilder::processSwitch( for (auto C : SI->cases()) { BasicBlock *TargetBlock = C.getCaseSuccessor(); if (SwitchEdges.lookup(TargetBlock) == 1) { - PredicateSwitch *PS = new PredicateSwitch( + PredicateSwitch *PS = new (Allocator) PredicateSwitch( Op, SI->getParent(), TargetBlock, C.getCaseValue(), SI); addInfoFor(OpsToRename, Op, PS); } @@ -704,9 +705,9 @@ PredicateInfoBuilder::getValueInfo(Value *Operand) const { } PredicateInfo::PredicateInfo(Function &F, DominatorTree &DT, - AssumptionCache &AC) + AssumptionCache &AC, BumpPtrAllocator &Allocator) : F(F) { - PredicateInfoBuilder Builder(*this, F, DT, AC); + PredicateInfoBuilder Builder(*this, F, DT, AC, Allocator); Builder.buildPredicateInfo(); } @@ -797,7 +798,8 @@ PreservedAnalyses PredicateInfoPrinterPass::run(Function &F, auto &DT = AM.getResult<DominatorTreeAnalysis>(F); auto &AC = AM.getResult<AssumptionAnalysis>(F); OS << "PredicateInfo for function: " << F.getName() << "\n"; - auto PredInfo = std::make_unique<PredicateInfo>(F, DT, AC); + BumpPtrAllocator Allocator; + auto PredInfo = std::make_unique<PredicateInfo>(F, DT, AC, Allocator); PredInfo->print(OS); replaceCreatedSSACopys(*PredInfo, F); @@ -859,7 +861,8 @@ PreservedAnalyses PredicateInfoVerifierPass::run(Function &F, FunctionAnalysisManager &AM) { auto &DT = AM.getResult<DominatorTreeAnalysis>(F); auto &AC = AM.getResult<AssumptionAnalysis>(F); - std::make_unique<PredicateInfo>(F, DT, AC)->verifyPredicateInfo(); + BumpPtrAllocator Allocator; + std::make_unique<PredicateInfo>(F, DT, AC, Allocator)->verifyPredicateInfo(); return PreservedAnalyses::all(); } diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp index e1b1cf68cd9f0..cb6aafa279b5f 100644 --- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp +++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp @@ -490,6 +490,8 @@ class SCCPInstVisitor : public InstVisitor<SCCPInstVisitor> { LLVMContext &Ctx; + BumpPtrAllocator PredicateInfoAllocator; + private: ConstantInt *getConstantInt(const ValueLatticeElement &IV, Type *Ty) const { return dyn_cast_or_null<ConstantInt>(getConstant(IV, Ty)); @@ -761,7 +763,8 @@ class SCCPInstVisitor : public InstVisitor<SCCPInstVisitor> { public: void addPredicateInfo(Function &F, DominatorTree &DT, AssumptionCache &AC) { - FnPredicateInfo.insert({&F, std::make_unique<PredicateInfo>(F, DT, AC)}); + FnPredicateInfo.insert({&F, std::make_unique<PredicateInfo>( + F, DT, AC, PredicateInfoAllocator)}); } void removeSSACopies(Function &F) { |
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
Currently predicates are allocated on the heap and tracked with an intrusive list. Use a bump pointer allocator instead, which is more efficient. The list is no longer needed, as we don't have to track predicates for freeing. The bump pointer allocator is provided as a parameter for PredicateInfo to allow reusing the same allocator for all functions during IPSCCP.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Currently predicates are allocated on the heap and tracked with an intrusive list. Use a bump pointer allocator instead, which is more efficient. The list is no longer needed, as we don't have to track predicates for freeing.
The bump pointer allocator is provided as a parameter for PredicateInfo to allow reusing the same allocator for all functions during IPSCCP.