[PATCH] D100845: [Debugify][Original DI] Test preservation of original debug var intrinsics in optimizations

Thu Apr 22 02:43:35 PDT 2021

djtodoro added a comment.

@Orlando Thanks for looking into this.

In D100845#2705685 <https://reviews.llvm.org/D100845#2705685>, @Orlando wrote:

> Hi @djtodoro I am looking at this (slowly, sorry!).
>
> IIUC this checks that the number of debug intrinsics for each non-inlined variable does not decrease after each optimisation pass. There are legitimate reasons for deleting debug intrinsics. `RemoveRedundantDbgInstrs`, for example, which is called in a few places. With that in mind I think I would prefer the table header "Number of bugs" to change as this number seems to be more of an indicator of possible bugs, rather than proof of their existence.

Sure. That is why we treat it as "WARNING". The same is happening with dropping of dbg location attached to instructions -- there are places where we cannot salvage !dbg, and we treat it as a warning as well (until we add some super cool logic to distinguish what was reasonable drop or opposite).

> You might be able to reduce (*) the number of false positive results by ignoring "redundant" debug intrinsics in the count, though maybe that could be follow-up work?

That could be follow-up for sure. Thanks. In addition, I am seeing a lot of artifitial variables location dropping (such as `__result`, `__s` etc.), and I am not sure if we should care about these.

> (*) Another example of a false positive, or at least a true positive that we probably want to ignore: I tried this patch out out using a different target codebase and investigated a debug intrinsic dropped in SROA/mem2reg. Amusingly, `git blame` pointed right back at me: D89810 <https://reviews.llvm.org/D89810>. D89810 <https://reviews.llvm.org/D89810>, and D85555 <https://reviews.llvm.org/D85555> which the former is based on, deliberately remove dbg.value+DW_OP_deref intrinsics to work around a common case of location coverage reduction ultimately caused by LowerDbgDeclare.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100845