[PATCH] D89818: [Compile Time] Make it possible to skip redundant debuginst removal

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

As I see it this solution focuses on the problem seen in LoopUnroll. So I agree, it is conservative and simple so if it solves the PR I think it is just fine.

Comparing with the D88928 <https://reviews.llvm.org/D88928> patch (in its current state), that patch removes the RemoveRedundantDbgInstrs call from MergeBlockIntoPredecessor, but only replaces that with extra calls to RemoveRedundantDbgInstrs in a few places where MergeBlockIntoPredecessor is used (so afaict that patch currently removes a lot of DbgInstr cleanups?). IMO this patch makes it more of an active choice to decide if the removal can be deferred or not. With D88928 <https://reviews.llvm.org/D88928> it is easier to forget about such cleanups.

An alternative could be to mix the solutions a bit. We could have a version of MergeBlockIntoPredecessor that is doing the cleanup automatically, and one that skips the cleanup but instead returns the predecessor BB when a merge was made. The latter could be used in usecases such as the one in CodeGenPrepare, when we want to collect all the pred blocks in a set and defer calling of RemoveRedundantDbgInstrs for each of those blocks until after the inner loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89818