[PATCH] D31032: [LoadCombine] Avoid analysing dead basic blocks

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.



In https://reviews.llvm.org/D31032#703353, @zzheng wrote:

> Can we remove such dead BBs before entering passes that would spin forever on them?


You don't want to :)



================
Comment at: lib/IR/LegacyPassManager.cpp:1283
 
+  // Scan the functions basic blocks in Reverse Post Order. This excludes dead
+  // basic blocks (it has been seen that the analysis in some passes could hang
----------------
1. You need to define dead.
If you mean "forward unreachable", please use that.

2. RPOT is not the optimal order for some problems (in fact, for some, it's optimally bad), and forcing that order seems ... concerning.
I'm pretty strongly against forcing a bb order in a generic pass infrastructure.

3. It looks like we have 3 or 4 basic block passes.

Are they getting anything from being basicblockpasses?


It looks like they could all be trivially changed to functionpasses, and those that can't handle dead code, could just be changed to do what they need to.

For example, DCE has a dominator tree around, it can tell whether things are forward unreachable without wasting time doing a reverse postorder traversal.

IMHO, that is a better solution (and helps new PM) than forcing a pass order here.


https://reviews.llvm.org/D31032