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

[Kyle Butt via llvm-commits]

iteratee added inline comments.

================
Comment at: lib/CodeGen/BranchFolding.cpp:648
@@ +647,3 @@
+  // duplicated.
+  if (!AfterPlacement)
+    if (SuccBB && MBB1 != PredBB && MBB2 != PredBB &&
----------------
davidxl wrote:
> Is there a better way to identify Pred BBs which are tailduplicated into during block placement? Looks to me we should avoid undoing any taildups done in layout stage.
While we could pass a set of blocks around, the thresholds don't overlap by default. e.g. We tail-duplicate blocks of 2 instructions or fewer, and tail-merge blocks of 3 instructions or more. The problem is with fractional overlap. e.g. a block of 2 instructions was duplicated and in one of the resulting block an additional branch had to be added.

Would you prefer that I create a set of blocks and pass them into the branch optimizer? It would make it faster to skip those blocks altogether.

================
Comment at: lib/CodeGen/TailDuplicator.cpp:512
@@ -509,3 +511,3 @@
 /// Determine if it is profitable to duplicate this block.
 bool TailDuplicator::shouldTailDuplicate(const MachineFunction &MF,
                                          bool IsSimple,
----------------
davidxl wrote:
> Document new parameters. While you are here, document IsSimple as well.
New parameter removed.

================
Comment at: lib/CodeGen/TailDuplicator.cpp:847
@@ -827,2 +846,3 @@
   // block, which falls through unconditionally, move the contents of this
-  // block into the prior block.
+  // block into the prior block. Don't do this when ForcedLayoutPred is
+  // non-null, as it can break layout to remove blocks.
----------------
davidxl wrote:
> Are there other places in taildup where blocks may get removed? It seems to me that tailDupllicator needs to have member to remember it is in 'layout' state, and the block deletor code should check that flag instead.
No, nowhere else that blocks may get removed, and ultimately I want to not have different modes.

I decided that for duplication during layout, it doesn't make sense to try to delete the block. I've updated the code accordingly, and will fix D20505 to match.


http://reviews.llvm.org/D18226