[PATCH] D81584: [BreakCritEdges] Support preserving loop-simplify form with indirectbr. (WIP)

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D81584#2086163 <https://reviews.llvm.org/D81584#2086163>, @efriedma wrote:

> > Agreed. I think that would go well with D81582 <https://reviews.llvm.org/D81582>, i.e. have a PreserveLoopSimplify option, that guarantees loop-simplify form is preserved or nullptr is returned if it's not possible
>
> We can kill off the call to SplitBlockPredecessors, and add an option to bail out if preserving loop-simplify form would require transforming other edges, sure.  (I'd rather not condition it on whether the other edges are IndirectBr edges; I'd like to avoid obscure IndirectBr edge cases where possible.)


Ok. In the short term, I'd like the fix the crash in GVN asap. Preserving simplify-form as in this patch will require a bit of extra work in a couple of places. I think in the short-term, it might be best to add a `PreserveLoopSimplify` option and have GVN not preserve loop-simplify form, until this patch is ready. What do you think?

>> Also, do you agree that preserving loop-simplify form by moving instructions to a different BB is more trouble than it is worth?
> 
> This doesn't seem worse than calling SplitBlockPredecessors(), in terms of transforming the CFG in unpredictable ways.

Agreed, unfortunately the existing users don't deal too gratefully with moving instructions, but I don't think anything in the SplitCriticalEdges interface guarantees that. If you are not concerned about the general approach and the small hoops that existing users have to jump through (see GVN & LSR changes), I am happy to continue with this approach, but it will probably require a few additional changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81584