[DebugInfo] Support for DW_OP_implicit_pointer (DW_OP_LLVM_argN)

[alok]

Summary

This patch (1/N) stems from D69787
It is suggested by @aprantl

Summary:

It Introduces a new Dwarf operator DW_OP_LLVM_argN (N=0-7).
Which represents Nth argument of containing intrinsic.

Example:

For example in intrinsic
  "call @llvm.dbg.someintrinsic(DILocalVariable("x"), DILocalVariable("y"),
         metadata !DIExpression(DW_OP_LLVM_arg0, DW_OP_LLVM_arg1, DW_OP_plus))"
  'DW_OP_LLVM_arg0' represents 'DILocalVariable("x")' and
  'DW_OP_LLVM_arg1' represents 'DILocalVariable("y")'.

[TWeaver]

Hiya,

thanks for the patch!

just a small nit from me.

Cheers.

llvm/docs/LangRef.rst

4818

'argumnet' should be 'argument'

[alok]

llvm/docs/LangRef.rst

4818

Thanks for pointing this out. It will be corrected in next version.

[alok]

Updated to incorporated review comment.

[aprantl]

Thanks! This will need some bitcode-compatibility planning as well:

We'll need to bump the version number, and then in MetadataLoader.cpp / upradeDIExpression() add an a DW_OP_LLVM_arg0 at the beginning for all older versions.
This should be tested with a .bc file that contains an old-form DIExpression, run through llvm-dis | llvm-as | llvm-dis and checked that the expression is upgraded correctly.

llvm/include/llvm/BinaryFormat/Dwarf.def

663

Since these extensions are only used inside LLVM IR, they shouldn't leak into dwarfdump & friends. Can you take a look at how DW_OP_LLVM_fragment is defined and copy that? I think it is hardcoded in Dwarf.cpp

[alok]

In D70642#1765515, @aprantl wrote:

Thanks! This will need some bitcode-compatibility planning as well:

We'll need to bump the version number, and then in MetadataLoader.cpp / upradeDIExpression() add an a DW_OP_LLVM_arg0 at the beginning for all older versions.
This should be tested with a .bc file that contains an old-form DIExpression, run through llvm-dis | llvm-as | llvm-dis and checked that the expression is upgraded correctly.

Thanks for your comment. I shall look into it.

llvm/include/llvm/BinaryFormat/Dwarf.def

663

Thanks for your comment. It will be incorporated in next version.

[alok]

This is updated to incorporate comments from @aprantl.

[aprantl]

Thanks. This part looks good, but the bitcode upgrade is still missing.

[alok]

In D70642#1767321, @aprantl wrote:

Thanks. This part looks good, but the bitcode upgrade is still missing.

Sorry as I could not include that in first set of modifications. I was just thinking about which operations can be upgraded. One candidate for this was new expression DW_OP_LLVM_implicit_pointer which comes later that the current patch so escapes the need for upgrade. Should I look for existing operations for potential candidates for upgrade. or should i re-arrange the DW_OP_LLVM_implicit_pointer patch such as first it should not use DW_OP_LLVM_arg0 and then after current patch it needs upgrade. Could you please clear my doubt here ?

[aprantl]

In D70642#1769175, @alok wrote:

In D70642#1767321, @aprantl wrote:

Thanks. This part looks good, but the bitcode upgrade is still missing.

Sorry as I could not include that in first set of modifications. I was just thinking about which operations can be upgraded. One candidate for this was new expression DW_OP_LLVM_implicit_pointer which comes later that the current patch so escapes the need for upgrade. Should I look for existing operations for potential candidates for upgrade. or should i re-arrange the DW_OP_LLVM_implicit_pointer patch such as first it should not use DW_OP_LLVM_arg0 and then after current patch it needs upgrade. Could you please clear my doubt here ?

The upgrade I was thinking about here is reading old bitcode that contains

llvm.dbg.value(%foo, DILocalVariable(...), DIExpression(DW_OP_deref))

and upgrades it to

llvm.dbg.value(%foo, DILocalVariable(...), DIExpression(DW_OP_arg0, DW_OP_deref))

