r182799 - Check the linkage cache at every recursive step.

Rafael Espindola rafael.espindola at gmail.com
Tue May 28 12:43:11 PDT 2013


Author: rafael
Date: Tue May 28 14:43:11 2013
New Revision: 182799

URL: http://llvm.org/viewvc/llvm-project?rev=182799&view=rev
Log:
Check the linkage cache at every recursive step.

Before this patch the linkage cache was only used by the entry level function
(getLinkage). The function that does the actual computation (getLVForDecl),
never looked at it.

This means that we would not reuse an entry in the cache when getLVForDecl did
a recursive call. This patch fixes that by adding another computation enum
value for when we don't care about the linkage at all and having getLVForDecl
check the cache in that case.

When running "clang -cc1" over SemaExpr.ii this brings the number of linkage
computations from 93749 to 58426. When running "clang -cc1 -emit-llvm -O3" it
goes from 198708 to 161444.

For SemaExpr.ii at least linkage computation is a small enough percentage of
the work that the time difference was in the noise.

When asserts are enabled this patch also causes clang to check the linkage
cache even on recursive calls.

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/include/clang/AST/DeclBase.h
    cfe/trunk/lib/AST/Decl.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=182799&r1=182798&r2=182799&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Tue May 28 14:43:11 2013
@@ -110,7 +110,6 @@ class NamedDecl : public Decl {
 
 private:
   NamedDecl *getUnderlyingDeclImpl();
-  void verifyLinkage() const;
 
 protected:
   NamedDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName N)

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=182799&r1=182798&r2=182799&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Tue May 28 14:43:11 2013
@@ -32,6 +32,7 @@ class DeclarationName;
 class DependentDiagnostic;
 class EnumDecl;
 class FunctionDecl;
+class LinkageComputer;
 class LinkageSpecDecl;
 class Module;
 class NamedDecl;
@@ -292,6 +293,7 @@ protected:
   friend class ASTDeclWriter;
   friend class ASTDeclReader;
   friend class ASTReader;
+  friend class LinkageComputer;
 
 private:
   void CheckAccessDeclContext() const;

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=182799&r1=182798&r2=182799&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Tue May 28 14:43:11 2013
@@ -85,6 +85,7 @@ using namespace clang;
 // and settings from the immediate context.
 
 const unsigned IgnoreExplicitVisibilityBit = 2;
+const unsigned IgnoreAllVisibilityBit = 4;
 
 /// Kinds of LV computation.  The linkage side of the computation is
 /// always the same, but different things can change how visibility is
@@ -108,7 +109,11 @@ enum LVComputationKind {
   /// Do an LV computation for, ultimately, a non-type declaration
   /// that already has some sort of explicit visibility.  Visibility
   /// may only be restricted by the visibility of template arguments.
-  LVForExplicitValue = (LVForValue | IgnoreExplicitVisibilityBit)
+  LVForExplicitValue = (LVForValue | IgnoreExplicitVisibilityBit),
+
+  /// Do an LV computation when we only care about the linkage.
+  LVForLinkageOnly =
+      LVForValue | IgnoreExplicitVisibilityBit | IgnoreAllVisibilityBit
 };
 
 /// Does this computation kind permit us to consider additional
@@ -391,6 +396,8 @@ static bool hasDirectVisibilityAttribute
     if (D->hasAttr<VisibilityAttr>())
       return true;
     return false;
+  case LVForLinkageOnly:
+    return false;
   }
   llvm_unreachable("bad visibility computation kind");
 }
