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

Alp Toker alp at nuanti.com
Fri Jan 3 01:50:40 PST 2014


HI Nico,

I've attached the fix we had for this in our product branch. As far as I 
can tell the other posts on this thread are missing the problem and just 
patching some of the symptoms.

The bug was that attribute 'used' was applied in public headers instead 
of the function definitions, causing missed dead stripping opportunities 
that were further compounded by inlining.

The commit message says it resolves two issues, "Missing dump() symbols 
embedding LLVM SDK" and "Debug functions included in release" so I'm 
guessing your use case is covered:

Only link dump() functions in debug builds

LLVM_DEBUG_HELPER marks debug functions definitions as 'used' when 
!NDEBUG or
LLVM_ENABLE_DUMP is defined, otherwise leaves them to be dead stripped 
by the
linker.

The macro also unconditionally marks the functions 'noinline' to (1) ensure
they're available to debuggers in debug builds and (2) assist linker dead
stripping in release builds.

The patches haven't yet been pushed upstream because they introduce an 
NDEBUG going against the LLVM 3.5 library-friendly headers goal.

In practice though I've checked and the macro isn't evaluated in any 
header file, plus it's a net LoC win, so I'm OK to land this if it works 
for you.

Alp.




On 28/12/2013 00:46, Nico Weber 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? (If so, I'm thankful for naming suggestions for this new 
> macro – LLVM_ATTRIBUTE_USED_IN_DEBUG comes to mind, but is too long. 
> Also, should this be an LLVM macro or a clang macro?)
>
> Thanks,
> Nico
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
diff --git a/include/clang/AST/Comment.h b/include/clang/AST/Comment.h
index 28849f5..e67ceab 100644
--- a/include/clang/AST/Comment.h
+++ b/include/clang/AST/Comment.h
@@ -194,9 +194,9 @@ public:
 
   const char *getCommentKindName() const;
 
-  LLVM_ATTRIBUTE_USED void dump() const;
-  LLVM_ATTRIBUTE_USED void dumpColor() const;
-  LLVM_ATTRIBUTE_USED void dump(const ASTContext &Context) const;
+  void dump() const;
+  void dumpColor() const;
+  void dump(const ASTContext &Context) const;
   void dump(raw_ostream &OS, const CommandTraits *Traits,
             const SourceManager *SM) const;
 
diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index cf016a7..4d5ff66 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -938,9 +938,9 @@ public:
                          raw_ostream &Out, const PrintingPolicy &Policy,
                          unsigned Indentation = 0);
   // Debuggers don't usually respect default arguments.
-  LLVM_ATTRIBUTE_USED void dump() const;
+  void dump() const;
   // Same as dump(), but forces color printing.
-  LLVM_ATTRIBUTE_USED void dumpColor() const;
+  void dumpColor() const;
   void dump(raw_ostream &Out) const;
 
 private:
@@ -1616,9 +1616,9 @@ public:
   static bool classof(const Decl *D);
   static bool classof(const DeclContext *D) { return true; }
 
-  LLVM_ATTRIBUTE_USED void dumpDeclContext() const;
-  LLVM_ATTRIBUTE_USED void dumpLookups() const;
-  LLVM_ATTRIBUTE_USED void dumpLookups(llvm::raw_ostream &OS) const;
+  void dumpDeclContext() const;
+  void dumpLookups() const;
+  void dumpLookups(llvm::raw_ostream &OS) const;
 
 private:
   void reconcileExternalVisibleStorage();
diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h
index 2396fb7..4303e7b 100644
--- a/include/clang/AST/Stmt.h
+++ b/include/clang/AST/Stmt.h
@@ -371,12 +371,12 @@ public:
 
   /// \brief Dumps the specified AST fragment and all subtrees to
   /// \c llvm::errs().
-  LLVM_ATTRIBUTE_USED void dump() const;
-  LLVM_ATTRIBUTE_USED void dump(SourceManager &SM) const;
+  void dump() const;
+  void dump(SourceManager &SM) const;
   void dump(raw_ostream &OS, SourceManager &SM) const;
 
   /// dumpColor - same as dump(), but forces color highlighting.
-  LLVM_ATTRIBUTE_USED void dumpColor() const;
+  void dumpColor() const;
 
   /// dumpPretty/printPretty - These two methods do a "pretty print" of the AST
   /// back to its original source language syntax.
diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h
index 9e5507b..9711bf7 100644
--- a/include/clang/AST/Type.h
+++ b/include/clang/AST/Type.h
@@ -1797,7 +1797,7 @@ public:
     return CanonicalType;
   }
   CanQualType getCanonicalTypeUnqualified() const; // in CanonicalType.h
