[PATCH] D81089: Mark InstCombine as not preserving CFG

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> As the pipeline diffs show, not preserving CFG has a very real cost (five new domtree calculations at least). We should try to avoid that :)



> You can always preserve domtrees by marking dfsnumbers as invalid.

I wasn't trying to imply that instcombine couldn't keep the domtree up-to-date; we can easily do that even if instcombine says it doesn't preserve the CFG.

> The issue I see with introducing fake exits is that postdominators is an analysis used by other analyses, and it would be probably complicated to schedule a transformation in front of any analysis that uses postdominators. Even if an analysis doesn't need postdominance info for reverse-unreachable code, it would see a postdominators analysis results for reverse-reachable nodes if a pass running before it introduced them. Note that this would affect all IR-like things that use postdominators, e.g., llvm ir, clang cfg, machine ir, callgraphs, and third-party ir, including irs outside of the monorepo.

Yes, this is messy.  Maybe it's just too terrible.  I think we could come up with some sort of transition plan if necessary; for things that don't need to update the CFG, the downsides of the heuristics are probably less terrible.

In terms of keeping the existing post-dominators working, I guess the "mark the dfsnumbers invalid" API and domtree updates can also recompute the postdominator roots.  Not sure how we make that fast, though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81089/new/

https://reviews.llvm.org/D81089