[PATCH] D85555: [InstCombine] Remove dbg.values describing contents of dead allocas

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D85555#2307077 <https://reviews.llvm.org/D85555#2307077>, @aprantl wrote:

>   %a = alloca i32
>   store i32 1, i32* %a ; DSE kills this store.
>   store i32 2, i32* %a
>   dbg.value(i32* %a, "var", DW_OP_deref)
>   call void @some_callee(i32* %a)
>   ; Assume the alloca %a is not erased.
>
> The salvage operation I had in mind (but didn't specify!) wouldn't trigger here:
>
> I wrote
>
>> We could define a new type salvage operation that looks at to-be-deleted store foo, a instructions that store to an alloca a that is used in a dbg.value(a, var, DIExpression(DW_OP_deref, expr)) that is following the store.
>
> but this should be clarified to say
>
> We could define a new type salvage operation that looks at to-be-deleted store foo, a instructions that store to an alloca a that is used in a dbg.value(a, var, DIExpression(DW_OP_deref, expr)) that is **immediately** following the store.
>
> or more elaborate
>
> We could define a new type salvage operation that looks at to-be-deleted store foo, a instructions that store to an alloca a that is used in a dbg.value(a, var, DIExpression(DW_OP_deref, expr)) that is between the store and the next store or call referencing the alloca, or another dbg.value(var).

It's possible there's some more general solution to this problem that I've missed, but I don't think this is it. My concern is that these salvage options could be fragile and expensive. Slight changes to clang's codegen, or to passes that run before the salvage occurs, might disable the salvage. E.g., if there's an alloca/bitcast in between the store and the DW_OP_deref dbg.value, the salvage couldn't proceed. You could work around this by doing a scan starting from the dead store to find the next instruction that writes to memory, but past attempts to do that have been piecemeal and fragile as well. For example, LowerDbgDeclare tries to reason about which instructions write to memory (it insert dbg.values after stores), but it misses a large set of instructions that write to memory (cmpxchg/memcpy/...).

Istm that the approach taken in this patch is simpler, and probably less fragile: it's cheap to locate stale DW_OP_deref dbg.values, and deleting them has known tradeoffs that we've accepted elsewhere (it's the same stale value leak problem as LowerDbgDeclare has).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85555