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

David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 10 23:11:50 PST 2016


davidxl added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:563
@@ -540,1 +562,3 @@
+    // from least probable landingpads to more probable ones.
+    if (BestBlock && (IsEHPad ^ (BestFreq >= CandidateFreq)))
       continue;
----------------
deadalnix wrote:
> davidxl wrote:
> > Since there is no fundamental difference between 
> > entry, hot, then, cold, hlp, lpret, clp vs
> > entry, hot, then, cold, clp, hlp, lpret
> > 
> > I think change at line 563 should be removed -- it is a distraction to the main code IMO.  Some of the test changes can also be avoided.
> It can be done if you feel strongly about it. But please consider the following. Quite often, ehpad code flow into each others. See pseudocode :
> 
>     invoke @foo to %then unwind label %outerlp
>   then:
>     invoke @foo to %exit unwind label %innerlp
>   exit:
>     ret void
>   innerlp:
>     landingpad { i8*, i32 } cleanup
>     br %innercleanup
>   innercleanup:
>     ; Do inner cleanup
>     br %outercleanup
>   outerlp:
>     landingpad { i8*, i32 } cleanup
>     br %outercleanup
>   outercleanup:
>     ; Do outer cleanup
>     resume
> 
> Note that this is typical RAII kind of code and/or nested try, or any combiantion of these. Long story short, this kind of pattern are very common.
> 
> Now, if you compute the probabilities, outerlp is consider more probable than innerlp, as the preceding invoke is itself more probable. That means that unwinding code ends up being scheduled completely backward every time. Here you'd get :
> 
>   outerlp, outercleanup, innerlp, innercleanup // which branch to outercleanup
> 
> This is the most simple case, but it if very common to have more level of nesting than that in the unwind path and it gets worse. You end with a "corkscrew" like basic block scheduling which I don't think is good.
This is a  convincing example that shows benefit of using this layout for lp. I suggest adding a small test case to cover this (i.e, inner scope ehpad 'flows' into outer ehpads in the final layout).

By the way, I think the fillworklist change can be separated out as a NFC refactoring.


http://reviews.llvm.org/D17625





More information about the llvm-commits mailing list