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

[Alexey Samsonov]

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.


>
> --paulr
>
> >
> > I think it's pretty much all we can do w/o actually changing the CodeGen
> > for
> > DBG_VALUE instructions.
> >
> > If you agree with the direction of this patch, I should probably test it
> > extensively. There are many concerns:
> > 0) Correctness. LLVM/Clang regression tests don't really check anything
> > -
> >    we should run it on gdb test-suite or smth. comprehensive.
> > 1) Debug info size. We should verify that .debug_loc section wouldn't
> > explode.
> > 2) Compilation time - in the worst case we smth. like
> >    O(NumVars * NumInstructions), which can be a lot for large functions.
> >
> > http://llvm-reviews.chandlerc.com/D2658
> >
> > Files:
> >   lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



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