[PATCH] D18077: Factor out MachineBlockPlacement::fillWorkLists. NFC

Chad Rosier via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 11 13:18:44 PST 2016


mcrosier added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:598
@@ +597,3 @@
+
+  if (Chain.UnscheduledPredecessors != 0)
+    return;
----------------
deadalnix wrote:
> mcrosier wrote:
> > Was this style suggested in D17625?
> D17625 introduce 2 worklist, so you need to add code there to choose which worklist you add the MBB to.
Honestly, I don't follow your argument.

I prefer this style (i.e., the original code):

  if (Chain.UnscheduledPredecessors == 0)
    BlockWorkList.push_back(*Chain.begin());

rather than your change (below):

  if (Chain.UnscheduledPredecessors != 0)
    return;
  BlockWorkList.push_back(*Chain.begin());

For this patch can we just stick with the original code?


http://reviews.llvm.org/D18077





More information about the llvm-commits mailing list