Page MenuHomePhabricator

[Debuginfo] (1/8) [DW_OP_implicit_pointer/second strategy] Support for DW_OP_LLVM_implicit_pointer
Needs ReviewPublic

Authored by alok on Jul 18 2020, 4:54 PM.

Details

Summary
This patch stems from D84112.
New dwarf operator DW_OP_LLVM_implicit_pointer is introduced (present only in LLVM IR)
This is needed because representation and specification (types of operands) of it a bit different than actual dwarf expression DW_OP_LLVM_implicit_pointer. while creating actual DWARF info it will be converted to DW_OP_LLVM_implicit_pointer

Diff Detail

Event Timeline

alok created this revision.Jul 18 2020, 4:54 PM
jmorse added a subscriber: jmorse.Jul 30 2020, 5:38 AM

Hi @alok, would you be able to re-title the reviews in this stack to indicate what's in the patch itself? That'll make it easier to navigate.

llvm/include/llvm/BinaryFormat/Dwarf.h
125

Any particular reason for 0x100c instead of the sequentially next 0x1004?

llvm/test/DebugInfo/X86/LLVM_implicit_pointer.ll
3

Would you be able to add a file-comment along the lines of "round-trip test for implicit pointer metadata"? That'll make it easier for future readers to know what's being tested.

alok marked 2 inline comments as done.Aug 3 2020, 4:05 AM

Thanks for your reviewing this.

llvm/include/llvm/BinaryFormat/Dwarf.h
125

Thanks for pointing this out. It shall be corrected in next version of patch.

llvm/test/DebugInfo/X86/LLVM_implicit_pointer.ll
3

It shall be included in next version of patch.

alok added a reviewer: jmorse.Aug 3 2020, 4:06 AM
alok updated this revision to Diff 282621.Aug 3 2020, 7:37 AM
alok marked 2 inline comments as done.
alok retitled this revision from [Debuginfo] (1/N) Support for DW_OP_implicit_pointer for named and unnamed variables (second strategy). to [Debuginfo] (1/8) [DW_OP_implicit_pointer/second strategy] Support for DW_OP_LLVM_implicit_pointer.
alok edited the summary of this revision. (Show Details)
alok added a comment.Aug 12 2020, 7:15 AM

@jmorse Do you have any more comment on this ?

No real additional comments on this or the 2nd patch -- for further progress though I think you need to stimulate some discussion in the llvm-dev "DW_OP_implicit_pointer design/implementation in general" thread, to convince David / Adrian that this patch series addresses their concerns from the earlier discussion.

aprantl added inline comments.Aug 12 2020, 4:34 PM
llvm/docs/LangRef.rst
5187

there's an article missing .. the variable?

5187

I'm also missing a Verifier check that enforces this constraint?

5188

instruction's

5189

I think we could expand this a bit.

  • is it illegal in a DIGlobalVariableExpression?

Maybe include an example use?

dblaikie added inline comments.Wed, Sep 23, 7:39 PM
llvm/lib/IR/DebugInfoMetadata.cpp
1025

Probably need to have test coverage for these invalid cases

(& I'm not quite clear on the reasoning for where this can appear (why only at the beginning? I would've naively guessed maybe only at the end? (top of the stack) but haven't thought it through in any great detail, for sure - making sure it generalizes over multiple lost indirection would be great (I guess given the DWARF format, it can't generalize over parts of an object lacking indirection, but maybe it'd be nice if the IR format supported it (eg: "struct x { int y; int *z; }" if 'z' no longer has pointable storage - can we still describe what it points to, while also describing the value of 'y'?))