llvm dev - Nov 2019 - DW_OP_implicit_pointer design/implementation in general

> I don't have a strong opinion on representation. I can see how having a
dedicated instruction to model implicit pointers would aid readability & be
simpler to document/grok, but perhaps in the future we'll want to support
other operations that refer to variable > DIEs. In the short term migrating
to an extended dbg.value representation might take more work. Alok, wdyt?

Below is what I think for each suggestion.

DW_OP_LLVM_implicit_pointer
  * This is a good suggestion to include that in LLVM IR, because
representation and specification (types of operands) of it a bit different
that actual dwarf expression DW_OP_LLVM_implicit_pointer. while creating
actual dwarf info it will be converted to DW_OP_LLVM_implicit_pointer. This
is implemented and patch is updated for it.

DW_OP_LLVM_arg0
  * This is good suggestion and will help in readability. It is also
implemented and is available in updated patch.

Splitting dbg.value
  * This is also a good idea from readability point of view. It also opens
possibility of extension below is explanation.
Since dbg.value currently represents (VAR=VALUE), the new intrinsic
dbg.deref_value will represent de-referenced value (*VAR = VAL)
    - Below represents ptr=null
      call void @llvm.dbg.value(metadata i32* null, metadata !21, metadata
!DIExpression())
    - And below represents *ptr=var
      call void @llvm.dbg.deref.value(metadata !16, metadata !21, metadata
!DIExpression(DW_OP_LLVM_implicit_pointer, DW_OP_LLVM_arg0, 0))
    - And below represents *ptr=arr[1]
      call void @llvm.dbg.deref.value(metadata !16, metadata !21, metadata
!DIExpression(DW_OP_LLVM_implicit_pointer, DW_OP_LLVM_arg0, 4))
With this new representation we should be able to represent the case
mentioned by David (in LLVM IR, we would still need some Dwarf operator to
be understood by LLDB) when a variable points to temporary (initialized by
constant) and temporary is optimized out.
      tmp=[CONST]; ptr=&tmp;
call void @llvm.dbg.deref.value(metadata [const], metadata !21, metadata
!DIExpression(DW_OP_LLVM_arg0))
I shall update my patch with introduction of dbg.deref_value. Please do
review.

Variadic dbg.value
   It is also a good idea. But since no immediate benefit seem to be
availed by implicit pointer, it can be done independently.

Regards,
Alok


