<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 31, 2014 at 11:04 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><br>
On Jan 31, 2014, at 6:41, Alexey Samsonov <<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>> wrote:<br>
><br>
> On Fri, Jan 31, 2014 at 3:13 AM, Robinson, Paul <<br>
> <a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</a>> wrote:<br>
><br>
> > I really really really wish I had time to review this now.<br>
> > For the moment some kibitzing based on prior experience.<br>
> ><br>
> > > -----Original Message-----<br>
> > > From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank">llvm-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:llvm-commits-" target="_blank">llvm-commits-</a><br>
> > > <a href="mailto:bounces@cs.uiuc.edu" target="_blank">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" target="_blank">dblaikie@gmail.com</a>; <a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a><br>
> > > Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">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>
> > 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>
> ><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>
> > Sure.<br>
> ><br>
> > > and avoid terminating the location range too early.<br>
> ><br>
> > 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>
> ><br>
><br>
> Yes, that's what complicates things. I take into account the order in which<br>
> machine<br>
> basic blocks are emitted and do a "best effort". Suppose that BB3 has two<br>
> predecessors<br>
> in control-flow graph: BB1 and BB2, and location of variable "v" at the end<br>
> of BB1 is known<br>
> and is the same as its location at the end of BB2. Then we check if we know<br>
> the location<br>
> of "v" at the end of basic block, _preceding BB3 in the physical layout_<br>
> (this may be BB1, BB2,<br>
> or some other basic block). If, again, the location happens to be the same,<br>
> then we can continue<br>
> the location range for "v" to BB3. Otherwise we terminate the location<br>
> range at the beginning of BB3.<br>
><br>
<br>
</div></div>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.</blockquote>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
I imagine that we could implement this as a pass on the MC level.<br></blockquote><div><br></div><div>Yep, sounds like this. However, we need to ensure that our pass will be the last one, and MachineFunction will not be changed<br>
</div><div>further.</div><div>We can also insert instruction of the form</div><div> DBG_VALUE: var "x" is undefined</div><div>right after instruction clobbering the register used to locate "x". This would streamline turning sequence of "DBG_VALUE" instructions into actual location ranges.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Before:<br>
-------<br>
<div>BB1:<br>
put "x" in %rax<br>
DBG_VALUE: var "x" is in %rax<br>
clobber %rbx<br>
</div> jmp BB4<br>
<div>BB2:<br>
put "x" in %rbx<br>
DBG_VALUE: var "x" is in %rbx<br>
clobber %rax<br>
jmp BB3<br>
BB3:<br>
</div> jmp BB4<br>
BB4:<br>
<br>
After:<br>
------<br>
<div>BB1:<br>
put "x" in %rax<br>
DBG_VALUE: var "x" is in %rax<br>
clobber %rbx<br>
</div> jmp BB4<br>
<div>BB2:<br>
put "x" in %rbx<br>
DBG_VALUE: var "x" is in %rbx<br>
clobber %rax<br>
jmp BB3<br>
BB3:<br>
</div><div> DBG_VALUE: var "x" is in %rbx<br>
</div> jmp BB4<br>
BB4:<br>
DBG_VALUE: var "x” is #undef<br>
<br>
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.<br>
</blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span><font color="#888888"><br>
<br>
-- adrian<br>
<br>
</font></span></blockquote></div><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div>