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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/BranchFolding.cpp:709
@@ +708,3 @@
+        if (Block1.empty())
+          // On some platforms return instructions are marked as indirect
+          // branches and not as return instructions. Work around this by
----------------
How abut Block1.back().isIndirectBranch() && !Block1.back().isReturn() ?

================
Comment at: lib/CodeGen/BranchFolding.cpp:713
@@ +712,3 @@
+          if (Block1.back().isIndirectBranch() && Block1.succ_size() != 0)
+            MinCommonTailLengthInside = 21;
+      }
----------------
I think this value should be passed in either via constructor or a separate API:

setMergeThresholdForBBwithIndirectBr(...).

In tailDup, the value 20 should also be controlled by an option.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1015
@@ +1014,3 @@
+                              Removed, DuplicatedToBB);
+      if (Removed) {
+        if (DuplicatedToBB) {
----------------
Why can't this part be folded into maybeTailDuplicateBlock? If not, the big chunk of code  between if (Removed) { ..} belongs to its own method.

================
Comment at: lib/CodeGen/TailDuplicator.cpp:572
@@ +571,3 @@
+    // the CFG.
+    HasIndirectbr = back.isIndirectBranch() && TailBB.succ_size() != 0;
+  }
----------------
I don't get it. This one looks like a heuristic change to not give block with return instruction (as indirect branch) a larger threshold for duplication, which is independent of this patch.


https://reviews.llvm.org/D18226