[PATCH] D28522: Codegen: Make chains from lattice-shaped CFGs

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:760
+    BlockFrequency DupCost = Qout + QinU + P * VProb;
+    return (BaseCost > DupCost);
     return greaterWithBias(BaseCost, DupCost, EntryFreq);
----------------
merge error here -- two returns


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:833
+      // It's not a lattice if the same block is on both sides.
+      if (Successors.count(SuccPred))
+        return false;
----------------
tail duplication can create this pattern - why is it skipped?
`

```
 `BB    Pred
  |  \      /|
  |    \  /  |
  |      /\  |
  |   /    S2  
  | /      /
S1  <-+`
```


`


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:836
+      ++PredCount;
+      if (SeenPreds.count(SuccPred))
+        continue;
----------------
merge this with the insert before?


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:850
+/// Get the best successor from \p BB based on \p BB being part of a lattice.
+MachineBasicBlock* MachineBlockPlacement::getBestLatticeSuccessor(
+    MachineBasicBlock *BB,
----------------
Have a high level description of the selection algo here as comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1220
       continue;
+    BlockFrequency PredEdgeFreq =
+        MBFI->getBlockFreq(Pred) * MBPI->getEdgeProbability(Pred, Succ);
----------------
unrelated change?


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1314
     BlockChain &SuccChain = *BlockToChain[Succ];
+
     // Skip the edge \c BB->Succ if block \c Succ has a better layout
----------------
unrelated change?


Repository:
  rL LLVM

https://reviews.llvm.org/D28522