<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Thanks for your feedback everyone.<div class=""><br class=""></div><div class="">I'll try to summarize what people have said:</div><div class=""><br class=""></div><div class="">- There seems to be general consensus that we should move away from applying a static debug location to constant SD nodes. This should improve single-stepping and be more consistent with how constants are modeled in IR. </div><div class="">(I think Björn suggested working around this by applying an is_stmt attribute to constant materialization instructions, but there are a number of barriers to doing that (e.g we only have partial compiler/debugger support for doing this).)</div><div class=""><br class=""></div><div class="">- It may be helpful to have a late pass backfill missing locations. This would make a constant materialization look like a part of the expression it's for. I think this needs a bit more thought. For example: how would we avoid backfilling bogus locations onto instructions which aren't used to materialize constants?</div><div class=""><br class=""></div><div class="">For now, I'll get started on patches to drop debug locations from constant nodes.</div><div class=""><br class=""></div><div class="">vedant</div><div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jun 21, 2018, at 11:50 AM, Eric Christopher via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Yeah, I'm with you here. I don't think there's any particular debug info gain to having debug info on materializing a constant either.<div class=""><br class=""></div><div class="">Vedant: Ultimately I think I'm in favor of this plan.</div><div class=""><br class=""></div><div class="">Thanks!</div><div class=""><br class=""></div><div class="">-eric<br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Jun 20, 2018 at 4:32 PM Matthias Braun via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space" class=""><div class="">FWIW: Debug information on constants feels odd to me. They are just values not something that is executed so conceptually I would not expect them to "happen" at a specific time/place in the program. That said most numbers are copied into registers or stored into memory and that is of course an interesting action. So in the original example I would hope to see debug info on whatever instructions are used to fill the array with values.</div><div class=""><br class=""></div><div class="">That said I'm not familiar with the inner workings of dwarf or other debugger formats, so it may very well be reasonable to backfill the information in a late pass to avoid having assembler instructions without debug info as some people proposed.</div></div><div style="word-wrap:break-word;line-break:after-white-space" class=""><div class=""><br class=""></div><div class="">- Matthias</div></div><div style="word-wrap:break-word;line-break:after-white-space" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Jun 20, 2018, at 11:09 AM, via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="m_6241682479114180037Apple-interchange-newline"><div class=""><div class="m_6241682479114180037WordSection1" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)" class="">DwarfDebug::beginInstruction sets the location to line 0 because otherwise the location is implicitly the same location as the last instruction of the physically preceding block. That location is often completely unrelated to what's at the top of the new block. Line 0 isn't great, but at least it's not a complete lie.<u class=""></u><u class=""></u></span></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)" class="">--paulr<u class=""></u><u class=""></u></span></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><a name="m_6241682479114180037__MailEndCompose" class=""><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)" class=""><u class=""></u> <u class=""></u></span></a></div><div style="border-style:none none none solid;border-left-width:1.5pt;border-left-color:blue;padding:0in 0in 0in 4pt" class=""><div class=""><div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(181,196,223);padding:3pt 0in 0in" class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><b class=""><span style="font-size:10pt;font-family:Tahoma,sans-serif" class="">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif" class=""><span class="m_6241682479114180037Apple-converted-space"> </span><a href="mailto:vsk@apple.com" target="_blank" class="">vsk@apple.com</a> [<a href="mailto:vsk@apple.com" target="_blank" class="">mailto:vsk@apple.com</a>]<span class="m_6241682479114180037Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="m_6241682479114180037Apple-converted-space"> </span>Wednesday, June 20, 2018 1:48 PM<br class=""><b class="">To:</b><span class="m_6241682479114180037Apple-converted-space"> </span>Reid Kleckner<br class=""><b class="">Cc:</b><span class="m_6241682479114180037Apple-converted-space"> </span>Robinson, Paul; llvm-dev; Justin Bogner; David Li; David Blaikie<br class=""><b class="">Subject:</b><span class="m_6241682479114180037Apple-converted-space"> </span>Re: [RFC] Removing debug locations from ConstantSDNodes<u class=""></u><u class=""></u></span></div></div></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div><blockquote style="margin-top:5pt;margin-bottom:5pt" class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">On Jun 19, 2018, at 6:36 PM, Reid Kleckner <<a href="mailto:rnk@google.com" style="color:purple;text-decoration:underline" target="_blank" class="">rnk@google.com</a>> wrote:<u class=""></u><u class=""></u></div></blockquote><div class=""><blockquote style="margin-top:5pt;margin-bottom:5pt" class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div><div class=""><div class=""><div class=""><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">On Tue, Jun 19, 2018 at 5:46 PM Vedant Kumar <<a href="mailto:vsk@apple.com" style="color:purple;text-decoration:underline" target="_blank" class="">vsk@apple.com</a>> wrote:<u class=""></u><u class=""></u></div></div><blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in" class=""><div class=""><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" 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.<u class=""></u><u class=""></u></div></div></div></blockquote><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">Our use case was in fastisel, so things are different. I don't think my solution will help you.<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">In our case, users were complaining about code like this:<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">volatile int do_something;<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">void f() {<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""> ++do_something;<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""> foo(1, 2, 3);<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""> ++do_something;<br class="">}<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">We'd generate locations like:<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""> .loc line 1<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""> incl do_something(%rip)<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""> movl $1, %ecx<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="background-color:white;background-position:initial initial;background-repeat:initial initial" class=""> movl $2, %edx</span><u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="background-color:white;background-position:initial initial;background-repeat:initial initial" class=""> movl $3, %r8d</span><u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="background-color:white;background-position:initial initial;background-repeat:initial initial" class=""> .loc line 2 # line 2 starts here, instead of 3 instructions earlier</span><u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="background-color:white;background-position:initial initial;background-repeat:initial initial" class=""> callq foo</span><u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="background-color:white;background-position:initial initial;background-repeat:initial initial" class=""> .loc line 3</span><u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="background-color:white;background-position:initial initial;background-repeat:initial initial" class=""> incl do_something(%rip)</span><u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><span style="background-color:white;background-position:initial initial;background-repeat:initial initial" class=""><br class=""><br class=""></span><u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">Our users really wanted the line table entry for line 2 to include the constant materialization code for some VS debugger feature.<u class=""></u><u class=""></u></div></div></div></div></div></blockquote><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">Got it. (For others following along, there's more discussion about this debugger feature in the commit description for r327581.)<br class=""><br class=""><br class=""><br class=""><u class=""></u><u class=""></u></div><div class=""><div class=""><div class=""><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" 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.<u class=""></u><u class=""></u></div></div></div></div></div></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">Interesting, I hadn't considered doing this.<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" 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?<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" 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?<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class=""><u class=""></u> <u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">thanks!<u class=""></u><u class=""></u></div></div><div class=""><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:"Times New Roman",serif" class="">vedant<u class=""></u><u class=""></u></div></div></div></div><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important" class="">_______________________________________________</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important" class="">LLVM Developers mailing list</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important" class=""><a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a></span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none;float:none;display:inline!important" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a></span></div></blockquote></div><br class=""></div>_______________________________________________<br class="">
LLVM Developers mailing list<br class="">
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br class="">
</blockquote></div></div></div>
_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<br class=""></div></blockquote></div><br class=""></div></body></html>