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

[David Li via llvm-commits]

davidxl added a subscriber: davidxl.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:598
@@ +597,3 @@
+
+  if (Chain.UnscheduledPredecessors != 0)
+    return;
----------------
deadalnix wrote:
> mcrosier wrote:
> > 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?
> It can. I just would have to change this in D17625 instead of here, which really I don't care. I was asked to extract NFC changes in this patch, so that made sense to include this.
I agree with Chad here -- unless it helps reducing if nesting level, early return here does not help readability -- the original code seems more readable:

if (Chain.UnscheduledPredecessors == 0) {
     // update worklist(s)
 }
return;


http://reviews.llvm.org/D18077