[llvm-dev] DW_OP_implicit_pointer design/implementation in general

[David Blaikie via llvm-dev]

Realized I didn't document the original reviews that motivated this thread:

A stack of reviews, split off from here: https://reviews.llvm.org/D69787

Alok's posted a new patch (with smaller patches split off from the
monolithic one) here: https://reviews.llvm.org/D84112

I haven't had a chance to page in all the old context, nor look at the
new ones in detail yet. But probably worth keeping high level design
review here, I think? Once the general direction seems good, we can go
into the separate review threads for the implementation/mechanical
details.

On Tue, Jan 14, 2020 at 8:47 AM David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Mon, Jan 13, 2020 at 8:49 PM Alok Sharma <aloksharma.knit at gmail.com> wrote:
>>
>> Hi,
>>
>> Let me consolidate what we discussed with my opinion.
>>
>> * On the point of new intrinsic llvm.dbg.derefval:
>> It (new intrinsic) was more a neater way than a needed way. The whole functionality can go ahead without it and using llvm.dbg.value instead. Though I liked it (new intrinsic), since most of us are against it, it should be fine for me to drop it.
>> This is because the transformation was like
>> llvm.dbg.value -> DBG_VALUE -> DWARF location-list
>> llvm.dbg.derefval -> DBG_VALUE -> DWARF location-list
>> Since it was just for better readability in LLVM IR only later it (new intrinsic) was sharing the same path with llvm.dbg.value. So it should be fine to drop it without any impact in later functionality.
>> - On question of identify such cases we can anyway identify using the type of expression (DW_OP_implicit_pointer).
>> - On question of ( DW_OP_implicit_pointer ) fitting to dbg.value intrinsic it perfectly does that as value in such case is metadata and prototype of dbg.value is dbg.value(metadata, metadata, metadata).
>> So it should be fine to drop it and back to where it was started before introduction of new intrinsic.
>>
>> * On the point of handing of pointer pointing to temporary / unnamed variables (Lets call it Scope S1)
>> As two proposed patches are there for bringing pointers referring to temporary / unnamed variable
>>    A) first patch uses (new proposed operator) DW_OP_LLVM_explicit_pointer(both in LLVM-IR and DWARF)
>>    B) Second patch uses artificial variable representing temporary (both in LLVM-IR and DWARF)
>>          https://reviews.llvm.org/D72055 [DebugInfo] Support for DW_OP_implicit_pointer (for temp references & dynamic allocations)
>> If I understood David correctly, he wants LLVM-IR look like patch-A and DWARF look like patch-B (lets call it way C)
>> Since patch-A is not desired because we don't support anything beyond DWARF-5 and patch proposes new DWARF operator. I want to clarify that patch-B can exist even without new intrinsic and can use dbg.value and fits perfectly in existing LLVM-IR template. if only reason to go way-C is to
>> I would like to go way-B or way-C for the scope of unnamed variables.
>>
>> * For the cases when pointer points to named variable (Lets call it Scope S2):
>> I would update the patches with replacing dbg.derefval to dbg.value and using DW_OP_implicit_pointer (to named variable) in both LLVM-IR and DWARF.
>>
>> In summary,
>> Scope S1 can be solved with
>> way-B)  DW_OP_implicit_pointer with artificial variable and with intrinsic dbg.value in LLVM-IR and DWARF
>> or
>> way-C) DW_OP_LLVM_explicit_pointer with intrinsic dbg.value in LLVM-IR + DW_OP_implicit_pointer with artificial variable in DWARF
>>
>> Scope S2 can be solved with
>> DW_OP_implicit_pointer with actual named variable with dbg.value in LLVM-IR and DWARF
>
>
> Based on my current understanding, I'd rather not do this ^ it seems like added complexity to LLVM optimizations/middle end/IR representation with limited value. I think doing (C) above but using it to cover both S1 and S2 (ie: not special casing "pointing to an existing variable" - instead treating that the same as "pointing to an unnamed entity") initially, and then, maybe later, evaluating whether referencing named variables from dbg.value is worth the added benefit, would be the right way to go.
>
>>
>>
>> Regards,
>> Alok
>>
>>
>> Though
>>
>>
>>
>> lets
>>
>> On Tue, Jan 14, 2020 at 1:28 AM Jeremy Morse <jeremy.morse.llvm at gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> David wrote:
>>> > Are you referring to the possibility of implicit values to refer to other variables? I'm sort of interested in maybe not doing that - and only implementing a more general form (what's been talked about with the LLVM_implicit_value (or was it LLVM_explicit_value? I forget)) - and synthesizing artificial variables in the backend rather than trying to track which variable a pointer points to. I think this would keep the impact on optimizations smaller & would be more general.
>>>
>>> Adrian wrote:
>>> > If it were possible to synthesize it in AsmPrinter, would that remove the motivation for the new intrinsic for you?
>>>
>>> Ah, yeah, those changes would avoid any need for a new intrinsic to my
>>> mind, and sounds much more palatable. Thanks for explaining.
>>>
>>> --
>>> Thanks,
>>> Jeremy