[PATCH] D18226: Codegen: Tail-duplicate during placement.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/BranchFolding.cpp:608
@@ -601,1 +607,3 @@
+                  DenseMap<const MachineBasicBlock *, int> &FuncletMembership,
+                  bool AfterPlacement) {
   // It is never profitable to tail-merge blocks from two different funclets.
----------------
I think it is cleaner to check at the beginning if anyone of MBB1 or MBB2 need to be skipped (either be the taildupped block or a block tail-dupped into), and return false if so. The set of skipped blocks can be passed into the BF by block placement.

================
Comment at: lib/CodeGen/BranchFolding.cpp:629
@@ +628,3 @@
+  // unless the other block has its own fallthrough.
+  if ((MBB1 == PredBB && !MBB2->canFallThrough())
+       || (MBB2 == PredBB && !MBB1->canFallThrough())) {
----------------
Is this change related?

================
Comment at: lib/CodeGen/TailDuplicator.cpp:519
@@ +518,3 @@
+  // If the block to be duplicated ends in an unanalyzable fallthrough, don't
+  // duplicate it.
+  MachineBasicBlock *PredTBB = nullptr, *PredFBB = nullptr;
----------------
Expand the comments a little more.

================
Comment at: lib/CodeGen/TailDuplicator.cpp:835
@@ -811,1 +834,3 @@
+  // block into the prior block. Don't do this when LayoutMode is
+  // true, as there is no point in removing the block during layout.
   MachineBasicBlock *PrevBB = &*std::prev(TailBB->getIterator());
----------------
nit: block placement does not expect any blocks to be deleted.


http://reviews.llvm.org/D18226