<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 11, 2017 at 6:18 AM Brian Cain <<a href="mailto:bcain@codeaurora.org">bcain@codeaurora.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple" class="gmail_msg"><div class="m_-4093212520546456173WordSection1 gmail_msg"><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d" class="gmail_msg">The resulting breakage is only to the feature that’s new as of r294231, or does it impact more than that?  <u class="gmail_msg"></u><u class="gmail_msg"></u></span></p><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d" class="gmail_msg">Could we do this partial revert first and then in a subsequent commit add the code to support both use cases?</span></p></div></div></blockquote><div><br>It's been in for a couple of months now (yeah, it's a pretty vague line where to change from "revert early/revert often" to "fix forward" - but I'm leaning the latter at this point) so I'd prefer to fix forward if possible.<br><br>Looks like it'd be a weird regression for llvm-symbolizer to have the function start line dropped because the function name info wasn't requested. (they seem to me like orthogonal features)<br><br>So probably best to update the test case added in r294231 to include a variant that switches off function name resolution, but shows the function start line is still printed. Then preserve that functionality/test while fixing your issue - probably with an extra flag down through DILineInfoSpecifier.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple" class="gmail_msg"><div class="m_-4093212520546456173WordSection1 gmail_msg"><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d" class="gmail_msg"><u class="gmail_msg"></u><u class="gmail_msg"></u></span></p><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d" class="gmail_msg">-Brian<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p><div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt" class="gmail_msg"><div class="gmail_msg"><div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0in 0in 0in" class="gmail_msg"><p class="MsoNormal gmail_msg"><b class="gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif" class="gmail_msg">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif" class="gmail_msg"> David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>] <br class="gmail_msg"><b class="gmail_msg">Sent:</b> Monday, April 10, 2017 3:53 PM<br class="gmail_msg"><b class="gmail_msg">To:</b> <a href="mailto:reviews%2BD31905%2Bpublic%2B410e06a7b468ba68@reviews.llvm.org" class="gmail_msg" target="_blank">reviews+D31905+public+410e06a7b468ba68@reviews.llvm.org</a>; <a href="mailto:bcain@codeaurora.org" class="gmail_msg" target="_blank">bcain@codeaurora.org</a>; <a href="mailto:sque@google.com" class="gmail_msg" target="_blank">sque@google.com</a>; <a href="mailto:echristo@gmail.com" class="gmail_msg" target="_blank">echristo@gmail.com</a><br class="gmail_msg"><b class="gmail_msg">Cc:</b> <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg"><b class="gmail_msg">Subject:</b> Re: [PATCH] D31905: Exit early from start line search for FunctionNameKind::None<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p></div></div></div></div></div><div lang="EN-US" link="blue" vlink="purple" class="gmail_msg"><div class="m_-4093212520546456173WordSection1 gmail_msg"><div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt" class="gmail_msg"><p class="MsoNormal gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p><div class="gmail_msg"><p class="MsoNormal gmail_msg">I would guess this would then cause the line to not be retrieved when it might be desired? But I haven't thought about it too hard (hopefully Simon can chime in).<br class="gmail_msg"><br class="gmail_msg">If that's the case & the existing functionality needs to be preserved, then maybe two functions will be needed or some other way to communicate "actually I don't need the line" (pass it by pointer instead of reference - null pointer indicates "not needed")<u class="gmail_msg"></u><u class="gmail_msg"></u></p></div><p class="MsoNormal gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p><div class="gmail_msg"><div class="gmail_msg"><p class="MsoNormal gmail_msg">On Mon, Apr 10, 2017 at 1:46 PM Brian Cain via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<u class="gmail_msg"></u><u class="gmail_msg"></u></p></div><blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in" class="gmail_msg"><p class="MsoNormal gmail_msg" style="margin-bottom:12.0pt">bcain created this revision.<br class="gmail_msg"><br class="gmail_msg"><a href="https://reviews.llvm.org/rL294231" class="gmail_msg" target="_blank">https://reviews.llvm.org/rL294231</a> introduced a regression -- memory consumption for llvm-objdump during disassembly of ~500MB hexagon elf file was ~1.5GiB higher after that commit.<br class="gmail_msg"><br class="gmail_msg">This change restores the previous behavior -- exit the search early for FunctionNameKind::None, saving some memory allocation.<br class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg">Repository:<br class="gmail_msg">  rL LLVM<br class="gmail_msg"><br class="gmail_msg"><a href="https://reviews.llvm.org/D31905" class="gmail_msg" target="_blank">https://reviews.llvm.org/D31905</a><br class="gmail_msg"><br class="gmail_msg">Files:<br class="gmail_msg">  lib/DebugInfo/DWARF/DWARFContext.cpp<br class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg">Index: lib/DebugInfo/DWARF/DWARFContext.cpp<br class="gmail_msg">===================================================================<br class="gmail_msg">--- lib/DebugInfo/DWARF/DWARFContext.cpp<br class="gmail_msg">+++ lib/DebugInfo/DWARF/DWARFContext.cpp<br class="gmail_msg">@@ -466,6 +466,9 @@<br class="gmail_msg">                                                   FunctionNameKind Kind,<br class="gmail_msg">                                                   std::string &FunctionName,<br class="gmail_msg">                                                   uint32_t &StartLine) {<br class="gmail_msg">+  if (Kind == FunctionNameKind::None)<br class="gmail_msg">+    return false;<br class="gmail_msg">+<br class="gmail_msg">   // The address may correspond to instruction in some inlined function,<br class="gmail_msg">   // so we have to build the chain of inlined functions and take the<br class="gmail_msg">   // name of the topmost function in it.<br class="gmail_msg">@@ -477,7 +480,7 @@<br class="gmail_msg">   const DWARFDie &DIE = InlinedChain[0];<br class="gmail_msg">   bool FoundResult = false;<br class="gmail_msg">   const char *Name = nullptr;<br class="gmail_msg">-  if (Kind != FunctionNameKind::None && (Name = DIE.getSubroutineName(Kind))) {<br class="gmail_msg">+  if ((Name = DIE.getSubroutineName(Kind))) {<br class="gmail_msg">     FunctionName = Name;<br class="gmail_msg">     FoundResult = true;<br class="gmail_msg">   }<br class="gmail_msg"><br class="gmail_msg"><u class="gmail_msg"></u><u class="gmail_msg"></u></p></blockquote></div></div></div></div></blockquote></div></div>