[PATCH] [RFC] Rewrite the way we generate debug locations for variables.

[Alexey Samsonov]

On Fri, Jan 31, 2014 at 11:04 PM, Adrian Prantl <aprantl at apple.com> wrote:

>
> On Jan 31, 2014, at 6:41, Alexey Samsonov <samsonov at google.com> wrote:
> >
> > On Fri, Jan 31, 2014 at 3:13 AM, Robinson, Paul <
> > Paul_Robinson at playstation.sony.com> wrote:
> >
> > > I really really really wish I had time to review this now.
> > > For the moment some kibitzing based on prior experience.
> > >
> > > > -----Original Message-----
> > > > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> > > > bounces at cs.uiuc.edu] On Behalf Of Alexey Samsonov
> > > > Sent: Thursday, January 30, 2014 8:20 AM
> > > > To: dblaikie at gmail.com; samsonov at google.com
> > > > Cc: llvm-commits at cs.uiuc.edu
> > > > Subject: [PATCH] [RFC] Rewrite the way we generate debug locations
> for
> > > > variables.
> > > >
> > > > Hi dblaikie,
> > > >
> > > > Completely change the way we turn DBG_VALUE machine instructions
> > > > into location information for local variables. This is inspired by
> > > > discussion
> > > > in r128327 review thread.
> > > >
> > > > We used to terminate location range for each variable at the end of
> the
> > > > basic
> > > > block with DBG_VALUE for it. This is incorrect, e.g. the register
> used
> > > > to hold
> > > > the variable location may be never clobbered in the entire function,
> so
> > > > one
> > > > DBG_VALUE is enough to define the location of a variable in the whole
> > > > function.
> > >
> > > Terminating at that point is the safe thing to do.  Working out
> > > when it's safe not to terminate is the tricky part, yeah.
> > >
> > > >
> > > > Now we actually analyze the control flow: calculate the presumed
> > > > location of
> > > > a variable at the end and the beginning of each basic block. For
> > > > example,
> > > > if the location of a variable is the same at the end of all
> predecessors
> > > > of
> > > > basic block B, we may assume that we know the location of a variable
> in
> > > > B,
> > >
> > > Sure.
> > >
> > > > and avoid terminating the location range too early.
> > >
> > > Whoa.  Looking at predecessors can tell you where something is on
> > > block entry, but that's not the same as being able to not terminate
> > > a range.  Predecessors are a control-flow thing, ranges are a
> > > physical-layout thing.  Maybe you account for this in your patch,
> > > like I said I haven't looked at it yet, but it's a trap for the
> > > unwary.
> > >
> >
> > Yes, that's what complicates things. I take into account the order in
> which
> > machine
> > basic blocks are emitted and do a "best effort". Suppose that BB3 has two
> > predecessors
> > in control-flow graph: BB1 and BB2, and location of variable "v" at the
> end
> > of BB1 is known
> > and is the same as its location at the end of BB2. Then we check if we
> know
> > the location
> > of "v" at the end of basic block, _preceding BB3 in the physical layout_
> > (this may be BB1, BB2,
> > or some other basic block). If, again, the location happens to be the
> same,
> > then we can continue
> > the location range for "v" to BB3. Otherwise we terminate the location
> > range at the beginning of BB3.
> >
>
> Sounds like a data flow analysis that computes the meet-over-all paths
> solution for the DBG_VALUE assignments for each DIVariable where the join
> only keeps DBG_VALUES that are identical for all predecessors.



>
> I imagine that we could implement this as a pass on the MC level.
>

Yep, sounds like this. However, we need to ensure that our pass will be the
last one, and MachineFunction will not be changed
further.
We can also insert instruction of the form
  DBG_VALUE: var "x" is undefined
right after instruction clobbering the register used to locate "x". This
would streamline turning sequence of "DBG_VALUE" instructions into actual
location ranges.


>
> Before:
> -------
> BB1:
>   put "x" in %rax
>   DBG_VALUE: var "x" is in %rax
>   clobber %rbx
>   jmp BB4
> BB2:
>   put "x" in %rbx
>   DBG_VALUE: var "x" is in %rbx
>   clobber %rax
>   jmp BB3
> BB3:
>   jmp BB4
> BB4:
>
> After:
> ------
> BB1:
>   put "x" in %rax
>   DBG_VALUE: var "x" is in %rax
>   clobber %rbx
>   jmp BB4
> BB2:
>   put "x" in %rbx
>   DBG_VALUE: var "x" is in %rbx
>   clobber %rax
>   jmp BB3
> BB3:
>   DBG_VALUE: var "x" is in %rbx
>   jmp BB4
> BB4:
>   DBG_VALUE: var "x” is #undef
>
> My point is that this would be more explicit and debuggable. And the naive
> location merging code in DwarfDebug::emitDebugLoc() would get rid of
> redundant DBG_VALUEs (BB3 in the example). I think it would be better to
> avoid introducing additional complexity into DwarfDebug and friends.
>

If we decide to design a specific MC pass for that, I may start with
factoring the existing logic for calculating variable history into such a
pass.


>
>
> -- adrian
>
>

-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140203/071ebe8b/attachment.html>