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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

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

> I believe that the following could work and be a self-contained local operation that doesn't need any contract with a previous optimization pass:

(A brief aside: as currently written, this patch doesn't rely on a special contract with a previous optimization pass either.)

>   %a = alloca i32              ; This alloca is erased.
>   store i32 %0, i32* %a
>   dbg.value(i32* %a, "arg0", DW_OP_deref)
>   call void @trivially_inlinable_no_op(i32* %a)
>
> 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. It would salvage the store by replacing the dbg.value with `dbg.value(foo, var, DIExpression(expr))`.
>
> This example would become
>
>   dbg.value(i32* %0, "arg0", DIExpression())
>   call void @trivially_inlinable_no_op(i32* %a)
>
> That would also apply to any store to `a` inside the function after inlining.

I think this more general approach could interact poorly with dead store elimination. For example, you might have:

  %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.

With the proposed salvage op, after DSE, we'd get:

  %a = alloca i32
  store i32 1, i32* %a ; DSE kills this store.
  store i32 2, i32* %a
  dbg.value(i32 1, "var", ())
  call void @some_callee(i32* %a)

This results in an incorrect description of "var" both before the call to "some_callee" and potentially after, depending on whether "some_callee" stores into "var".

I don't think this addresses the problem Jeremy identified either. In his example, there's a `store i32 1, i32* %addr` in the inlined callee: under your proposal, this wouldn't get salvaged because it's /after/ the `dbg.value(a, var, DIExpression(DW_OP_deref, expr))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85555