[PATCH] D88928: [Utils] Skip RemoveRedundantDbgInstrs in MergeBlockIntoPredecessor (PR47746)

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D88928#2316194 <https://reviews.llvm.org/D88928#2316194>, @fhahn wrote:

>> So: skip the call to RemoveRedundantDbgInstrs. There's surprisingly little fallout from this, and most of it can be addressed by doing RemoveRedundantDbgInstrs later.
>
> As an alternative, would it make sense to have a dedicated pass to eliminate redundant debug instructions, that's run late in the pipeline? not sure if running it once, but for every function will be better or worse in the end. It should be easy to skip for functions without debug info though.

I don't remember exactly, but isn't one goal with removing the redundant dbg instructions early to speed up compilation (when -g is used). It's like saying that we shouldn't remove dead code until late in the pipeline to me (spending time on optimizing the dead code all the way through opt).

Looks like it has been unfortunate to do the RemoveRedundantDbgInstrs inside MergeBlockIntoPredecessor for the CodeGenPrepare use-case, as it can end up merging blocks in several steps (resulting in bad complexity). So for example having an option to MergeBlockIntoPredecessor (or a wrapper) to be able to postpone the cleanup to afterwards in such use-cases seems like a good idea to me.
There actually is a pass that only is used in lit tests (afaik) that is doing this kind of cleanup (RedundantDbgInstElimination). But if we want to cleanup along the way, we might end up wanting to run that pass after every pass that may introduce redundant dbg instrs. In the end that might be more costly, since we wouldn't know if the earlier passes actually did any transformations or not. The currently implementation tries to limit the amount of cleanup by doing it at transforms that are known to introduce redundant dbg instrs.

One thing I realize is that we probaly spend time looking for dbg instrs also in non-debug builds, right? I'm not sure if there is a quick way to check if the IR has dbg instrs or not, to make an early return in RemoveRedundantDbgInstrs if the module/function/basicblock is known to not having any dbg instrs. Any ideas on how to do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88928