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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added a comment.

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

> We could make preserving LoopSimplify form optional, and let users turn it on with CriticalEdgeSplittingOptions.  That way, if some user isn't prepared to handle the transform, they can just let LoopSimplify form break, and fix it up later if necessary.  We probably should do that anyway: there isn't any obvious connection between preserving LoopInfo and preserving LoopSimplify form.


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. If  `PreserveLoopSimplify` is false, don't preserve it if it is not possible .  But currently GVN for example preserves loop-simplify form, but I suppose we could change that. Might be best as a follow-on of the bail out if preserving is not possible. 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?



================
Comment at: llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp:324
+          DT->applyUpdates({{DominatorTree::Insert, NewBB, ExitCont},
+                            {DominatorTree::Delete, NewBB, DestBB}});
         }
----------------
efriedma wrote:
> Missing LoopInfo update?
I don't think we need to do anything about LI here, as the block should already be added to LI by `SplitBlock` and we only adjust the successor to another successor in the same loop I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81584