[llvm-dev] [RFC] Allowing debug intrinsics to reference multiple SSA Values

[David Blaikie via llvm-dev]

This ambiguity could also be resolved without the "push everything"
approach, though, right?

If the format supported zero operands - then the expression would be
evaluated with an empty stack (rather than the implicit first element stack
- since there's no element). It'd make the "treat an operand list of length
1 specially, by pushing it onto the stack before evaluating the DWARF
expression" look even more special/less tidy, but not totally outside the
realm of possibility.

(I don't have super strong feelings here - I like consistency, so I like
the idea of pushing all the operands or using an extension op to access
them all - but I can see the value in special casing the 1 operand case to
maintain the behavior/syntax it has today)


On Mon, Feb 24, 2020 at 9:45 AM Adrian Prantl <aprantl at apple.com> wrote:

>
>
> > On Feb 21, 2020, at 5:24 AM, Tozer, Stephen <stephen.tozer at sony.com>
> wrote:
> >
> > What would it look like without this extension? If we modeled it as if
> all the register values were already on the stack (an extension of the
> current way where the singular value is modeled as being already on the
> stack, if I understand it correctly?)?
> >
> > If it's decided that the best approach is to introduce something like
> DW_OP_LLVM_register - might be worth migrating to that first (basically
> adding DW_OP_LLVM_register(0) at the start of every DIExpression) and then
> expanding it from unary to n-ary support
> > Putting the register values initially on the stack reduces the
> verbosity, though it could complicate successive salvages of variadic
> DIExpressions - if any value other than the last needs salvaging, then you
> have to use DWARF stack operations to move it to the top of the stack. For
> example, if the elements are pushed in order so that the last element is on
> the top of the stack:
> >
> > %c = mul 3, %a
> > %d = add 5, %b
> > dbg.value(!DILocalVariable("x"), !DIExpression(DW_OP_plus), %c, %d)
> > ; Salvage %d
> > dbg.value(!DILocalVariable("x"), !DIExpression(DW_OP_plus_constu, 5,
> DW_OP_plus), %c, %b)
> > ; Salvage %c needs to use DW_OP_swap
> > dbg.value(!DILocalVariable("x"), !DIExpression(DW_OP_plus_constu, 5,
> DW_OP_swap, DW_OP_constu, 3, DW_OP_mul, DW_OP_plus), %a, %b)
> >
> > Or, written out as the stack state:
> > [%a, %b]               ; Initial state
> > [%a, %b + 5]           ; DW_OP_plus_constu, 5
> > [%b + 5, %a]           ; DW_OP_swap
> > [%b + 5, %a, 3]        ; DW_OP_constu, 3
> > [%b + 5, %a * 3]       ; DW_OP_mul
> > [(%b + 5) + (%a * 3)]  ; DW_OP_plus
> >
> > The simplest and most general solution would be to use DW_OP_pick, which
> duplicates the stack element at a given index to the top of the stack. This
> is more or less the same as using DW_OP_LLVM_register - both of them take
> an index corresponding to a register value - with a few differences: we get
> to maintain the default concise dbg.value with a single argument and empty
> DIExpression, but salvaging becomes more brittle. If we rely on elements
> being on the stack in their declared order, then we need to guarantee that
> nothing modifies those stack elements. The cleanest implementation of this
> would be for SalvageDebugInfo to salvage expressions normally when the last
> register is salvaged, and switch to using DW_OP_pick for every register
> whenever any other register is salvaged:
> >
> > %c = add %a, 5
> > %e = div %c, %d
> > ; x = %b * ((%a + 5) / %d)
> > dbg.value(!DILocalVariable("x"), !DIExpression(DW_OP_mul), %b, %e)
> > ; Salvage %e; last operator so salvage normally
> > dbg.value(!DILocalVariable("x"), !DIExpression(DW_OP_mul, DW_OP_div),
> %b, %c, %d)
> > ; Salvage %c; not last operator and expression isn't already using pick,
> so add DW_OP_pick for registers
> > dbg.value(!DILocalVariable("x"), !DIExpression(DW_OP_pick, 0,
> DW_OP_pick, 1, DW_OP_plus_constu, 5, DW_OP_pick, 2, DW_OP_div, DW_OP_mul),
> %b, %a, %d)
> >
> > The major advantage of putting all registers on the stack is that it
> reduces verbosity and also doesn't require us to implement the new operator
> or update existing DIExpressions to contain it, which is useful. Personally
> I lean towards keeping things simple and consistent, and so using the
> pick/register operator in every expression rather than only the ones that
> need it, but there's a good case for either I think.
>
>
> As the person who has advocated for DW_OP_LLVM_arg(N) before, my main
> motivation was to resolve the ambiguity of constant DIExpressions: As a
> worst-case example:
>
> dbg.value(%undef, !DILocalVariable(x), DIExpression(DW_OP_constu, 42))
>
> Is this undefined, or constant 42?
>
> But if we make dbg.value fully variadic with all parameters pushed to the
> stack ahead of time, we can distinguish between
>
> dbg.value(!DILocalVariable(x), DIExpression(DW_OP_constu, 42)) ; a constant
> dbg.value(i32 42, !DILocalVariable(x), DIExpression()) ; the same constant
> dbg.value(%undef, !DILocalVariable(x), DIExpression(DW_OP_constu, 42)) ;
> undef + garbage leftover expression
>
> If we do it this way and allow 0-n SSA values then I'm in support of the
> push-first scheme.
>
> As a side note, updating dbg.values in salvageDebugInfo when we allow
> multiple SSA values per dbg.value will be interesting, but possible. We'll
> need to put the value we need to update at the top of the stack, prepend
> the salvage expression and then restore the order of arguments on the stack
> that the rest of the expression expects.
>
> -- adrian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200224/ee189630/attachment.html>