[PATCH] D69999: [DebugInfo] Support for DW_OP_implicit_pointer (IR Verifier and Bitcode)

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok marked 4 inline comments as done.
alok added a comment.

In D69999#1741360 <https://reviews.llvm.org/D69999#1741360>, @aprantl wrote:

> My first thought was that this might be an opportunity to finally implement explicit `dbg.value` arguments. (There is prior discussion of this online somewhere but I couldn't find it in my mailbox).
>
> The idea behind a  explicit `dbg.value` arguments is to move away from the implicit first argument of `dbg.value` to a format where that makes them explicit and thus more flexible.
>
> For example, currently we would write:
>
>   call @llvm.dbg.value(%1, !DILocalVariable(name: "x"), !DIExpression(DW_OP_uconst, 4, DW_OP_plus)
>
>
> which (assuming `%1` compiles to `reg0`) may generate something like
>
>   DW_TAG_variable
>     DW_AT_name("x")
>     DW_AT_location(DW_OP_reg0, DW_OP_uconst, 4, DW_OP_plus)
>
>
> Note the implicit first element in the DIExpression.
>
> An explicit notation would be something like:
>
>   call @llvm.dbg.value(%1, !DILocalVariable(name: "x"), !DIExpression(DW_OP_LLVM_arg0, DW_OP_uconst, 4, DW_OP_plus)
>
>
> which has the advantage that we could put `DW_OP_LLVM_arg0` into places other than just the beginning of the expression.
>
> Similarly, we could then further extend it to allow referring to more than one LLVM SSA value within an expression, using a variadic `dbg.value`. For example:
>
>   call @llvm.dbg.value(%1, %2, !DILocalVariable(name: "x"), !DIExpression(DW_OP_LLVM_arg0 /* %1 */ , DW_OP_uconst, DW_OP_LLVM_arg1 /* %2 */, DW_OP_plus)
>
>
> This mechanism seems to apply nicely to `DW_OP_implicit_pointer` — applied to your testcase we'd write:
>
>   call @llvm.dbg.value(!DILocalVariable(name: "arr"), !DILocalVariable(name: "ptr"), !DIExpression(DW_OP_LLVM_implicit_pointer, DW_OP_LLVM_arg0, 4)
>
>
> I think this would make the handling of DW_OP_LLVM_implicit_pointer less magic in LLVM IR and would open the door to future features, such as the variadic dbg.value.
>  What do you think?


Your suggestion looks great. I shall include that in next revision.

> What does the debug info cabal think?





================
Comment at: llvm/docs/LangRef.rst:4818
   expression over two registers.
+- ``DW_OP_implicit_pointer, +4`` can only appear at the beginning of a
+  ``DIExpression``, and it specifies that that an optimized out pointer's
----------------
aprantl wrote:
> +4 is abit ambiguous. Perhaps first do a generic description, followed by an example?
Thanks, it will be updated in next revision.


================
Comment at: llvm/docs/LangRef.rst:4819
+- ``DW_OP_implicit_pointer, +4`` can only appear at the beginning of a
+  ``DIExpression``, and it specifies that that an optimized out pointer's
+  value can be found at the given offset (+4) in the dereferenced object.
----------------
aprantl wrote:
> that that
Thanks, it will be updated in next revision.


================
Comment at: llvm/docs/LangRef.rst:4820
+  ``DIExpression``, and it specifies that that an optimized out pointer's
+  value can be found at the given offset (+4) in the dereferenced object.
 
----------------
aprantl wrote:
> Is that an accurate description? Per DWARF5:
> 
> > The DW_OP_implicit_pointer operation specifies that the object is a pointer that cannot be represented as a real pointer, even though the value it would point to can be described
> 
> 
It will be updated to incorporate your other comment to introduce DW_OP_LLVM_arg0 / DW_OP_LLVM_implicit_pointer 


================
Comment at: llvm/docs/SourceLevelDebugging.rst:248
+represets DW_OP_implicit_pointer operation followed by an offset, the first
+argument is `local variable <LangRef.html#dilocalvariable>`_ which represents a
+variable which at given offset is pointed by the variable represented by the
----------------
aprantl wrote:
> Let's move this into the paragraph in LangRef, so it's all in one place.
It will be updated in next revision.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69999/new/

https://reviews.llvm.org/D69999