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

[Eric Christopher via Phabricator via llvm-commits]

echristo added a comment.

A few comments if you wouldn't mind?



================
Comment at: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp:570
+/// \p BB Block to check.
+bool MachineBlockPlacement::shouldTailDuplicate(MachineBasicBlock *BB) {
+  // Blocks with single successors don't create additional fallthrough
----------------
const arguments?


================
Comment at: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp:589
+/// tail-duplication and lay them out in the same manner.
+bool MachineBlockPlacement::canTailDuplicateUnplacedPreds(
+    MachineBasicBlock *BB, MachineBasicBlock *Succ, BlockChain &Chain,
----------------
Ditto.


================
Comment at: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp:595
+
+  for (MachineBasicBlock *Pred : Succ->predecessors()) {
+    // Make sure all unplaced and unfiltered predecessors can be
----------------
Ditto.


================
Comment at: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp:598
+    // tail-duplicated into.
+    if (Pred == BB || (BlockFilter && !BlockFilter->count(Pred))
+        || BlockToChain[Pred] == &Chain)
----------------
Can you comment/explain the BlockFilter part here?


================
Comment at: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp:599
+    if (Pred == BB || (BlockFilter && !BlockFilter->count(Pred))
+        || BlockToChain[Pred] == &Chain)
+      continue;
----------------
Same with BlockToChain.


================
Comment at: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp:1961
+
+  if (!shouldTailDuplicate(BB))
     return false;
----------------
Maybe this should be above the previous line?


Repository:
  rL LLVM

https://reviews.llvm.org/D27742