However, for this to work, we also need to implement support for DW_OP_argN to AsmPrinter to interpret the new operands and DIBuilder.

My assumption is that after this change DIExpression() is illegal and should be replaced by DIExpression(DW_OP_LLVM_arg0).
This transformation will cause a lot of churn in the Clang and LLVM testsuite (most of which you will be able to fix with a a sed script), but it will allow to resolve the ambiguity between:

!DIGlobalVariableExpression(!DIGlobalVariable("a"), !DIExpression())
!DIGlobalVariableExpression(!DIGlobalVariable("a"), !DIExpression(DW_OP_constu 42, DW_OP_stack_value))
!DIGlobalVariableExpression(!DIGlobalVariable("a"), !DIExpression(DW_OP_constu 42, DW_OP_plus, DW_OP_stack_value))

in the new scheme, these expressions would be

!DIExpression(DW_OP_LLVM_arg0)
!DIExpression(DW_OP_constu 42, DW_OP_stack_value) # constant, no arg!
!DIExpression(DW_OP_LLVM_arg0, DW_OP_constu 42, DW_OP_plus, DW_OP_stack_value))

respectively. We currently disambiguate between these cases by context: if the DIGlobalVariableExpression is a !dbg attachment of an llvm::Global, the global is the implicit arg0, otherwise there is no implict argument.

Since this is going to be a lot of work, we could also opt for a smaller, incremental change first that introduces only DW_OP_LLVM_arg1..DW_OP_LLVM_arg7 and keep the DW_OP_LLVM_arg0 implicit. Then we don't need to worry about bitcode upgrades now.

llvm/docs/LangRef.rst

4823

I just realized that DIGLobalVariableExpression also takes a DIExpression. I think we should also allow DW_OP_LLVM_arg0 there.

[alok]

In D70642#1769887, @aprantl wrote:

In D70642#1769175, @alok wrote:

In D70642#1767321, @aprantl wrote:

Thanks. This part looks good, but the bitcode upgrade is still missing.

Sorry as I could not include that in first set of modifications. I was just thinking about which operations can be upgraded. One candidate for this was new expression DW_OP_LLVM_implicit_pointer which comes later that the current patch so escapes the need for upgrade. Should I look for existing operations for potential candidates for upgrade. or should i re-arrange the DW_OP_LLVM_implicit_pointer patch such as first it should not use DW_OP_LLVM_arg0 and then after current patch it needs upgrade. Could you please clear my doubt here ?

The upgrade I was thinking about here is reading old bitcode that contains

llvm.dbg.value(%foo, DILocalVariable(...), DIExpression(DW_OP_deref))

and upgrades it to

llvm.dbg.value(%foo, DILocalVariable(...), DIExpression(DW_OP_arg0, DW_OP_deref))

However, for this to work, we also need to implement support for DW_OP_argN to AsmPrinter to interpret the new operands and DIBuilder.

My assumption is that after this change DIExpression() is illegal and should be replaced by DIExpression(DW_OP_LLVM_arg0).
This transformation will cause a lot of churn in the Clang and LLVM testsuite (most of which you will be able to fix with a a sed script), but it will allow to resolve the ambiguity between:

!DIGlobalVariableExpression(!DIGlobalVariable("a"), !DIExpression())
!DIGlobalVariableExpression(!DIGlobalVariable("a"), !DIExpression(DW_OP_constu 42, DW_OP_stack_value))
!DIGlobalVariableExpression(!DIGlobalVariable("a"), !DIExpression(DW_OP_constu 42, DW_OP_plus, DW_OP_stack_value))

in the new scheme, these expressions would be

!DIExpression(DW_OP_LLVM_arg0)
!DIExpression(DW_OP_constu 42, DW_OP_stack_value) # constant, no arg!
!DIExpression(DW_OP_LLVM_arg0, DW_OP_constu 42, DW_OP_plus, DW_OP_stack_value))

respectively. We currently disambiguate between these cases by context: if the DIGlobalVariableExpression is a !dbg attachment of an llvm::Global, the global is the implicit arg0, otherwise there is no implict argument.

