[PATCH] D37911: [DebugInfo] Insert DW_OP_deref when spilling indirect DBG_VALUEs

[Adrian Prantl via Phabricator via llvm-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Thanks! Are you running the debuginfo-tests on your changes prior to committing?



================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:500
   // Get or create the UserValue for (variable,offset).
-  bool IsIndirect = MI.isIndirectDebugValue();
+  bool IsIndirect = MI.getOperand(1).isImm();
   if (IsIndirect)
----------------
rnk wrote:
> aprantl wrote:
> > rnk wrote:
> > > aprantl wrote:
> > > > Why can't this be API?
> > > I want to consider frame indices and constants as "indirect" so that they use the same UserValue. Some tests (I'll check how many) rely on this for eliminating adjacent DBG_VALUE instructions describing the same variable, like this:
> > >   DBG_VALUE %vreg3, 0, !"v" # Goes away after rewriting if we use isImm() directly
> > >   DBG_VALUE 42, 0, !"v"
> > >   mov $42, %eax
> > >   retq
> > > 
> > > In the long run, IsIndirect should not be considered when getting or creating a UserValue. I have a follow-on patch that implements this, but I split it out for incrementality.
> > Can you post you work-in-progress for the follow-up? I'd like to better understand where this is heading.
> D37932. It needs tests, though. I haven't found a way to write any without waiting for llvm.dbg.addr or manually editting MIR, so I held off on it. But, the basic idea is that a DBG_VALUE location is more than just one MachineOperand: it's that plus the IsIndirect bit.
Thanks! It would be great if you could split out the NFC bits of the follow-up into a separate commit (hereby LGTM'ed) following  shortly after this one.


https://reviews.llvm.org/D37911