llvm dev - Oct 2020 - Notes from dbg.value coffee chat

On Fri, Oct 9, 2020 at 9:38 AM Cazalet-Hyams, Orlando <
orlando.hyams at sony.com> wrote:
> Hi Reid,
>
>
>
> Thanks for sharing this. I plan to work on improving debug-info for
> variables
>
> living in memory as my next "project" so I am very interested to
hear with
> what
>
> others have to say about "Idea 1".
>
>
>
> There is one part of the idea that confuses me. You say we could "keep
the
>
> dbg.declare instructions", but I don't see where dbg.declare
instructions
> - at
>
> least with their current semantics - fit into the design if the frontend is
>
> emitting dbg.values after every assignment. Could you please expand on
> this part
> a little?
>
I think what I meant is that we need to keep the association between the
alloca and the local variable somewhere. The current implementation of
dbg.declare is not what we want: when we have one, it overrides any
dbg.values, and the alloca is used as the variable location for the full
scope. That's not what we want, but we do need an association between
alloca and variable+scope somewhere. The dbg.values in the design as we
discussed it would not contain the alloca, but the SSA value which is
stored to the alloca. Maybe if we augmented the dbg.values with the store
destination, we could get by without tracking that information separately.

The main design goal here was to have the variable location information be
correct even when DSE occurs, without updating every pass that deletes
stores, because there are many. Thinking about it today, I'm not sure this
design is complete yet. Even if the frontend effectively emits two stores
for every assignment, a real store, and dbg.value, the backend needs to
determine if the real store survived optimization. If it did, then the
variable value lives in memory. If it did not, then the variable value is
the value which would've been stored, if it is available at this program
point, or if it is available somewhere nearby. Maybe that's acceptable, but
it seems difficult. However, maybe that's similar to what we already do for
dbg.values uses that precede definitions.

The alternative to this design would be to say that stores to static
allocas are special, and cannot be deleted without updating debug info.
This is basically https://llvm.org/pr34136#c25. Thinking about it again
today, maybe this approach is feasible. We could build tooling to audit for
passes that delete these special "assigning stores". Maybe that's
better
than bending over backwards just to make it easy for the optimizers.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201009/0ef50f73/attachment.html>

> I think what I meant is that we need to keep the association between the
alloca and the local variable somewhere. The current implementation of
dbg.declare is not what we want: when we have one, it overrides any dbg.values,
and the alloca is used as the variable location for the full scope. That's
not what we want, but we do need an association between alloca and
variable+scope somewhere. The dbg.values in the design as we discussed it would
not contain the alloca, but the SSA value which is stored to the alloca. Maybe
if we augmented the dbg.values with the store destination, we could get by
without tracking that information separately.
> The main design goal here was to have the variable location information be
correct even when DSE occurs, without updating every pass that deletes stores,
because there are many. Thinking about it today, I'm not sure this design is
complete yet. Even if the frontend effectively emits two stores for every
assignment, a real store, and dbg.value, the backend needs to determine if the
real store survived optimization. If it did, then the variable value lives in
memory. If it did not, then the variable value is the value which would've
been stored, if it is available at this program point, or if it is available
somewhere nearby. Maybe that's acceptable, but it seems difficult. However,
maybe that's similar to what we already do for dbg.values uses that precede
definitions.
That makes sense, thank you. I had also thought about extending dbg.value to
keep the store destination. One step further could be to introduce a mechanism
to refer to the store itself instead of the store destination (possibly similar
to Jeremy's recent MIR debug instruction referencing work [1], but for this
situation in IR). If the referenced store has been removed (e.g. by DSE) then we
know to use the value instead, which may also have been salvaged or be undef at
this point. And if the store has not been removed then we can grab the
destination from the store itself.

