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_implicit_pointer. It will later be converted to DW_OP_implicit_pointer while creating actual DWARF info
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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 | ||
---|---|---|
127 | 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. |
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.
llvm/lib/IR/DebugInfoMetadata.cpp | ||
---|---|---|
1120 | 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'?)) |
llvm/lib/IR/DebugInfoMetadata.cpp | ||
---|---|---|
1120 | Thanks for your comment. I shall add the invalid test cases. Transformation of these operators are |
llvm/docs/LangRef.rst | ||
---|---|---|
5286 | I think this could really benefit from an example. Also, it is odd to refer to a DILocalVariable in the documentation of a DIExpression operand, since DIExpressions can be pointed to by dbg.values (which may bind them to one or more DILocalVariables) but they can also appear in other places. I understand that the DWARF definition of DW_OP_implicit_pointer makes some of this strangeness necessary, but we should do at least our best in documenting how this is supposed to be used. |
llvm/docs/LangRef.rst | ||
---|---|---|
5286 | Thanks for your comment. I shall work on this. |
Seems generally good to me - but @aprantl and/or @JDevlieghere is probably the keeper of DWARF expression tracking stuff, so they ought to take a look/sign off.
Out of interest, can this be composed? eg:
struct x { int i; int *j; }; void f1(int p) { x y = {5, &p}; f(); // opaque function call - breaking here it'd be good to still be able to describe y.j as pointing to the value stored in 'p', even though that value shouldn't be in memory anywhere (shuold just be in a register) return *y.j; }
This is needed because representation and specification (types of operands) of
it a bit different than actual dwarf expression DW_OP_implicit_pointer.
Can you add this to the documention to explain what/how this is different from the DWARF op?
Otherwise this LGTM
there's an article missing .. the variable?