@@ -899,59 +906,15 @@ bool NamedDecl::isLinkageValid() const {
 }
 
 Linkage NamedDecl::getLinkageInternal() const {
-  if (hasCachedLinkage())
-    return getCachedLinkage();
-
   // We don't care about visibility here, so ask for the cheapest
   // possible visibility analysis.
-  setCachedLinkage(getLVForDecl(this, LVForExplicitValue).getLinkage());
-
-#ifndef NDEBUG
-  verifyLinkage();
-#endif
-
-  return getCachedLinkage();
+  return getLVForDecl(this, LVForLinkageOnly).getLinkage();
 }
 
 LinkageInfo NamedDecl::getLinkageAndVisibility() const {
   LVComputationKind computation =
     (usesTypeVisibility(this) ? LVForType : LVForValue);
-  LinkageInfo LI = getLVForDecl(this, computation);
-  if (hasCachedLinkage()) {
-    assert(getCachedLinkage() == LI.getLinkage());
-    return LI;
-  }
-  setCachedLinkage(LI.getLinkage());
-
-#ifndef NDEBUG
-  verifyLinkage();
-#endif
-
-  return LI;
-}
-
-void NamedDecl::verifyLinkage() const {
-  // In C (because of gnu inline) and in c++ with microsoft extensions an
-  // static can follow an extern, so we can have two decls with different
-  // linkages.
-  const LangOptions &Opts = getASTContext().getLangOpts();
-  if (!Opts.CPlusPlus || Opts.MicrosoftExt)
-    return;
-
-  // We have just computed the linkage for this decl. By induction we know
-  // that all other computed linkages match, check that the one we just computed
-  // also does.
-  NamedDecl *D = NULL;
-  for (redecl_iterator I = redecls_begin(), E = redecls_end(); I != E; ++I) {
-    NamedDecl *T = cast<NamedDecl>(*I);
-    if (T == this)
-      continue;
-    if (T->hasCachedLinkage()) {
-      D = T;
-      break;
-    }
-  }
-  assert(!D || D->getCachedLinkage() == getCachedLinkage());
+  return getLVForDecl(this, computation);
 }
 
 Optional<Visibility>
@@ -1077,8 +1040,8 @@ static LinkageInfo getLVForLocalDecl(con
                      LV.isVisibilityExplicit());
 }
 
-static LinkageInfo getLVForDecl(const NamedDecl *D,
-                                LVComputationKind computation) {
+static LinkageInfo computeLVForDecl(const NamedDecl *D,
+                                    LVComputationKind computation) {
   // Objective-C: treat all Objective-C declarations as having external
   // linkage.
   switch (D->getKind()) {
@@ -1159,6 +1122,57 @@ static LinkageInfo getLVForDecl(const Na
   return LinkageInfo::none();
 }
 
+namespace clang {
+class LinkageComputer {
+public:
+  static LinkageInfo getLVForDecl(const NamedDecl *D,
+                                  LVComputationKind computation) {
+    if (computation == LVForLinkageOnly && D->hasCachedLinkage())
+      return LinkageInfo(D->getCachedLinkage(), DefaultVisibility, false);
+
+    LinkageInfo LV = computeLVForDecl(D, computation);
+    if (D->hasCachedLinkage())
+      assert(D->getCachedLinkage() == LV.getLinkage());
+
+    D->setCachedLinkage(LV.getLinkage());
+
+#ifndef NDEBUG
+    // In C (because of gnu inline) and in c++ with microsoft extensions an
+    // static can follow an extern, so we can have two decls with different
+    // linkages.
+    const LangOptions &Opts = D->getASTContext().getLangOpts();
+    if (!Opts.CPlusPlus || Opts.MicrosoftExt)
+      return LV;
+
+    // We have just computed the linkage for this decl. By induction we know
+    // that all other computed linkages match, check that the one we just
+    // computed
+    // also does.
+    NamedDecl *Old = NULL;
+    for (NamedDecl::redecl_iterator I = D->redecls_begin(),
+                                    E = D->redecls_end();
+         I != E; ++I) {
+      NamedDecl *T = cast<NamedDecl>(*I);
+      if (T == D)
+        continue;
+      if (T->hasCachedLinkage()) {
+        Old = T;
+        break;
+      }
+    }
+    assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
+#endif
+
+    return LV;
+  }
+};
+}
+
+static LinkageInfo getLVForDecl(const NamedDecl *D,
+                                LVComputationKind computation) {
+  return clang::LinkageComputer::getLVForDecl(D, computation);
+}
+
 std::string NamedDecl::getQualifiedNameAsString() const {
   return getQualifiedNameAsString(getASTContext().getPrintingPolicy());
 }





More information about the cfe-commits mailing list