-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[IR] Use single map in PredIteratorCache (NFC) #101950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Use a single map storing ArrayRef instead of two separate maps for the base pointer and size. This saves one map lookup per cache accesses. This is an alternative to llvm#101885.
@llvm/pr-subscribers-llvm-ir Author: Nikita Popov (nikic) ChangesUse a single map storing ArrayRef instead of two separate maps for the base pointer and size. This saves one map lookup per cache accesses. This is an alternative to #101885 with a similar compile-time improvement: http://llvm-compile-time-tracker.com/compare.php?from=1fa7f05b709748e8a36936cbb5508070c8214354&to=1af82f2ca51aa324c772941770dd3ee4043b2b4d&stat=instructions:u Full diff: https://github.com/llvm/llvm-project/pull/101950.diff 1 Files Affected:
diff --git a/llvm/include/llvm/IR/PredIteratorCache.h b/llvm/include/llvm/IR/PredIteratorCache.h
index fc8cf20e9f759..d52e78b25fc33 100644
--- a/llvm/include/llvm/IR/PredIteratorCache.h
+++ b/llvm/include/llvm/IR/PredIteratorCache.h
@@ -25,52 +25,29 @@ namespace llvm {
/// predecessor iterator queries. This is useful for code that repeatedly
/// wants the predecessor list for the same blocks.
class PredIteratorCache {
- /// BlockToPredsMap - Pointer to null-terminated list.
- mutable DenseMap<BasicBlock *, BasicBlock **> BlockToPredsMap;
- mutable DenseMap<BasicBlock *, unsigned> BlockToPredCountMap;
+ /// Cached list of predecessors, allocated in Memory.
+ mutable DenseMap<BasicBlock *, ArrayRef<BasicBlock *>> BlockToPredsMap;
/// Memory - This is the space that holds cached preds.
BumpPtrAllocator Memory;
-private:
- /// GetPreds - Get a cached list for the null-terminated predecessor list of
- /// the specified block. This can be used in a loop like this:
- /// for (BasicBlock **PI = PredCache->GetPreds(BB); *PI; ++PI)
- /// use(*PI);
- /// instead of:
- /// for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI)
- BasicBlock **GetPreds(BasicBlock *BB) {
- BasicBlock **&Entry = BlockToPredsMap[BB];
- if (Entry)
+public:
+ size_t size(BasicBlock *BB) { return get(BB).size(); }
+ ArrayRef<BasicBlock *> get(BasicBlock *BB) {
+ ArrayRef<BasicBlock *> &Entry = BlockToPredsMap[BB];
+ if (Entry.data())
return Entry;
SmallVector<BasicBlock *, 32> PredCache(predecessors(BB));
- PredCache.push_back(nullptr); // null terminator.
-
- BlockToPredCountMap[BB] = PredCache.size() - 1;
-
- Entry = Memory.Allocate<BasicBlock *>(PredCache.size());
- std::copy(PredCache.begin(), PredCache.end(), Entry);
+ BasicBlock **Data = Memory.Allocate<BasicBlock *>(PredCache.size());
+ std::copy(PredCache.begin(), PredCache.end(), Data);
+ Entry = ArrayRef(Data, PredCache.size());
return Entry;
}
- unsigned GetNumPreds(BasicBlock *BB) const {
- auto Result = BlockToPredCountMap.find(BB);
- if (Result != BlockToPredCountMap.end())
- return Result->second;
- return BlockToPredCountMap[BB] = pred_size(BB);
- }
-
-public:
- size_t size(BasicBlock *BB) const { return GetNumPreds(BB); }
- ArrayRef<BasicBlock *> get(BasicBlock *BB) {
- return ArrayRef(GetPreds(BB), GetNumPreds(BB));
- }
-
/// clear - Remove all information.
void clear() {
BlockToPredsMap.clear();
- BlockToPredCountMap.clear();
Memory.Reset();
}
};
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit
mutable DenseMap<BasicBlock *, BasicBlock **> BlockToPredsMap; | ||
mutable DenseMap<BasicBlock *, unsigned> BlockToPredCountMap; | ||
/// Cached list of predecessors, allocated in Memory. | ||
mutable DenseMap<BasicBlock *, ArrayRef<BasicBlock *>> BlockToPredsMap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No mutable
.
Use a single map storing ArrayRef instead of two separate maps for the base pointer and size. This saves one map lookup per cache accesses.
This is an alternative to #101885 with a similar compile-time improvement: http://llvm-compile-time-tracker.com/compare.php?from=1fa7f05b709748e8a36936cbb5508070c8214354&to=1af82f2ca51aa324c772941770dd3ee4043b2b4d&stat=instructions:u