[PATCH] Move the IsIndirect flag from DIVariable into DIExpression.

[David Blaikie]

On Fri, Jan 16, 2015 at 10:52 AM, Adrian Prantl <aprantl at apple.com> wrote:

> In http://reviews.llvm.org/D6986#109826, @dblaikie wrote:
>
> > (might be easier to review/commit by adding the expression handling then
> just removing the flag as dead code afterwards, but I'm not sure - it's not
> bad the way you've got it either)
>
>
> Oh that would be great, but it's complicated:
> After having read this patch, look at this gem from SelectionDAGBuilder:
>
>   uint64_t Offset = DI->getOffset();
>     // A dbg.value for an alloca is always indirect.
>     bool IsIndirect = isa<AllocaInst>(V) || Offset != 0;
>     SDDbgValue *SDV;
>     if (Val.getNode()) {
>       if (!EmitFuncArgumentDbgValue(V, Variable, Expr, Offset, IsIndirect,
>                                     Val)) {
>         SDV = DAG.getDbgValue(Variable, Expr, Val.getNode(),
> Val.getResNo(),
>                               IsIndirect, Offset, dl, DbgSDNodeOrder);
>         DAG.AddDbgValue(SDV, Val.getNode(), false);
>       }
>     } else
>
> It is rematerializing the OP_deref that this patch removes in the form a
> of an indirect MachineLocation.
>
> Horrible, right?
>

OK, so the indirect flag on MachineLocation is separate/in addition to the
indirect flag on DIVariable that gets passed around through a few other
layers/side channels.

So you could still remove the DIVariable related indirection and keep the
MachineLocation indirection for now, maybe? Not sure. Maybe they end up
together eventually.


> We //could// get rid of the indirect field in MachineLocation, if we were
> to redefine alloca's to be treated as addresses rather than values in
> dbg.value/dbg.declare.
> Let me give an example. Currently, the following code:
>
>   %x = alloca i64
>   call void @llvm.dbg.declare(metadata %i64* %b, [var=x], [expr=[]])
>   store i64 42, %x
>
> is lowered into
>
>   %x = i64 42
>   call void @llvm.dbg.value(metadata %i64* %b, [var=x], [expr=[]])
>
> Observe that the alloca, although it is treated like an address in IR, is
> treated like it was the value as far as the debug info is concerned.
> If we want to fix this we'd have to change the frontend to emit
>
>   %x = alloca i64
>   call void @llvm.dbg.declare(metadata %i64* %b, [var=x],
> [expr=[DW_OP_deref]])
>   store i64 42, %x
>
> which could then be lowered into the same code as above. After separating
> out DIExpression from DIVariable this is really easy to do; before it was
> pretty much impossible.
>
> I think I'd love to do that instead. Should we go break stuff? ;-)
>

I believe this is the right direction to go sooner or later (Eric - can you
confirm/deny?). This would get us pretty close to just removing dbg.declare
entirely, wouldn't it? Or am I misremembering/misunderstanding the
distinction. It'd just be a matter of whether we are indirecting through a
register that doesn't change or not. (modulo lifetime intrinsics that make
this harder - but they shouldn't shrink variable lifetime beyond the
lexical scope anyway... maybe? so perhaps they don't matter)


>
>
> REPOSITORY
>   rL LLVM
>
> http://reviews.llvm.org/D6986
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150116/5ee53d25/attachment.html>