[PATCH] D70642: [DebugInfo] Support for DW_OP_implicit_pointer (DW_OP_LLVM_argN)

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok added a comment.

In D70642#1769887 <https://reviews.llvm.org/D70642#1769887>, @aprantl wrote:

> In D70642#1769175 <https://reviews.llvm.org/D70642#1769175>, @alok wrote:
>
> > In D70642#1767321 <https://reviews.llvm.org/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.


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

https://reviews.llvm.org/D70642