[PATCH] D20505: Codegen: Outline for chains of tail-duplicable blocks.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:308
@@ +307,3 @@
+  /// 2 blocks.
+  SmallPtrSet<MachineBasicBlock *, 8> TailDupDelayBlocks;
+
----------------
This document does not help understand the meaning. Can add a reference to detailed description of the algoirthm in other place (e.g. function definition).

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:385
@@ +384,3 @@
+  void computeUnavoidableBlocks();
+  bool canTailDuplicateAllPreds(MachineBasicBlock *BB, MachineBasicBlock *Succ,
+                                BlockChain &Chain,
----------------
Brief documentation.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:388
@@ +387,3 @@
+                                const BlockFilterSet *BlockFilter);
+  void delayTailDuplicatedBlocks(MachineBasicBlock *BB, MachineBasicBlock *Succ,
+                                 BlockChain &Chain,
----------------
Same here.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1030
@@ +1029,3 @@
+    // Check for that now.
+    if (TailDupPlacement && BestSucc) {
+      DEBUG(dbgs() << "Redoing tail duplication for BestSucc#"
----------------
Please outline this big part into its own method. 

================
Comment at: lib/CodeGen/TailDuplicator.cpp:787
@@ -768,7 +786,3 @@
 
-    MachineBasicBlock *PredTBB, *PredFBB;
-    SmallVector<MachineOperand, 4> PredCond;
-    if (TII->AnalyzeBranch(*PredBB, PredTBB, PredFBB, PredCond, true))
-      continue;
-    if (!PredCond.empty())
+    if (!canTailDuplicate(TailBB, PredBB))
       continue;
----------------
Split out the refactor change.

================
Comment at: lib/CodeGen/TailDuplicator.cpp:872
@@ -853,2 +871,3 @@
   // AnalyzeBranch.
-  if (ForcedLayoutPred == nullptr && PrevBB->succ_size() == 1 &&
+  if (PrevBB->succ_size() == 1 &&
+      *PrevBB->succ_begin() == TailBB &&
----------------
Split out the clean-up changes.

================
Comment at: test/CodeGen/PowerPC/tail-dup-layout.ll:1
@@ -1,2 +2,1 @@
-; RUN: llc -outline-optional-branches -O2 < %s | FileCheck %s
 target datalayout = "e-m:e-i64:64-n32:64"
----------------
This test case can use some simplifications. Why not just do simple function call in optional branches?  The test block can also be simplified for instance testing input parameters.


http://reviews.llvm.org/D20505