[PATCH] D28583: CodeGen: Allow small copyable blocks to "break" the CFG.

[Kyle Butt via Phabricator via llvm-commits]

iteratee added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1086
+  if (DupCandidates.size() != 0)
+    std::make_heap(DupCandidates.begin(), DupCandidates.end());
+  while (DupCandidates.size() != 0) {
----------------
davidxl wrote:
> Why not just stable sort it? The vector should be of size 1 for most of the cases.  Also why do you need position ?
Will just sort the vector.

Position is because we rely on the successor order being stable and the first successor being a subtle hint. Without the position, we lose track of whether the block in the vector came before or after the block we picked without tail duplication.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1098
+    if (canTailDuplicateUnplacedPreds(BB, Succ, Chain, BlockFilter)
+        && isProfitableToTailDup(BB, Succ, AdjustedSumProb, Chain,
+                                     BlockFilter)) {
----------------
davidxl wrote:
> isProfitableToTailDup assumes the baseline layout does not pick Succ.  The assumption may not be true here as there are other two possibilities:
> 1) Succ == BestSucc.BB in the base layout
> 2) BestSucc.BB == null in the base layout (all BB's successors have conflicts).
> 
> In such two cases, isProfitable check should probably be skipped (as it is benefitial)
1.
Succ won't equal BestSucc.BB because of the continue. These blocks were not chosen by the first loop by construction.

2.
Good catch. I'll add that.


https://reviews.llvm.org/D28583