[PATCH] D67492: [DebugInfo] Add a DW_OP_LLVM_entry_value operation

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/test/Verifier/diexpression-valid-entry-value.ll:5
 ; CHECK-NOT: invalid expression
-!0 = !DIExpression(DW_OP_entry_value, 1)
+!0 = !DIExpression(DW_OP_LLVM_entry_value, 1)
----------------
aprantl wrote:
> aprantl wrote:
> > dstenb wrote:
> > > djtodoro wrote:
> > > > I am just curious. Having the D67768, do we really still need the size of following expression at this point?
> > > I guess that the size operand is currently redundant, and we can omit it. However, I have made the assumption that specifying the size will simplify things when we get to describing more complex variables like `local` in the example below using entry values:
> > > 
> > > ```
> > > int foo(int param) {
> > >   int local = param + 1;
> > >   use_of_param = param;
> > >   clobber();
> > >   return 0;
> > > }
> > > ```
> > > 
> > > Here `local` will have the entry value of $edi + 1. GDB will not understand a `DW_OP_entry_value` whose block is that complex, so I imagine that we want to emit the same expression as GCC:
> > > 
> > > ```
> > > (DW_OP_entry_value: (DW_OP_reg5 (rdi)); DW_OP_plus_uconst: 1; DW_OP_stack_value)
> > > ```
> > > 
> > > I guess that the `DW_OP_LLVM_entry_value` operation could cover the whole expression in the DIExpression world, and we then wrap only the parts that actually needs to be covered by entry values when we lower that to `DW_OP_entry_value` in DwarfExpression. However, I'm a bit afraid that that would further complicate the DwarfExpression state machine, which is already quite complex.
> > > 
> > > Looking further onwards, I guess that we want to transform the current `dbg.value`/`DBG_VALUE`/`DIExpression` framework to something that allows us to describe a variable using multiple register / memory values. For example, like `local` in the example below:
> > > 
> > > ```
> > > int bar(int a, int b) {
> > >   int local = a + b;
> > >   [...]
> > > }
> > > ```
> > > 
> > > I assume that we want to be able to describe such variables using entry values for only a subset of the registers. GCC is able to emit such expressions. I guess that having a `DW_OP_LLVM_entry_value` operation that only applies for a part of an `DIExpression` will be helpful when we get there, but that's pure speculation as I don't know how the debug framework will look.
> > IMO, we should make the IR support as generic as possible and accept more complex uses of DW_OP_LLVM_entry_value that LLVM currently cannot produce itself and generate the correct DWARF for it.
> > 
> > In order to generate that particular example though, it might be useful to add another operand (let's call it DW_OP_LLVM_dbg_value) to make the implicit first operand explicit, so this could be
> > 
> > ```
> > !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_LLVM_dbg_value, DW_OP_plus, DW_OP_uconst, 1, DW_OP_stack_value).
> > ```
> > 
> > and the backend substitutes `DW_OP_breg5` for `DW_OP_dbg_value`. In the future, we could further generalize llvm.dbg.value to take extra arguments that would show up as `DW_OP_LLVM_dbg_value_[0-9]+` to allow binding more than one SSA value to a DIExpression.
> (and of course to confuse everybody I accidentally flipped the order of DW_OP_plus and DW_OP_uconst in my example...)
>Here local will have the entry value of $edi + 1. GDB will not understand a DW_OP_entry_value whose block is that complex, so I imagine that we want to emit the same expression as GCC:
Sure. Not only local variables, I am currently extending the `LiveDebugValues` to support such scenario when parameters are part of such expression and I will share that in the following days. Let's keep it for now.

Let me say something about extending the `llvm.dbg.value`, I already got similar idea to extend the `salvageDebugInfo()` functionality when we are removing an instruction operating on two registers (rather then reg and constant), so I think that having that type of instruction will give us a lot of benefits, not only in situations like that. :)


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

https://reviews.llvm.org/D67492