I'm starting to form the opinion that the ideal solution will provide
correctness by default without passes having to do anything, but allow the onus
of improving coverage to fall upon those passes. We've recently taken a turn
in this direction in D80264 [2] whereby we now replace an instruction's
ValueAsMetadata uses with undef on deletion. This provides correctness by
default for debug intrinsics using the instruction, and the coverage may be
improved by a pass appropriately calling salvageDebugInfo beforehand.
> The alternative to this design would be to say that stores to static
allocas are special, and cannot be deleted without updating debug info. This is
basically https://llvm.org/pr34136#c25. Thinking about it again today, maybe
this approach is feasible. We could build tooling to audit for passes that
delete these special "assigning stores". Maybe that's better than
bending over backwards just to make it easy for the optimizers.
The "static allocas are special" [3] approach is appealing because it
seems like it may give promising results without too much work (compared with
these other ideas, anyway). It does rely on passes to provide debug-info
correctness, but tooling may be an appropriate stand in for correctness by
default if we decide to go down this route. Did you have anything in mind for
this?

Thanks,
Orlando

[1] http://lists.llvm.org/pipermail/llvm-dev/2020-February/139440.html
[2] https://reviews.llvm.org/D80264
[3] https://llvm.org/pr34136#c25

From: Reid Kleckner <rnk at google.com>
Sent: 09 October 2020 19:13
To: Cazalet-Hyams, Orlando <orlando.hyams at sony.com>
Cc: llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] Notes from dbg.value coffee chat

On Fri, Oct 9, 2020 at 9:38 AM Cazalet-Hyams, Orlando <orlando.hyams at
sony.com<mailto:orlando.hyams at sony.com>> wrote:
Hi Reid,

Thanks for sharing this. I plan to work on improving debug-info for variables
living in memory as my next "project" so I am very interested to hear
with what
others have to say about "Idea 1".

There is one part of the idea that confuses me. You say we could "keep the
dbg.declare instructions", but I don't see where dbg.declare
instructions - at
least with their current semantics - fit into the design if the frontend is
emitting dbg.values after every assignment. Could you please expand on this part
a little?

I think what I meant is that we need to keep the association between the alloca
and the local variable somewhere. The current implementation of dbg.declare is
not what we want: when we have one, it overrides any dbg.values, and the alloca
is used as the variable location for the full scope. That's not what we
want, but we do need an association between alloca and variable+scope somewhere.
The dbg.values in the design as we discussed it would not contain the alloca,
but the SSA value which is stored to the alloca. Maybe if we augmented the
dbg.values with the store destination, we could get by without tracking that
information separately.

The main design goal here was to have the variable location information be
correct even when DSE occurs, without updating every pass that deletes stores,
because there are many. Thinking about it today, I'm not sure this design is
complete yet. Even if the frontend effectively emits two stores for every
assignment, a real store, and dbg.value, the backend needs to determine if the
real store survived optimization. If it did, then the variable value lives in
memory. If it did not, then the variable value is the value which would've
been stored, if it is available at this program point, or if it is available
somewhere nearby. Maybe that's acceptable, but it seems difficult. However,
maybe that's similar to what we already do for dbg.values uses that precede
definitions.

The alternative to this design would be to say that stores to static allocas are
special, and cannot be deleted without updating debug info. This is basically
https://llvm.org/pr34136#c25. Thinking about it again today, maybe this approach
is feasible. We could build tooling to audit for passes that delete these
special "assigning stores". Maybe that's better than bending over
backwards just to make it easy for the optimizers.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201012/8f84a4a9/attachment.html>

On Mon, Oct 12, 2020 at 9:13 AM Cazalet-Hyams, Orlando <
orlando.hyams at sony.com> wrote:
> The "static allocas are special" [3] approach is appealing
because it
> seems like it may give promising results without too much work (compared
> with these other ideas, anyway). It does rely on passes to provide
> debug-info correctness, but tooling may be an appropriate stand in for
> correctness by default if we decide to go down this route. Did you have
> anything in mind for this?
>
All I had in mind was something like:
- add a pass that finds all "local variable stores" (stores to a
static
alloca used by a dbg.declare), and inserts markers (llvm.donothing?)
carrying the local variable metadata after every such store
- after optimization, check that every marker is preceded by either a store
to a local variable of a dbg.value for the same local variable. Any marker
with no dbg.value or store before it is a bug, unless the variable is
available nowhere (I forget how that is represented)
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201013/13e814ce/attachment.html>