[llvm] [CycleInfo] Reference cycles by preorder index, not pointer. NFC (PR #210271)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 17 01:22:02 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-adt
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
The block-to-innermost-cycle map (BlockMap) and each cycle's parent link
hold a GenericCycle pointer. Store the cycle's preorder index instead, so
no raw cycle pointer remains in the stored representation: BlockMap
becomes a SmallVector<unsigned>, four bytes per block rather than eight,
and the parent link becomes ParentIndex.
The flat Cycles array does not exist during construction, so both hold
creation-order indices into the temporary forest until flatten() resolves
them to preorder indices.
Aided by Claude Opus 4.8
---
Full diff: https://github.com/llvm/llvm-project/pull/210271.diff
2 Files Affected:
- (modified) llvm/include/llvm/ADT/GenericCycleImpl.h (+48-27)
- (modified) llvm/include/llvm/ADT/GenericCycleInfo.h (+30-22)
``````````diff
diff --git a/llvm/include/llvm/ADT/GenericCycleImpl.h b/llvm/include/llvm/ADT/GenericCycleImpl.h
index 092f7bd16a71f..e2da17af20826 100644
--- a/llvm/include/llvm/ADT/GenericCycleImpl.h
+++ b/llvm/include/llvm/ADT/GenericCycleImpl.h
@@ -205,9 +205,9 @@ void GenericCycleInfo<ContextT>::verifyCycleNest(CycleRef C) const {
assert(deref(Child).Depth == Cyc.Depth + 1);
}
- // Check the parent cycle pointer.
- if (Cyc.ParentCycle) {
- assert(is_contained(children(ref(*Cyc.ParentCycle)), C) &&
+ // Check the parent cycle.
+ if (Cyc.hasParent()) {
+ assert(is_contained(children(CycleRef(Cyc.ParentIndex)), C) &&
"Cycle is not a subcycle of its parent!");
}
#endif
@@ -267,16 +267,26 @@ template <typename ContextT> class GenericCycleInfoCompute {
/// Make top-level cycle \p Child a child of \p NewParent.
void moveTopLevelCycleToNewParent(CycleT *NewParent, CycleT *Child) {
- assert((!Child->ParentCycle && !NewParent->ParentCycle) &&
+ assert((!Child->hasParent() && !NewParent->hasParent()) &&
"NewParent and Child must be both top level cycle!\n");
- assert(NewParent == &AllCycles.back() &&
- "attach slices in AttachedChildren must stay contiguous");
auto Pos = llvm::find(TopLevelCycles, Child);
assert(Pos != TopLevelCycles.end());
*Pos = TopLevelCycles.back();
TopLevelCycles.pop_back();
AttachedChildren.push_back(Child);
- Child->ParentCycle = NewParent;
+ // NewParent is the newest cycle and its creation-order index is
+ // AllCycles.size() - 1.
+ assert(NewParent == &AllCycles.back() &&
+ "attach slices in AttachedChildren must stay contiguous");
+ Child->ParentIndex = AllCycles.size() - 1;
+ }
+
+ /// Record that \p Block's innermost cycle is the one currently being built
+ /// (always AllCycles.back()), storing its creation-order index. flatten()
+ /// remaps it to the preorder index.
+ void recordInnermostCycle(BlockT *Block) {
+ Info.BlockMap[GraphTraits<BlockT *>::getNumber(Block)] =
+ AllCycles.size() - 1;
}
public:
@@ -291,10 +301,11 @@ template <typename ContextT> class GenericCycleInfoCompute {
template <typename ContextT>
void GenericCycleInfo<ContextT>::addToBlockMap(BlockT *Block, CycleT *Cycle) {
- // The caller should ensure that BlockMap is large enough.
+ // The caller should ensure that BlockMap is large enough. \p Cycle is a flat
+ // cycle, so its preorder index is well-defined.
verifyBlockNumberEpoch(Block->getParent());
unsigned Number = GraphTraits<BlockT *>::getNumber(Block);
- BlockMap[Number] = Cycle;
+ BlockMap[Number] = getCycleIndex(*Cycle);
}
template <typename ContextT>
@@ -303,7 +314,8 @@ void GenericCycleInfo<ContextT>::addBlockToCycle(BlockT *Block, CycleRef C) {
// Make sure BlockMap is large enough for the new block.
unsigned Number = GraphTraits<BlockT *>::getNumber(Block);
if (Number >= BlockMap.size())
- BlockMap.resize(GraphTraits<FunctionT *>::getMaxNumber(Block->getParent()));
+ BlockMap.resize(GraphTraits<FunctionT *>::getMaxNumber(Block->getParent()),
+ NoCycle);
// Insert Block at the end of Cyc's slice and shift every later cycle's
// range right. Ranges straddling Pos belong to Cyc's ancestors and are
@@ -320,16 +332,17 @@ void GenericCycleInfo<ContextT>::addBlockToCycle(BlockT *Block, CycleRef C) {
addToBlockMap(Block, &Cyc);
// Cyc and its ancestors gain the new block: extend each one's slice and
// invalidate its exit-block cache in a single walk up the tree.
- for (CycleT *P = &Cyc; P; P = P->ParentCycle) {
- ++P->IdxEnd;
+ for (unsigned I = getCycleIndex(Cyc); I != NoCycle;
+ I = Cycles[I].ParentIndex) {
+ ++Cycles[I].IdxEnd;
if (!ExitBlocksCaches.empty())
- ExitBlocksCaches[getCycleIndex(*P)].clear();
+ ExitBlocksCaches[I].clear();
}
}
/// Move the discovered forest into Info's flat preorder array. Assigns preorder
-/// IDs, depths and descendant counts, remaps BlockMap from the temporary nodes,
-/// and lays out every cycle's blocks in BlockLayout.
+/// IDs, depths and descendant counts, remaps BlockMap from creation-order to
+/// preorder indices, and lays out every cycle's blocks in BlockLayout.
template <typename ContextT>
void GenericCycleInfoCompute<ContextT>::flatten(ArrayRef<BlockT *> Order) {
unsigned N = AllCycles.size();
@@ -354,7 +367,8 @@ void GenericCycleInfoCompute<ContextT>::flatten(ArrayRef<BlockT *> Order) {
auto enter = [&](CycleT *Temp, CycleT *Parent) {
unsigned ID = NextID++;
CycleT &Flat = Info.Cycles[ID];
- Flat.ParentCycle = Parent;
+ Flat.ParentIndex =
+ Parent ? Info.getCycleIndex(*Parent) : CycleInfoT::NoCycle;
Flat.Depth = Parent ? Parent->Depth + 1 : 1;
Flat.Entries = std::move(Temp->Entries);
Cursor += Temp->IdxEnd; // IdxEnd currently holds Temp's own-block count.
@@ -378,14 +392,18 @@ void GenericCycleInfoCompute<ContextT>::flatten(ArrayRef<BlockT *> Order) {
}
// Place every block into its innermost cycle's own region, remapping its
- // BlockMap entry from the temporary node to the flat one.
+ // BlockMap entry from a creation-order index to the flat preorder index.
Info.BlockLayout.resize_for_overwrite(Cursor);
for (BlockT *B : llvm::reverse(Order)) {
unsigned Number = GraphTraits<const BlockT *>::getNumber(B);
- if (CycleT *Temp = Info.BlockMap[Number]) {
- CycleT *Flat = &Info.Cycles[Temp->IdxBegin];
+ unsigned Created = Info.BlockMap[Number];
+ if (Created != CycleInfoT::NoCycle) {
+ // Created indexes AllCycles; enter() stashed the flat preorder index in
+ // that temporary node's IdxBegin.
+ unsigned Flat = AllCycles[Created].IdxBegin;
Info.BlockMap[Number] = Flat;
- Info.BlockLayout[--Flat->IdxBegin] = B;
+ CycleT &FlatCycle = Info.Cycles[Flat];
+ Info.BlockLayout[--FlatCycle.IdxBegin] = B;
}
}
}
@@ -420,7 +438,7 @@ void GenericCycleInfoCompute<ContextT>::run(FunctionT *F) {
CycleT *NewCycle = &AllCycles.emplace_back();
NewCycle->IdxBegin = AttachedChildren.size(); // Attach-log slice start.
NewCycle->appendEntry(HeaderCandidate);
- Info.addToBlockMap(HeaderCandidate, NewCycle);
+ recordInnermostCycle(HeaderCandidate);
// The header is this cycle's first own block. Until flatten() runs,
// IdxEnd accumulates this cycle's own-block count (see the IdxBegin/
// IdxEnd doc comment), so flatten() needs no separate counting pass.
@@ -462,10 +480,13 @@ void GenericCycleInfoCompute<ContextT>::run(FunctionT *F) {
// If the block has already been discovered by some cycle
// (possibly by ourself), then the outermost cycle containing it
// should become our child. Walk the temporary forest directly:
- // handles are not meaningful until flatten() builds the flat array.
- CycleT *BlockParent = Info.getCyclePtr(Block);
- while (BlockParent && BlockParent->ParentCycle)
- BlockParent = BlockParent->ParentCycle;
+ // handles are not meaningful until flatten() builds the flat array, so
+ // BlockMap still holds creation-order indices into AllCycles.
+ unsigned Created = Info.BlockMap[GraphTraits<BlockT *>::getNumber(Block)];
+ CycleT *BlockParent =
+ Created == CycleInfoT::NoCycle ? nullptr : &AllCycles[Created];
+ while (BlockParent && BlockParent->hasParent())
+ BlockParent = &AllCycles[BlockParent->ParentIndex];
if (BlockParent) {
LLVM_DEBUG(errs() << " block " << Info.Context.print(Block) << ": ");
@@ -484,7 +505,7 @@ void GenericCycleInfoCompute<ContextT>::run(FunctionT *F) {
<< Info.Context.print(BlockParent->Entries[0]) << "\n");
}
} else {
- Info.addToBlockMap(Block, NewCycle);
+ recordInnermostCycle(Block);
++NewCycle->IdxEnd; // Block's innermost cycle is NewCycle.
ProcessPredecessors(Block);
}
@@ -576,7 +597,7 @@ void GenericCycleInfo<ContextT>::compute(FunctionT &F) {
GenericCycleInfoCompute<ContextT> Compute(*this);
Context = ContextT(&F);
BlockNumberEpoch = GraphTraits<FunctionT *>::getNumberEpoch(&F);
- BlockMap.resize(GraphTraits<FunctionT *>::getMaxNumber(&F));
+ BlockMap.assign(GraphTraits<FunctionT *>::getMaxNumber(&F), NoCycle);
LLVM_DEBUG(errs() << "Computing cycles for function: " << F.getName()
<< "\n");
diff --git a/llvm/include/llvm/ADT/GenericCycleInfo.h b/llvm/include/llvm/ADT/GenericCycleInfo.h
index d827f963b8d79..67c301dd1121c 100644
--- a/llvm/include/llvm/ADT/GenericCycleInfo.h
+++ b/llvm/include/llvm/ADT/GenericCycleInfo.h
@@ -80,12 +80,17 @@ template <typename ContextT> class GenericCycleInfo {
template <typename> friend class GenericCycleInfoCompute;
private:
+ /// Sentinel for a cycle-index slot that refers to no cycle.
+ static constexpr unsigned NoCycle = ~0u;
+
/// Internal, data-only storage for a cycle. Consumers name a cycle by a
/// CycleRef handle and query it through GenericCycleInfo.
class Cycle {
public:
- /// The parent cycle. Is null for top-level cycles.
- Cycle *ParentCycle = nullptr;
+ /// Preorder index of the parent cycle, or NoCycle for a top-level
+ /// cycle. Before flatten() this holds a creation-order index into
+ /// GenericCycleInfoCompute::AllCycles.
+ unsigned ParentIndex = NoCycle;
/// The entry block(s) of the cycle. The header is the only entry if this
/// is a loop.
@@ -112,6 +117,9 @@ template <typename ContextT> class GenericCycleInfo {
void appendEntry(BlockT *Block) { Entries.push_back(Block); }
+ /// Whether this cycle has a parent, i.e. is not top-level.
+ bool hasParent() const { return ParentIndex != NoCycle; }
+
Cycle() = default;
Cycle(const Cycle &) = delete;
Cycle &operator=(const Cycle &) = delete;
@@ -123,8 +131,12 @@ template <typename ContextT> class GenericCycleInfo {
ContextT Context;
unsigned BlockNumberEpoch;
- /// Map basic block numbers to their inner-most containing cycle.
- SmallVector<CycleT *> BlockMap;
+ /// Map each basic block number to the preorder index of its inner-most
+ /// containing cycle, or NoCycle if none. During construction this
+ /// transiently holds creation-order indices into
+ /// GenericCycleInfoCompute::AllCycles, which flatten() remaps to preorder
+ /// indices.
+ SmallVector<unsigned> BlockMap;
/// Euler tour of the cycle forest: every cycle's blocks form a contiguous
/// slice [IdxBegin, IdxEnd) of this array, nested inside its parent's.
@@ -156,15 +168,6 @@ template <typename ContextT> class GenericCycleInfo {
/// The handle for a stored cycle.
CycleRef ref(const CycleT &C) const { return CycleRef(getCycleIndex(C)); }
- /// The innermost cycle containing \p Block as a raw pointer, or null. Used
- /// internally and during construction, where handles are not yet meaningful
- /// because the flat array does not exist.
- CycleT *getCyclePtr(const BlockT *Block) const {
- verifyBlockNumberEpoch(Block->getParent());
- unsigned Number = GraphTraits<const BlockT *>::getNumber(Block);
- return Number < BlockMap.size() ? BlockMap[Number] : nullptr;
- }
-
void verifyBlockNumberEpoch(const FunctionT *Fn) const {
assert(BlockNumberEpoch ==
GraphTraits<const FunctionT *>::getNumberEpoch(Fn) &&
@@ -218,15 +221,20 @@ template <typename ContextT> class GenericCycleInfo {
/// \returns the innermost cycle containing \p Block or an invalid handle if
/// it is not contained in any cycle.
CycleRef getCycle(const BlockT *Block) const {
- CycleT *C = getCyclePtr(Block);
- return C ? ref(*C) : CycleRef();
+ verifyBlockNumberEpoch(Block->getParent());
+ unsigned Number = GraphTraits<const BlockT *>::getNumber(Block);
+ // A block added after compute() that no cycle contains (e.g. a critical
+ // edge MachineSink split outside every cycle) has a number beyond BlockMap.
+ if (Number >= BlockMap.size() || BlockMap[Number] == NoCycle)
+ return CycleRef();
+ return CycleRef(BlockMap[Number]);
}
BlockT *getHeader(CycleRef C) const { return deref(C).Entries[0]; }
bool isReducible(CycleRef C) const { return deref(C).Entries.size() == 1; }
CycleRef getParentCycle(CycleRef C) const {
- CycleT *P = deref(C).ParentCycle;
- return P ? ref(*P) : CycleRef();
+ auto P = deref(C).ParentIndex;
+ return P == NoCycle ? CycleRef() : CycleRef(P);
}
unsigned getDepth(CycleRef C) const { return deref(C).Depth; }
size_t getNumBlocks(CycleRef C) const {
@@ -287,12 +295,12 @@ template <typename ContextT> class GenericCycleInfo {
}
CycleRef getTopLevelParentCycle(const BlockT *Block) const {
- CycleT *C = getCyclePtr(Block);
+ CycleRef C = getCycle(Block);
if (!C)
- return CycleRef();
- while (C->ParentCycle)
- C = C->ParentCycle;
- return ref(*C);
+ return C;
+ while (CycleRef P = getParentCycle(C))
+ C = P;
+ return C;
}
/// Return all of the successor blocks of \p C: the blocks outside of \p C
``````````
</details>
https://github.com/llvm/llvm-project/pull/210271
More information about the llvm-commits
mailing list