[PATCH] D17625: Do not select EhPad BB in MachineBlockPlacement when there is regular BB to schedule
Amaury SECHET via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 10 22:15:25 PST 2016
deadalnix 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;
----------------
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.
http://reviews.llvm.org/D17625
More information about the llvm-commits
mailing list