[PATCH] D27742: 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:568
+
+  if (!IsSimple && BB->succ_size() == 1)
+    return false;
----------------
davidxl wrote:
> && --> || ?
> 
> In other words, do we care about block dup of simple blocks in layout mode?
Simple blocks contain only an unconditional jump. I don't see a good reason to exclude them during layout.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:581
+/// tail-duplication and lay them out in the same manner.
+bool MachineBlockPlacement::canTailDuplicateAllPreds(
+    MachineBasicBlock *BB, MachineBasicBlock *Succ, BlockChain &Chain,
----------------
davidxl wrote:
> Perhaps change the name to "canTailDuplicateUnplacedPreds" ?
Done. Thanks.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:593
+      continue;
+    if (!TailDup.canTailDuplicate(Succ, Pred))
+      return false;
----------------
davidxl wrote:
> We have have a situation where some of the predecessors can share a common successor (newly created), and the block can be tail-Duped into that block.  It might be worth considering supporting.
While I'll keep that in mind, that would basically be block copying. We should support that as well, but I think it's out of scope for what I'm trying to do here.


https://reviews.llvm.org/D27742