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

[Chandler Carruth]

On Fri, Dec 27, 2013 at 8: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.
>

Oh, and a concrete suggestion of what I think would be easier:

#if LLVM_ENABLE_DUMP
  void some_helper_function_only_used_by_dump() const;
  void LLVM_DUMP_METHOD dump() const;
#endif

I've no better name for "LLVM_DUMP_METHOD" though, it's a terrible name.
Anything you come up with will likely be better. =]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131227/186a98a3/attachment.html>