[PATCH] D23191: [BranchFolding] Restrict tail merging loop blocks after machine block placement
Kyle Butt via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 10 17:29:13 PDT 2016
iteratee added a comment.
Mostly I'm fine with this Tidy up the comments and the test.
================
Comment at: lib/CodeGen/BranchFolding.cpp:1001
@@ +1000,3 @@
+
+ // Bail out if IBB is the loop header after the block placement because
+ // -- If merging predecessors that belong to the same loop as IBB, the
----------------
This is tricky to parse. It would be clearer as:
Bail if merging after placement and IBB is the loop header.
Whether IBB is the loop header or not shouldn't be changed by placement.
================
Comment at: lib/CodeGen/BranchFolding.cpp:1006
@@ +1005,3 @@
+ // tail and require more branches. This can be relaxed if
+ // MachineBlockPlacement::findBestLoopTop is more flexable.
+ // --If merging predecessors that do not belong to the same loop as IBB, the
----------------
nit: flexible.
================
Comment at: lib/CodeGen/BranchFolding.cpp:1010
@@ +1009,3 @@
+ // block placement again may make big change to the layout and eliminate the
+ // reasone to do tail merging here.
+ if (AfterBlockPlacement && MLI) {
----------------
reasone
================
Comment at: test/CodeGen/X86/tail-merge-after-mbp.ll:13
@@ +12,3 @@
+; check loop block BB#9 is not merged with BB#5, BB#7
+define i32 @foo(%0* nocapture readonly, i32) local_unnamed_addr {
+; CHECK-LABEL: foo:
----------------
I'm not a fan of undef in tests. They have a tendency to be brittle when the optimizer decides to pick a value for you as it is allowed to do. Please take arguments or call a function for the values.
Repository:
rL LLVM
https://reviews.llvm.org/D23191
More information about the llvm-commits
mailing list