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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1849
@@ +1848,3 @@
+    DupBB = *(--ChainEnd);
+    // Now try to duplicate again.
+    if (ChainEnd == Chain.begin())
----------------
This one needs a test case to cover.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1910
@@ +1909,3 @@
+                << getBlockName(Pred) << " -> "
+                << getBlockName(BB) << " -> "
+                << getBlockName(NewSucc) << "\n");
----------------
iteratee wrote:
> davidxl wrote:
> > There are two problems here
> > 1) many Pred (other than LPred) has been skipped as illegal candidates, but tailDupiicator does not know about will happily do tailDup -- as the tailDuplicator does not know about the new constraints from MBP
> > 2) if the total number of candidate predecessors that BB can tailDuped into is fewer than 2, there is no point do the tail duplication
> I think you misunderstand the point of this loop. The point is not to find "legal candidates", but to find the candidates for which duplicating will require update.
> 
> Because of that, we don't have an accurate count, and don't want one. We'll gladly let tailDuplicator do its job.
Ok.

Do you see the possibility that some Pred simply can not be selected as dup target bb (for legality or performance reasons)? Overall, the code should be organizied in the way that the list of BBs that MBP thinks it can/should be tailDup should match the actual transformations.  




https://reviews.llvm.org/D18226