On Wed, Nov 20, 2019 at 5:23 AM Vedant Kumar via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
>
> > On Nov 19, 2019, at 9:41 AM, Adrian Prantl via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >
> >
> >
> >> On Nov 18, 2019, at 8:33 AM, Jeremy Morse <jeremy.morse.llvm at
gmail.com>
> wrote:
> >>
> >> Hi llvm-dev@,
> >>
> >> Switching focus to the LLVM implementation, the significant change
is
> >> using dbg.value's first operand to refer to a DILocalVariable,
rather
> >> than a Value. There's some impedance mismatch here, because
all the
> >> documentation (for example in the DbgVariableIntrinsic class)
> >> expresses everything in terms of the variables location, whereas
> >> implicit pointers don't have a location as they represent an
extra
> >> level of indirection. This is best demonstrated by the change to
> >> IntrinsicInst.cpp in this patch [0] -- calling getVariableLocation
on
> >> any normal dbg.value will return the locations Value, but if
it's an
> >> implicit pointer then you'll get the meaningless
MetadataAsValue
> >> wrapper back instead. This isn't the variable location, might
surprise
> >> existing handlers of dbg.values, and just seems a little off.
> >>
> >> I can see why this route has been taken, but by putting a
non-Value in
> >> dbg.value's, it really changes what dbg.values represent, a
variable
> >> location in the IR. Is there any appetite out there for using a
> >> different intrinsic, something like 'dbg.loc.implicit',
instead of
> >> using dbg.value? IMO it would be worthwhile to separate:
> >> * Debug intrinsics where their position in the IR is important,
from
> >> * Debug intrinsics where both their position in the IR, _and_ a
Value
> >> in the IR, are important.
> >> Of which (I think) implicit pointers are the former, and current
[2]
> >> dbg.values are the latter. This would also avoid putting
> >> DW_OP_implicit_pointer into expressions in the IR, pre-isel at
least.
> >>
> >
> >
> > On that particular point, I would like to see is a generalization of
> dbg.value: Currently llvm.dbg.value binds an SSA value (including constants
> and undef) and a DIExpression to a DILocalVariable at a position in the
> instruction stream. That first SSA value argument is an implicit first
> element in the DIExpression.
> >
> > A more general form would be a more printf-like signature:
> >
> > llvm.dbg.value(DILocalVariable, DIExpression, ...)
> >
> > for example
> >
> > llvm.dbg.value_new(DILocalVariable("x"),
DIExpression(DW_OP_LLVM_arg0),
> %x)
> > llvm.dbg.value_new(DILocalVariable("y"),
DIExpression(DW_OP_LLVM_arg0,
> DW_OP_LLVM_arg1, DW_OP_plus),
> >                   %ptr, %ofs)
> > llvm.dbg.value_new(DILocalVariable("z"),
> DIExpression(DW_OP_implicit_pointer, DW_OP_LLVM_arg0, 32),
> >                   DILocalVariable("base"))
> > llvm.dbg.value_new(DILocalVariable("c"),
DIExpression(DW_OP_constu, 1))
> >
> > The mandatory arguments would be the variable and the expression, and
an
> arbitrary number of SSA values and potentially other variables.
>
> I don't have a strong opinion on representation. I can see how having a
> dedicated instruction to model implicit pointers would aid readability
& be
> simpler to document/grok, but perhaps in the future we'll want to
support
> other operations that refer to variable DIEs. In the short term migrating
> to an extended dbg.value representation might take more work. Alok, wdyt?
>
> vedant
>
> >
> >
> > As far as DW_OP_LLVM_implicit_pointer in particular is concerned, we
> could also treat the peculiarities of DW_OP_implicit_pointer as a DWARF
> implementation detail, introduce DW_OP_LLVM_implicit_pointer which
> transforms the top-of-stack into an implicit pointer (similar to
> DW_OP_stack_value) and have the DWARF backend insert an artificial variable
> on the fly.
> >
> > LLVM IR:
> >
> > llvm.dbg.value(%base, DILocalVariable("z"),
> DIExpression(DW_OP_LLVM_implicit_pointer))
> >
> > AsmPrinter would expand this into two DW_TAG_variable tags with one
> location (list) entry each.
> >
> > -- adrian
> >
> >> There's also Vedants suggestion [1] for linking implicit
pointer
> >> locations with the dbg.values of the underlying DILocalVariable. I
> >> suspect the presence of control flow might make it difficult
(there's
> >> no dbg.phi instruction), but I like the idea of having more
explicit
> >> links in the IR, it would be much clearer to interpret what's
going
> >> on.
> >>
> >> [0] https://reviews.llvm.org/D69999?id=229790
> >> [1] https://reviews.llvm.org/D69886#1736182
> >> [2] Technically dbg.value(undef,...) is the former too, I guess.
> >>
> >> --
> >> Thanks,
> >> Jeremy
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191120/c0870841/attachment.html>

Hi,

For a new way of representing things,

Adrian wrote:
> llvm.dbg.value_new(DILocalVariable("y"),
DIExpression(DW_OP_LLVM_arg0, DW_OP_LLVM_arg1, DW_OP_plus),
>                    %ptr, %ofs)
I think this would be great -- there're definitely some constructs
created by the induction-variables pass and similar where one could
recover an implicit variable value, if you could for example subtract
one pointer from another.

With the current model of storing DIExpressions as a vector of
opcodes, it might become a pain to salvage a Value that gets optimised
out --in the example, if %ofs were salvaged, presumably
DW_OP_LLVM_arg1 could have to be replaced with several extra
operations. This isn't insurmountable, but I've repeatedly shied away
from scanning through DIExpressions to patch them up. A vector of
opcodes is the final output of the compiler, IMHO richer metadata
should be used in the meantime.

IMHO the implicit pointer work doesn't need to block on this. As said
my mild preference would be for a new intrinsic for this form of
variable location.

~

Inre PR37682,
> I’ve been reminded of PR37682, where a function with a reference parameter
might spend all its time computing the “referenced” value in a temp, and only
move the final value back to the referenced object at the end.  This is clearly
a situation that could benefit from DW_OP_implicit_pointer, and there is really
no other-object DIE for it to refer to.  Given the current spec, the compiler
would need to produce a DW_TAG_dwarf_procedure for the parameter DIE to refer
to.  Appendix D (Figure D.61) has an example of this construction, although it’s
a more contrived source example.
This has been working through my mind too, and I think it's slightly
different to what implicit_pointer is trying to achieve. In the case
implicit_pointer is designed for, it's a strict improvement in debug
experience because you're recovering information that couldn't be
expressed. However for PR37682 it's a trade-off between whether the
user might want to examine the pointer, or the pointed-at integer:
AFAIUI, we can only express one of the two, not both. Wheras for
mem2reg'd variables referred to by DIE, there is never a pointer to be
lost.