Since this is going to be a lot of work, we could also opt for a smaller, incremental change first that introduces only DW_OP_LLVM_arg1..DW_OP_LLVM_arg7 and keep the DW_OP_LLVM_arg0 implicit. Then we don't need to worry about bitcode upgrades now.

Thanks a lot for the clarification. Next version of patch will include this.

[alok]

This is re-based and updated to apply LLVM_arg0 to existing DWARF operator and to upgrade for older styled DWARF expressions. This is done as suggested by @aprantl .

[alok]

Re-based and some cleanup.

llvm/docs/LangRef.rst

- ``DW_OP_LLVM_entry_value, N`` can only appear at the beginning of a
   register location descriptions. The operation is also introduced by the
   ``AsmPrinter`` pass when a call site parameter value
   (``DW_AT_call_site_parameter_value``) is represented as entry value of the
   parameter.
 - ``DW_OP_breg`` (or ``DW_OP_bregx``) represents a content on the provided
   signed offset of the specified register. The opcode is only generated by the
   ``AsmPrinter`` pass to describe call site parameter value which requires an
   expression over two registers.
+- ``DW_OP_LLVM_argN`` represents argument number N of the containing LLVM
+  intrinsic. Where N can be any value from 0 to 7. For example in intrinsic
+  ``call @llvm.dbg.someintrinsic(DILocalVariable("x"), DILocalVariable("y"),
+  metadata !DIExpression(DW_OP_LLVM_arg0, DW_OP_LLVM_arg1, DW_OP_plus))``
+  ``DW_OP_LLVM_arg0`` represents ``DILocalVariable("x")`` and
+  ``DW_OP_LLVM_arg1`` represents ``DILocalVariable("y")``.
 
 DWARF specifies three kinds of simple location descriptions: Register, memory,
 and implicit location descriptions.  Note that a location description is
 defined over certain ranges of a program, i.e the location of a variable may
 change over the course of the program. Register and memory location
 descriptions describe the *concrete location* of a source variable (in the
 sense that a debugger might modify its value), whereas *implicit locations*
 describe merely the actual *value* of a source variable which might not exist

llvm/include/llvm/BinaryFormat/Dwarf.def

 // Vendor extensions:
 // Extensions for GNU-style thread-local storage.
 HANDLE_DW_OP(0xe0, GNU_push_tls_address, 0, GNU)
 // The GNU entry value extension.
 HANDLE_DW_OP(0xf3, GNU_entry_value, 0, GNU)
 // Extensions for Fission proposal.
 HANDLE_DW_OP(0xfb, GNU_addr_index, 0, GNU)
 HANDLE_DW_OP(0xfc, GNU_const_index, 0, GNU)
+// The LLVM extension.
+HANDLE_DW_OP(0x4000, LLVM_arg0, 0, LLVM)
+HANDLE_DW_OP(0x4001, LLVM_arg1, 0, LLVM)
+HANDLE_DW_OP(0x4002, LLVM_arg2, 0, LLVM)
+HANDLE_DW_OP(0x4003, LLVM_arg3, 0, LLVM)
+HANDLE_DW_OP(0x4004, LLVM_arg4, 0, LLVM)
+HANDLE_DW_OP(0x4005, LLVM_arg5, 0, LLVM)
+HANDLE_DW_OP(0x4006, LLVM_arg6, 0, LLVM)
+HANDLE_DW_OP(0x4007, LLVM_arg7, 0, LLVM)
 
 // DWARF languages.
 HANDLE_DW_LANG(0x0001, C89, 0, 2, DWARF)
 HANDLE_DW_LANG(0x0002, C, 0, 2, DWARF)
 HANDLE_DW_LANG(0x0003, Ada83, 1, 2, DWARF)
 HANDLE_DW_LANG(0x0004, C_plus_plus, 0, 2, DWARF)
 HANDLE_DW_LANG(0x0005, Cobol74, 1, 2, DWARF)
 HANDLE_DW_LANG(0x0006, Cobol85, 1, 2, DWARF)