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

[Argyrios Kyrtzidis]

On Jan 2, 2014, at 4:02 PM, Nico Weber <thakis at chromium.org> wrote:

> On Thu, Jan 2, 2014 at 11:36 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> 
> On Jan 2, 2014, at 11:05 AM, Nico Weber <thakis at chromium.org> wrote:
> 
>> On Fri, Dec 27, 2013 at 5:01 PM, Chandler Carruth <chandlerc at google.com> wrote:
>> 
>> On Fri, Dec 27, 2013 at 7:46 PM, Nico Weber <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.

>  
> We should be consistent across LLVM but having the dump methods available at least for !NDEBUG, as Chandler suggested, makes sense for me.
> 
> 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).

Not sure I’m following, what is the benefit if the dump methods are always stripped, even in debug build ? Do you care about code-bloat in debug builds ?
How about just do your original simple suggestion for a “LLVM_ATTRIBUTE_USED_IN_DEBUG” macro ?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140102/252f66b0/attachment.html>