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

[David Li via llvm-commits]

davidxl added a comment.

Sorry for the delay -- for some reason I did not receive the email notification at all..


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:420
@@ -405,3 +419,3 @@
     bool SkipSucc = false;
-    if (BlockFilter && !BlockFilter->count(Succ)) {
+    if (Succ->isEHPad() || (BlockFilter && !BlockFilter->count(Succ))) {
       SkipSucc = true;
----------------
deadalnix wrote:
> davidxl wrote:
> > Why is it always good to skip ehpad? With real profile data, can ehpad be 'hotter' than some of the cold blocks?
> There is no point in eagerly scheduling ehpadsas they are never reached the "conventional" way. When you reach an EHPad, you've gone throught the personality function and a bunch of other runtime code, which makes it pretty much irrelevant where it is. Even if the EHPad ends up hotter than some other BB, the way you get to the EHPad makes it irrelevant if it is scheduled eagerly.
> 
> There is also considerations linked to D17555 , as to make sure all EHPads ends up on the same side of the split, which simplify dramatically LSDA generation.
> 
This makes total sense.

================
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;
----------------
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.


http://reviews.llvm.org/D17625