[PATCH] D17625: Do not select EhPad BB in MachineBlockPlacement when there is regular BB to schedule

[Amaury SECHET via llvm-commits]

deadalnix added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:545
@@ -543,1 +544,3 @@
 
+    // We prefer non ehpad over ehpads.
+    bool IsCandidateEHPad = MBB->isEHPad();
----------------
chandlerc wrote:
> davidxl wrote:
> > The new code really lowers the readability. Can you describe the algorithm here? Is it
> > 1) if the best non eh block exists, return it; otherwise
> > 2) return the least likely eh pad?
> > 
> > if that is the case, why not simply skip eh blocks in the original worklist loop. If the best bb is found return it, otherwise have another helper loop to find the least likely eh pad?
> I agree that this makes it essentially unreadable. =/
> 
> I'd suggest instead to maintain two worklists, one of non-EH-pads, and another of EH-pads. Then you can write the direct loop over each, and you can even put the algorithm David suggested in a comment on the second loop. =]
> 
> Note that looping over the worklist twice could be quite expensive, which is why I'd suggest two lists.
@davisxl : Yes, that is exactly what this is doing. Going over the ehpad starting with the least likely makes them flow into each other more naturally. Consider you have something like:

  hotehpad:
    br exit  // fallthrought
  exit:
    ret
  coldehpad:
    br exit

vs

  coldehpad:
    br exit
  hotehpad:
    br exit  // fallthrought
  exit:
    ret

@chandlerc , ok for the 2 worklists.


http://reviews.llvm.org/D17625