[cfe-dev] Should dump methods be LLVM_ATTRIBUTE_USED only in debug builds?

[Philip Reames]

On 1/2/14 5:10 PM, Argyrios Kyrtzidis wrote:
> On Jan 2, 2014, at 4:02 PM, Nico Weber <thakis at chromium.org 
> <mailto:thakis at chromium.org>> wrote:
>
>> On Thu, Jan 2, 2014 at 11:36 AM, Argyrios Kyrtzidis 
>> <akyrtzi at gmail.com <mailto:akyrtzi at gmail.com>> wrote:
>>
>>
>>     On Jan 2, 2014, at 11:05 AM, Nico Weber <thakis at chromium.org
>>     <mailto:thakis at chromium.org>> wrote:
>>
>>>     On Fri, Dec 27, 2013 at 5:01 PM, Chandler Carruth
>>>     <chandlerc at google.com <mailto:chandlerc at google.com>> wrote:
>>>
>>>
>>>         On Fri, Dec 27, 2013 at 7:46 PM, Nico Weber
>>>         <thakis at chromium.org <mailto:thakis at chromium.org>> wrote:
>>>
>>>             Hi,
>>>
>>>             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).
>>>
>>>             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.
>>>
>>>             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.
>>>
>>>             Does it make sense to only mark dump methods
>>>             as LLVM_ATTRIBUTE_USED if !NDEBUG?
>>>
>>>
>>>         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:
>>>
>>>         #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
>>>           void some_helper_function_only_used_by_dump() const;
>>>           void LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED dump() const;
>>>         #endif
>>>
>>>         At least, this is how SROA.cpp does it, and I think it has
>>>         been suggested to do it consistently in this way.
>>>
>>>         My preference would be to:
>>>
>>>         a) Make this pattern easier to reproduce where needed.
>>>         b) Try to make existing dump code use it more regularly
>>>         throughout both LLVM and Clang to fix just the problem
>>>         you've identified.
>>>
>>>
>>>     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?
>>>
>>>     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.
>>
>>     Are you suggesting that one should add 'LLVM_ATTRIBUTE_USED'
>>     locally and rebuild if s/he wants to dump something in the debugger ?
>>
>>
>> Yes.
>
> I don't like this at all, I use the dump methods in the debugger all 
> the time, I don't want to perpetually have local changes in order to 
> use them.
I very strongly second this.  I would be in complete opposition to any 
proposal which has the effect of worsening the out-of-box debugging 
experience.

I understand and support your desire to reduce size in Release builds.  
Can you spell out a case for such reduction in Debug builds?  Maybe 
there's something I'm missing here.

Philip

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140102/a7e93323/attachment.html>