[PATCH] D50658: Hot cold splitting pass

[Sebastian Pop via Phabricator via llvm-commits]

sebpop added inline comments.


================
Comment at: lib/Transforms/IPO/HotColdSplitting.cpp:166
+    if (!Visited.count(&BB))
+      AllColdBlocks.insert(&BB);
+  // TODO: Backward propagation to construct larger cold region
----------------
As AllColdBlocks is the complement of Visited, you could return Visited and avoid this loop.
Then you would ask the negative: a block is cold if it is not in Visited.


================
Comment at: lib/Transforms/IPO/HotColdSplitting.cpp:168
+  // TODO: Backward propagation to construct larger cold region
+  // as a union of smaller cold regions. e.g., an if-block branches out to
+  // two cold regions then the if block is also cold. CHI nodes to track
----------------
I think this TODO should be done in the code generation where you outline cold basic blocks: see below in outlineColdBlocks ...


================
Comment at: lib/Transforms/IPO/HotColdSplitting.cpp:300
+        // Candidate for outlining. FIXME: Continue outlining.
+        return extractColdRegion(Region, DT, BFI, ORE);
+      }
----------------
... implement the expansion to larger cold regions here.
Again, you can extend the region as long as SingleExit is cold: then you ask for another exit block that post-dominates Exit, etc. recursively.
Like so:
```
    if (PSI->isColdBB(BB, BFI) || ColdBlock.count(BB)) {
      SmallVector<BasicBlock *, 4> Region;
      BasicBlock *ExitColdRegion = nullptr;
      BasicBlock *Exit = (*PDT)[BB]->getIDom()->getBlock();
      // Estimate cold region between a BB and its dom-frontier.
      while (isSingleEntrySingleExit(BB, Exit, DT, PDT, Region) &&
                 isOutlineCandidate(Region, Exit)) {
        ExitColdRegion = Exit;
        // Update Exit recursively to its dom-frontier.
        Exit = (*PDT)[Exit]->getIDom()->getBlock();
        Region.clear();
      }
      if (ExitColdRegion) {
        ++NumColdSESEFound;
        Region.clear();
        isSingleEntrySingleExit(BB, ExitColdRegion, DT, PDT, Region);
        return extractColdRegion(Region, DT, BFI, ORE);
      }
   }
```


https://reviews.llvm.org/D50658