<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 31, 2014 at 3:13 AM, Robinson, Paul <span dir="ltr"><<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I really really really wish I had time to review this now.<br>
For the moment some kibitzing based on prior experience.<br>
<div class="im"><br>
> -----Original Message-----<br>
> From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:llvm-commits-">llvm-commits-</a><br>
> <a href="mailto:bounces@cs.uiuc.edu">bounces@cs.uiuc.edu</a>] On Behalf Of Alexey Samsonov<br>
> Sent: Thursday, January 30, 2014 8:20 AM<br>
> To: <a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>; <a href="mailto:samsonov@google.com">samsonov@google.com</a><br>
> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> Subject: [PATCH] [RFC] Rewrite the way we generate debug locations for<br>
> variables.<br>
><br>
> Hi dblaikie,<br>
><br>
> Completely change the way we turn DBG_VALUE machine instructions<br>
> into location information for local variables. This is inspired by<br>
> discussion<br>
> in r128327 review thread.<br>
><br>
> We used to terminate location range for each variable at the end of the<br>
> basic<br>
> block with DBG_VALUE for it. This is incorrect, e.g. the register used<br>
> to hold<br>
> the variable location may be never clobbered in the entire function, so<br>
> one<br>
> DBG_VALUE is enough to define the location of a variable in the whole<br>
> function.<br>
<br>
</div>Terminating at that point is the safe thing to do.  Working out<br>
when it's safe not to terminate is the tricky part, yeah.<br>
<div class="im"><br>
><br>
> Now we actually analyze the control flow: calculate the presumed<br>
> location of<br>
> a variable at the end and the beginning of each basic block. For<br>
> example,<br>
> if the location of a variable is the same at the end of all predecessors<br>
> of<br>
> basic block B, we may assume that we know the location of a variable in<br>
> B,<br>
<br>
</div>Sure.<br>
<div class="im"><br>
> and avoid terminating the location range too early.<br>
<br>
</div>Whoa.  Looking at predecessors can tell you where something is on<br>
block entry, but that's not the same as being able to not terminate<br>
a range.  Predecessors are a control-flow thing, ranges are a<br>
physical-layout thing.  Maybe you account for this in your patch,<br>
like I said I haven't looked at it yet, but it's a trap for the<br>
unwary.<br></blockquote><div><br></div><div>Yes, that's what complicates things. I take into account the order in which machine</div><div>basic blocks are emitted and do a "best effort". Suppose that BB3 has two predecessors</div>
<div>in control-flow graph: BB1 and BB2, and location of variable "v" at the end of BB1 is known</div><div>and is the same as its location at the end of BB2. Then we check if we know the location</div><div>of "v" at the end of basic block, _preceding BB3 in the physical layout_ (this may be BB1, BB2,</div>
<div>or some other basic block). If, again, the location happens to be the same, then we can continue</div><div>the location range for "v" to BB3. Otherwise we terminate the location range at the beginning of BB3.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
--paulr<br>
<div class="im"><br>
><br>
> I think it's pretty much all we can do w/o actually changing the CodeGen<br>
> for<br>
> DBG_VALUE instructions.<br>
><br>
> If you agree with the direction of this patch, I should probably test it<br>
> extensively. There are many concerns:<br>
> 0) Correctness. LLVM/Clang regression tests don't really check anything<br>
> -<br>
>    we should run it on gdb test-suite or smth. comprehensive.<br>
> 1) Debug info size. We should verify that .debug_loc section wouldn't<br>
> explode.<br>
> 2) Compilation time - in the worst case we smth. like<br>
>    O(NumVars * NumInstructions), which can be a lot for large functions.<br>
><br>
> <a href="http://llvm-reviews.chandlerc.com/D2658" target="_blank">http://llvm-reviews.chandlerc.com/D2658</a><br>
><br>
> Files:<br>
>   lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
<br>
</div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div>