Skip to content

[GenericCycle] Add a Cache for getExitBlocks in GenericCycle #112290

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 29, 2024
Merged

[GenericCycle] Add a Cache for getExitBlocks in GenericCycle #112290

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
08ce9cd
Add cache in GenericCycleInfo
Chengjunp Oct 14, 2024
dd45e4b
Format
Chengjunp Oct 15, 2024
80792ad
Update cache and clear the cache in non-const func
Chengjunp Oct 16, 2024
75435a8
Format
Chengjunp Oct 16, 2024
b56225b
Remove redundant cache clear
Chengjunp Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions llvm/include/llvm/ADT/GenericCycleImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ bool GenericCycle<ContextT>::contains(const GenericCycle *C) const {
template <typename ContextT>
void GenericCycle<ContextT>::getExitBlocks(
SmallVectorImpl<BlockT *> &TmpStorage) const {
if (!ExitBlocksCache.empty()) {
TmpStorage = ExitBlocksCache;
return;
}

TmpStorage.clear();

size_t NumExitBlocks = 0;
Expand All @@ -65,6 +70,7 @@ void GenericCycle<ContextT>::getExitBlocks(

TmpStorage.resize(NumExitBlocks);
}
ExitBlocksCache.append(TmpStorage.begin(), TmpStorage.end());
}

template <typename ContextT>
Expand Down Expand Up @@ -298,6 +304,8 @@ void GenericCycleInfo<ContextT>::moveTopLevelCycleToNewParent(CycleT *NewParent,
for (auto &It : BlockMapTopLevel)
if (It.second == Child)
It.second = NewParent;
NewParent->clearCache();
Child->clearCache();
}

template <typename ContextT>
Expand All @@ -316,6 +324,7 @@ void GenericCycleInfo<ContextT>::addBlockToCycle(BlockT *Block, CycleT *Cycle) {
}

BlockMapTopLevel.try_emplace(Block, Cycle);
Cycle->clearCache();
}

/// \brief Main function of the cycle info computations.
Expand Down
28 changes: 25 additions & 3 deletions llvm/include/llvm/ADT/GenericCycleInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,27 @@ template <typename ContextT> class GenericCycle {
/// always have the same depth.
unsigned Depth = 0;

/// Cache for the results of GetExitBlocks
mutable SmallVector<BlockT *, 4> ExitBlocksCache;

void clear() {
Entries.clear();
Children.clear();
Blocks.clear();
Depth = 0;
ParentCycle = nullptr;
clearCache();
}

void appendEntry(BlockT *Block) { Entries.push_back(Block); }
void appendBlock(BlockT *Block) { Blocks.insert(Block); }
void appendEntry(BlockT *Block) {
Entries.push_back(Block);
clearCache();
}

void appendBlock(BlockT *Block) {
Blocks.insert(Block);
clearCache();
}

GenericCycle(const GenericCycle &) = delete;
GenericCycle &operator=(const GenericCycle &) = delete;
Expand All @@ -102,6 +113,11 @@ template <typename ContextT> class GenericCycle {
return Entries;
}

/// Clear the cache of the cycle.
/// This should be run in all non-const function in GenericCycle
/// and GenericCycleInfo.
void clearCache() const { ExitBlocksCache.clear(); }

/// \brief Return whether \p Block is an entry block of the cycle.
bool isEntry(const BlockT *Block) const {
return is_contained(Entries, Block);
Expand All @@ -112,6 +128,7 @@ template <typename ContextT> class GenericCycle {
assert(contains(Block));
Entries.clear();
Entries.push_back(Block);
clearCache();
}

/// \brief Return whether \p Block is contained in the cycle.
Expand All @@ -124,7 +141,12 @@ template <typename ContextT> class GenericCycle {
bool contains(const GenericCycle *C) const;

const GenericCycle *getParentCycle() const { return ParentCycle; }
GenericCycle *getParentCycle() { return ParentCycle; }

GenericCycle *getParentCycle() {
clearCache();
return ParentCycle;
}
Copy link
Contributor

@arsenm arsenm Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing the cache in a getter (and only the non-const one) seems dubious

Copy link
Collaborator

@ssahasra ssahasra Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, clearly there is no change in state here and we don't need to clear the cache. At least this one is obvious enough. We could later revisit other non-const functions too, maybe as a future enhancement.


unsigned getDepth() const { return Depth; }

/// Return all of the successor blocks of this cycle.
Expand Down
Loading