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

[Robinson, Paul]

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.

--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