<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><blockquote type="cite" class="">On Jun 19, 2018, at 6:36 PM, Reid Kleckner <<a href="mailto:rnk@google.com" class="">rnk@google.com</a>> wrote:<br class=""></blockquote><div><blockquote type="cite" class=""><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, Jun 19, 2018 at 5:46 PM Vedant Kumar <<a href="mailto:vsk@apple.com" class="">vsk@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">Someone (Reid?) mentioned that we could try sinking constants to their point of first use as an alternative, and (IIUC) create new nodes with distinct DebugLocs for each use of a constant. I don't recall the details of that alternative clearly. Based on my (likely incorrect) understanding of it, dropping locations from constants outright might be simpler.<br class=""></div></div></blockquote><div class=""><br class=""></div><div class="">Our use case was in fastisel, so things are different. I don't think my solution will help you.</div><div class=""><br class=""></div><div class="">In our case, users were complaining about code like this:</div><div class="">volatile int do_something;</div><div class="">void f() {</div><div class=""> ++do_something;</div><div class=""> foo(1, 2, 3);</div><div class=""> ++do_something;<br class="">}</div><div class=""><br class=""></div><div class="">We'd generate locations like:</div><div class=""> .loc line 1</div><div class=""> incl do_something(%rip)</div><div class=""> movl $1, %ecx</div><div class="">
<span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline" class=""> movl $2, %edx</span><br class=""></div><div class=""><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline" class=""> movl $3, %r8d</span></div><div class=""><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline" class=""> .loc line 2 # line 2 starts here, instead of 3 instructions earlier</span></div><div class=""><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline" class=""> callq foo</span></div><div class=""><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline" class=""> .loc line 3</span></div><div class=""><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline" class=""> incl do_something(%rip)</span></div><div class=""><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline" class=""><br class=""></span></div><div class="">Our users really wanted the line table entry for line 2 to include the constant materialization code for some VS debugger feature.</div></div></div></div></blockquote><div><br class=""></div><span class="">Got it. (For others following along, there's more discussion about this debugger feature in the commit description for r327581.)<br class=""></span><br class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class="">I think if you remove locations from ConstantSDNodes, you might want to add a late pass that propagates source locations backwards onto location-less instructions. This would also avoid some special cases when a basic block starts with an instruction that lacks location information. See CodeViewDebug::beginInstruction and DwarfDebug::beginInstruction for what we do today.</div></div></div></div></blockquote></div><br class=""><div class="">Interesting, I hadn't considered doing this.</div><div class=""><br class=""></div><div class="">What happens in the special case where a block starts with a location-less instruction? Ah, I see that CodeViewDebug::beginInstruction does a forward scan to find the first instruction with location. This can fail though: there might not be such an instruction, in which case... I assume we either apply a line-0 location, or nothing at all, similar to Dwarf::beginInstruction?</div><div class=""><br class=""></div><div class="">It's a bit unclear to me what the benefits of a late pass that backwards-propagated locations would be. I suppose we'd have fewer forward scans in CodeViewDebug::beginInstruction, so is the benefit a compile-time win (which would offset the cost of a late pass)? Would the final debug info quality would be better in some way I'm just missing?</div><div class=""><br class=""></div><div class="">thanks!</div><div class="">vedant</div></body></html>