-  LLVM_ATTRIBUTE_USED void dump() const;
+  void dump() const;
 
   friend class ASTReader;
   friend class ASTWriter;
diff --git a/include/clang/Analysis/AnalysisContext.h b/include/clang/Analysis/AnalysisContext.h
index b6f183d..b1cd212 100644
--- a/include/clang/Analysis/AnalysisContext.h
+++ b/include/clang/Analysis/AnalysisContext.h
@@ -252,7 +252,7 @@ public:
   virtual void Profile(llvm::FoldingSetNodeID &ID) = 0;
 
   void dumpStack(raw_ostream &OS, StringRef Indent = "") const;
-  LLVM_ATTRIBUTE_USED void dumpStack() const;
+  void dumpStack() const;
 
 public:
   static void ProfileCommon(llvm::FoldingSetNodeID &ID,
diff --git a/include/clang/Basic/SourceLocation.h b/include/clang/Basic/SourceLocation.h
index 10ae07b..1062c53 100644
--- a/include/clang/Basic/SourceLocation.h
+++ b/include/clang/Basic/SourceLocation.h
@@ -172,7 +172,7 @@ public:
   }
 
   void print(raw_ostream &OS, const SourceManager &SM) const;
-  LLVM_ATTRIBUTE_USED std::string printToString(const SourceManager &SM) const;
+  std::string printToString(const SourceManager &SM) const;
   void dump(const SourceManager &SM) const;
 };
 
@@ -331,7 +331,7 @@ public:
   /// \brief Prints information about this FullSourceLoc to stderr.
   ///
   /// This is useful for debugging.
-  LLVM_ATTRIBUTE_USED void dump() const;
+  void dump() const;
 
   friend inline bool
   operator==(const FullSourceLoc &LHS, const FullSourceLoc &RHS) {
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
index 87e850f..f373b18 100644
--- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -423,7 +423,7 @@ public:
     return Result;
   }
 
-  LLVM_ATTRIBUTE_USED void dump() const;
+  void dump() const;
 };
 
 class PathDiagnosticSpotPiece : public PathDiagnosticPiece {
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index cfaf085..d69f50a 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -379,7 +379,7 @@ public:
 
   // For debugging purposes only
   void dump(raw_ostream &Out) const;
-  LLVM_ATTRIBUTE_USED void dump() const;
+  void dump() const;
 };
 
 
diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
index 2f40255..370f1f1 100644
--- a/lib/AST/ASTDumper.cpp
+++ b/lib/AST/ASTDumper.cpp
@@ -2095,27 +2095,25 @@ void ASTDumper::visitVerbatimLineComment(const VerbatimLineComment *C) {
 // Decl method implementations
 //===----------------------------------------------------------------------===//
 
-void Decl::dump() const {
-  dump(llvm::errs());
-}
+LLVM_DEBUG_HELPER void Decl::dump() const { dump(llvm::errs()); }
 
-void Decl::dump(raw_ostream &OS) const {
+LLVM_DEBUG_HELPER void Decl::dump(raw_ostream &OS) const {
   ASTDumper P(OS, &getASTContext().getCommentCommandTraits(),
               &getASTContext().getSourceManager());
   P.dumpDecl(this);
 }
 
-void Decl::dumpColor() const {
+LLVM_DEBUG_HELPER void Decl::dumpColor() const {
   ASTDumper P(llvm::errs(), &getASTContext().getCommentCommandTraits(),
               &getASTContext().getSourceManager(), /*ShowColors*/true);
   P.dumpDecl(this);
 }
 
-void DeclContext::dumpLookups() const {
+LLVM_DEBUG_HELPER void DeclContext::dumpLookups() const {
   dumpLookups(llvm::errs());
 }
 
-void DeclContext::dumpLookups(raw_ostream &OS) const {
+LLVM_DEBUG_HELPER void DeclContext::dumpLookups(raw_ostream &OS) const {
   const DeclContext *DC = this;
   while (!DC->isTranslationUnit())
     DC = DC->getParent();
@@ -2128,21 +2126,21 @@ void DeclContext::dumpLookups(raw_ostream &OS) const {
 // Stmt method implementations
 //===----------------------------------------------------------------------===//
 
-void Stmt::dump(SourceManager &SM) const {
+LLVM_DEBUG_HELPER void Stmt::dump(SourceManager &SM) const {
   dump(llvm::errs(), SM);
 }
 
-void Stmt::dump(raw_ostream &OS, SourceManager &SM) const {
+LLVM_DEBUG_HELPER void Stmt::dump(raw_ostream &OS, SourceManager &SM) const {
   ASTDumper P(OS, 0, &SM);
   P.dumpStmt(this);
 }
 
-void Stmt::dump() const {
+LLVM_DEBUG_HELPER void Stmt::dump() const {
   ASTDumper P(llvm::errs(), 0, 0);
   P.dumpStmt(this);
 }
 
-void Stmt::dumpColor() const {
+LLVM_DEBUG_HELPER void Stmt::dumpColor() const {
   ASTDumper P(llvm::errs(), 0, 0, /*ShowColors*/true);
   P.dumpStmt(this);
 }
@@ -2151,11 +2149,9 @@ void Stmt::dumpColor() const {
 // Comment method implementations
 //===----------------------------------------------------------------------===//
 
-void Comment::dump() const {
-  dump(llvm::errs(), 0, 0);
-}
+LLVM_DEBUG_HELPER void Comment::dump() const { dump(llvm::errs(), 0, 0); }
 
-void Comment::dump(const ASTContext &Context) const {
+LLVM_DEBUG_HELPER void Comment::dump(const ASTContext &Context) const {
   dump(llvm::errs(), &Context.getCommentCommandTraits(),
        &Context.getSourceManager());
 }
@@ -2167,7 +2163,7 @@ void Comment::dump(raw_ostream &OS, const CommandTraits *Traits,
   D.dumpFullComment(FC);
 }
 
-void Comment::dumpColor() const {
+LLVM_DEBUG_HELPER void Comment::dumpColor() const {
   const FullComment *FC = dyn_cast<FullComment>(this);
   ASTDumper D(llvm::errs(), 0, 0, /*ShowColors*/true);
   D.dumpFullComment(FC);
diff --git a/lib/AST/DeclPrinter.cpp b/lib/AST/DeclPrinter.cpp
index 26e2cda..9f65b99 100644
--- a/lib/AST/DeclPrinter.cpp
+++ b/lib/AST/DeclPrinter.cpp
@@ -167,7 +167,7 @@ void Decl::printGroup(Decl** Begin, unsigned NumDecls,
   }
 }
 
-void DeclContext::dumpDeclContext() const {
+LLVM_DEBUG_HELPER void DeclContext::dumpDeclContext() const {
   // Get the translation unit
   const DeclContext *DC = this;
   while (!DC->isTranslationUnit())
diff --git a/lib/AST/TypePrinter.cpp b/lib/AST/TypePrinter.cpp
index 082d027..1315eef 100644
--- a/lib/AST/TypePrinter.cpp
+++ b/lib/AST/TypePrinter.cpp
@@ -1423,13 +1423,10 @@ void QualType::dump(const char *msg) const {
   print(llvm::errs(), PrintingPolicy(LO), "identifier");
   llvm::errs() << '\n';
 }
-void QualType::dump() const {
-  dump(0);
-}
 
-void Type::dump() const {
-  QualType(this, 0).dump();
-}
+LLVM_DEBUG_HELPER void QualType::dump() const { dump(0); }
+
+LLVM_DEBUG_HELPER void Type::dump() const { QualType(this, 0).dump(); }
 
 std::string Qualifiers::getAsString() const {
   LangOptions LO;
diff --git a/lib/Analysis/AnalysisDeclContext.cpp b/lib/Analysis/AnalysisDeclContext.cpp
index 465f0c3..4d6515d 100644
--- a/lib/Analysis/AnalysisDeclContext.cpp
+++ b/lib/Analysis/AnalysisDeclContext.cpp
@@ -435,7 +435,7 @@ void LocationContext::dumpStack(raw_ostream &OS, StringRef Indent) const {
   }
 }
 
-void LocationContext::dumpStack() const {
+LLVM_DEBUG_HELPER void LocationContext::dumpStack() const {
   dumpStack(llvm::errs());
 }
 
diff --git a/lib/Basic/SourceLocation.cpp b/lib/Basic/SourceLocation.cpp
index 1822091..6aa2014 100644
--- a/lib/Basic/SourceLocation.cpp
+++ b/lib/Basic/SourceLocation.cpp
@@ -61,14 +61,15 @@ void SourceLocation::print(raw_ostream &OS, const SourceManager &SM)const{
   OS << '>';
 }
 
-std::string SourceLocation::printToString(const SourceManager &SM) const {
+LLVM_DEBUG_HELPER std::string
+SourceLocation::printToString(const SourceManager &SM) const {
   std::string S;
   llvm::raw_string_ostream OS(S);
   print(OS, SM);
   return OS.str();
 }
 
-void SourceLocation::dump(const SourceManager &SM) const {
+LLVM_DEBUG_HELPER void SourceLocation::dump(const SourceManager &SM) const {
   print(llvm::errs(), SM);
 }
 
@@ -122,7 +123,7 @@ bool FullSourceLoc::isBeforeInTranslationUnitThan(SourceLocation Loc) const {
   return SrcMgr->isBeforeInTranslationUnit(*this, Loc);
 }
 
-void FullSourceLoc::dump() const {
+LLVM_DEBUG_HELPER void FullSourceLoc::dump() const {
   SourceLocation::dump(*SrcMgr);
 }
 
diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index 1940fa7..71db3b2 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -3501,8 +3501,7 @@ BugType *BugReporter::getBugTypeForName(StringRef name,
   return BT;
 }
 
-
-void PathPieces::dump() const {
+LLVM_DEBUG_HELPER void PathPieces::dump() const {
   unsigned index = 0;
   for (PathPieces::const_iterator I = begin(), E = end(); I != E; ++I) {
     llvm::errs() << "[" << index++ << "]  ";
diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index a3b34f4..cb6a37b 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -209,9 +209,7 @@ SVal CallEvent::getReturnValue() const {
   return getSVal(E);
 }
 
-void CallEvent::dump() const {
-  dump(llvm::errs());
-}
+LLVM_DEBUG_HELPER void CallEvent::dump() const { dump(llvm::errs()); }
 
 void CallEvent::dump(raw_ostream &Out) const {
   ASTContext &Ctx = getState()->getStateManager().getContext();
-------------- next part --------------
diff --git a/include/llvm/Support/Compiler.h b/include/llvm/Support/Compiler.h
index d4f55e1..251dcc2 100644
--- a/include/llvm/Support/Compiler.h
+++ b/include/llvm/Support/Compiler.h
@@ -434,4 +434,13 @@
 #define LLVM_HAS_INITIALIZER_LISTS 0
 #endif
 
+/// \brief Mark debug helper function definitions like dump() that should not be
+/// stripped from debug builds.
+// FIXME: Move this to a private config.h as it's not usable in public headers.
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+#define LLVM_DEBUG_HELPER LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED
+#else
+#define LLVM_DEBUG_HELPER LLVM_ATTRIBUTE_NOINLINE
+#endif
+
 #endif
diff --git a/lib/CodeGen/MachineBlockPlacement.cpp b/lib/CodeGen/MachineBlockPlacement.cpp
index f297c5f..e69fa5a 100644
--- a/lib/CodeGen/MachineBlockPlacement.cpp
+++ b/lib/CodeGen/MachineBlockPlacement.cpp
@@ -152,7 +152,7 @@ public:
 
 #ifndef NDEBUG
   /// \brief Dump the blocks in this chain.
-  void dump() LLVM_ATTRIBUTE_USED {
+  LLVM_DEBUG_HELPER void dump() {
     for (iterator I = begin(), E = end(); I != E; ++I)
       (*I)->dump();
   }
diff --git a/lib/Transforms/Scalar/SROA.cpp b/lib/Transforms/Scalar/SROA.cpp
index 9f3fc83..89bf2ad 100644
--- a/lib/Transforms/Scalar/SROA.cpp
+++ b/lib/Transforms/Scalar/SROA.cpp
@@ -244,8 +244,8 @@ public:
   void printUse(raw_ostream &OS, const_iterator I,
                 StringRef Indent = "  ") const;
   void print(raw_ostream &OS) const;
-  void LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED dump(const_iterator I) const;
-  void LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED dump() const;
+  void dump(const_iterator I) const;
+  void dump() const;
 #endif
 
 private:
@@ -712,8 +712,10 @@ void AllocaSlices::print(raw_ostream &OS) const {
     print(OS, I);
 }
 
-void AllocaSlices::dump(const_iterator I) const { print(dbgs(), I); }
-void AllocaSlices::dump() const { print(dbgs()); }
+LLVM_DEBUG_HELPER void AllocaSlices::dump(const_iterator I) const {
+  print(dbgs(), I);
+}
+LLVM_DEBUG_HELPER void AllocaSlices::dump() const { print(dbgs()); }
 
 #endif // !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 


More information about the cfe-dev mailing list