<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jan 2, 2014 at 11:36 AM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:akyrtzi@gmail.com" target="_blank">akyrtzi@gmail.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 style="word-wrap:break-word"><div><div class="h5"><br>
<div><div>On Jan 2, 2014, at 11:05 AM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:</div><br><blockquote type="cite"><div dir="ltr">On Fri, Dec 27, 2013 at 5:01 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<div class="gmail_extra"><div class="gmail_quote">
<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 dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">

<div>On Fri, Dec 27, 2013 at 7:46 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</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 dir="ltr">Hi,<div><br></div><div>r151033 and r151037 marked a few dump() methods as LLVM_ATTRIBUTE_USED, and over time this inspired marking several other dump() methods to be marked as such too (see e.g. 178400, 182186, 159790, 173548).</div>



<div><br></div><div>I understand that having the dump() methods available in the debugger is useful, but these annotations prevent the dump() methods from being dead-stripped, and they end up keeping lots of code alive. For example, clang-format depends on ASTDumper, TypePrinter, StmtVisitor and related stuff solely for these dump methods.</div>



<div><br></div><div>Since binaries now get dead-stripped, this leads to measurable bloat: clang-format goes from 1.7 MB to 1.2 MB if I remove the LLVM_ATTRIBUTE_USEDs on the 17 dump methods in include/clang – an almost 30% reduction.</div>



<div><br></div><div>Does it make sense to only mark dump methods as LLVM_ATTRIBUTE_USED if !NDEBUG?</div></div></blockquote><div><br></div></div><div>I think what makes sense is to not have these methods in !NDEBUG. This is the way dump methods are being written in LLVM these days:</div>


<div><br></div><div>#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)<br></div><div>  void some_helper_function_only_used_by_dump() const;</div><div>  void LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED dump() const;</div><div>


#endif</div><div><br></div><div>At least, this is how SROA.cpp does it, and I think it has been suggested to do it consistently in this way.</div><div><br></div><div>My preference would be to:</div><div><br></div><div>a) Make this pattern easier to reproduce where needed.</div>


<div>b) Try to make existing dump code use it more regularly throughout both LLVM and Clang to fix just the problem you've identified.</div></div></div></div>
</blockquote></div><br></div><div class="gmail_extra">Ah, good point bringing up dump() in llvm! Looking through ` ack -C 3 -i dump include/` in llvm, it looks like none of the dump methods in llvm are marked as LLVM_USED – maybe it's reasonable that people who want to call this from gdb during debugging insert that locally manually?</div>

<div class="gmail_extra"><br></div><div class="gmail_extra">Argyrios, would just removing all the LLVM_ATTRIBUTE_USEDs on dump methods in clang be fine with you? That sounds like the simples change, and it would be consistent with what's done in llvm.</div>

</div>
</blockquote></div><div><br></div></div></div>Are you suggesting that one should add ‘LLVM_ATTRIBUTE_USED’ locally and rebuild if s/he wants to dump something in the debugger ?</div></blockquote><div><br></div><div>Yes.</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"><div style="word-wrap:break-word"><div>We should be consistent across LLVM but having the dump methods available at least for !NDEBUG, as Chandler suggested, makes sense for me.</div>
</div></blockquote></div><br></div><div class="gmail_extra">With "available", do you mean "marked LLVM_ATTRIBUTE_USED"? If we remove LLVM_ATTRIBUTE_USED, dump() could be compiled in both release and debug, which is nice in a way (and it's unlikely someone will accidentally add a call to them from live code).</div>
</div>