I think my preference would always be to see temporarily-promoted
values as there's no other way of observing them, but others might
disagree.

--
Thanks,
Jeremy

Hi folks,

I am pushing a PoC patch https://reviews.llvm.org/D70833 for review which
includes the case when temporary is promoted.

For such cases it generates IR as

  call void @llvm.dbg.derefval(metadata i32 3, metadata !25, metadata
!DIExpression(DW_OP_LLVM_explicit_pointer, DW_OP_LLVM_arg0)), !dbg !32

And llvm-darfdump output looks like

-------------
0x0000007b:     DW_TAG_inlined_subroutine
                  DW_AT_abstract_origin (0x0000004f "_Z4sinkRKi")
                  DW_AT_low_pc  (0x00000000004004c6)
                  DW_AT_high_pc (0x00000000004004d0)
                  DW_AT_call_file
("/home/alok/openllvm/llvm-project_derefval/build.d/david.cc")
                  DW_AT_call_line       (10)
                  DW_AT_call_column     (0x03)

0x00000088:       DW_TAG_formal_parameter
                    DW_AT_location      (indexed (0x0) loclist = 0x00000010:
                       [0x00000000004004c6, 0x00000000004004d4):
DW_OP_explicit_pointer, DW_OP_lit3)
                    DW_AT_abstract_origin       (0x00000055 "p")
------------

Please note that DW_OP_explicit_pointer denotes that following value
represents de-referenced value of optimized out pointer. With necessary
changes in LLDB debugger this dwarf info can help to detect the explicit
de-referenced value of 'p'.

Hi David,

Should we keep on working for the above case separately and resume the
review of implicit pointer independently now, which is updated with many
suggestions from this discussion?

Regards,
Alok


On Wed, Nov 20, 2019 at 11:24 PM Jeremy Morse <jeremy.morse.llvm at
gmail.com>
wrote:
> Hi,
>
> For a new way of representing things,
>
> Adrian wrote:
> > llvm.dbg.value_new(DILocalVariable("y"),
DIExpression(DW_OP_LLVM_arg0,
> DW_OP_LLVM_arg1, DW_OP_plus),
> >                    %ptr, %ofs)
>
> I think this would be great -- there're definitely some constructs
> created by the induction-variables pass and similar where one could
> recover an implicit variable value, if you could for example subtract
> one pointer from another.
>
> With the current model of storing DIExpressions as a vector of
> opcodes, it might become a pain to salvage a Value that gets optimised
> out --in the example, if %ofs were salvaged, presumably
> DW_OP_LLVM_arg1 could have to be replaced with several extra
> operations. This isn't insurmountable, but I've repeatedly shied
away
> from scanning through DIExpressions to patch them up. A vector of
> opcodes is the final output of the compiler, IMHO richer metadata
> should be used in the meantime.
>
> IMHO the implicit pointer work doesn't need to block on this. As said
> my mild preference would be for a new intrinsic for this form of
> variable location.
>
> ~
>
> Inre PR37682,
>
> > I’ve been reminded of PR37682, where a function with a reference
> parameter might spend all its time computing the “referenced” value in a
> temp, and only move the final value back to the referenced object at the
> end.  This is clearly a situation that could benefit from
> DW_OP_implicit_pointer, and there is really no other-object DIE for it to
> refer to.  Given the current spec, the compiler would need to produce a
> DW_TAG_dwarf_procedure for the parameter DIE to refer to.  Appendix D
> (Figure D.61) has an example of this construction, although it’s a more
> contrived source example.
>
> This has been working through my mind too, and I think it's slightly
> different to what implicit_pointer is trying to achieve. In the case
> implicit_pointer is designed for, it's a strict improvement in debug
> experience because you're recovering information that couldn't be
> expressed. However for PR37682 it's a trade-off between whether the
> user might want to examine the pointer, or the pointed-at integer:
> AFAIUI, we can only express one of the two, not both. Wheras for
> mem2reg'd variables referred to by DIE, there is never a pointer to be
> lost.
>
> I think my preference would always be to see temporarily-promoted
> values as there's no other way of observing them, but others might
> disagree.
>
> --
> Thanks,
> Jeremy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191129/368d2139/attachment.html>