[CodeLayout] CDSortImpl: remove HotChains and remove linear-time erase_value from mergeChains

[MaskRay]

After mergeChainPairs initializes a priority queue, HotChains is unused
except a HotChains.size() use in LLVM_DEBUG. Optimize it out.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Fangrui Song (MaskRay)

Changes

After mergeChainPairs initializes a priority queue, HotChains is unused
except a HotChains.size() use in LLVM_DEBUG. Replace it with a variable.

---

Full diff: https://github.com/llvm/llvm-project/pull/69276.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Utils/CodeLayout.cpp (+4-4)

diff --git a/llvm/lib/Transforms/Utils/CodeLayout.cpp b/llvm/lib/Transforms/Utils/CodeLayout.cpp
index d9e302d8b4fa54d..d2f39b0284c6799 100644
--- a/llvm/lib/Transforms/Utils/CodeLayout.cpp
+++ b/llvm/lib/Transforms/Utils/CodeLayout.cpp
@@ -1027,7 +1027,7 @@ class CDSortImpl {
 
     LLVM_DEBUG(dbgs() << "Cache-directed function sorting reduced the number"
                       << " of chains from " << NumNodes << " to "
-                      << HotChains.size() << "\n");
+                      << NumHotChains << "\n");
 
     // Collect nodes from all the chains.
     return concatChains();
@@ -1085,6 +1085,7 @@ class CDSortImpl {
       if (Node.ExecutionCount > 0)
         HotChains.push_back(&AllChains.back());
     }
+    NumHotChains = HotChains.size();
 
     // Initialize chain edges.
     AllEdges.reserve(AllJumps.size());
@@ -1152,6 +1153,7 @@ class CDSortImpl {
       MergeGainT BestGain = BestEdge->getMergeGain();
       mergeChains(BestSrcChain, BestDstChain, BestGain.mergeOffset(),
                   BestGain.mergeType());
+      --NumHotChains;
 
       // Insert newly created edges into the queue.
       for (const auto &[_, Edge] : BestSrcChain->Edges) {
@@ -1301,9 +1303,6 @@ class CDSortImpl {
     // Merge the edges.
     Into->mergeEdges(From);
     From->clear();
-
-    // Remove the chain from the list of active chains.
-    llvm::erase_value(HotChains, From);
   }
 
   /// Concatenate all chains into the final order.
@@ -1372,6 +1371,7 @@ class CDSortImpl {
 
   /// Active chains. The vector gets updated at runtime when chains are merged.
   std::vector<ChainT *> HotChains;
+  size_t NumHotChains = 0;
 
   /// The total number of samples in the graph.
   uint64_t TotalSamples{0};

[spupyrev]

Seems like the list, HotChains, is not used in the algorithm. Can we get rid of it altogether?

Instead of for (ChainT *ChainPred : HotChains) {... we'd have something like

for (NodeT &Node : AllNodes) {
  if (Node.ExecutionCount > 0) {
    ChainT *ChainPred = Node.CurChain;
    ...
  }
}

[MaskRay]

Thanks for the suggestion. I'll optimize out HotChains and rename NumHotChains to NumActiveChains.

[spupyrev]

One can move the debug statement to concatChains() where we have access to the number of remaining chains, SortedChains.size(). So this variable, NumHotChains, won't be needed

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[MaskRay]

Thanks for the review!

This is not a bottleneck.

  Time (mean ± σ):     534.928 s ±  1.463 s    [User: 545.663 s, System: 2.177 s]
  Range (min … max):   533.894 s … 535.963 s    2 runs

Benchmark 2: numactl -C 20-27 /t/lld1 -flavor gnu @response.txt --call-graph-profile-sort=cdsort --threads=8
  Time (mean ± σ):     535.081 s ±  0.563 s    [User: 545.860 s, System: 2.193 s]
  Range (min … max):   534.683 s … 535.479 s    2 runs

Summary
  'numactl -C 20-27 /t/lld0 -flavor gnu @response.txt --call-graph-profile-sort=cdsort --threads=8' ran
    1.00 ± 0.00 times faster than 'numactl -C 20-27 /t/lld1 -flavor gnu @response.txt --call-graph-profile-sort=cdsort --threads=8'