[clang] [ASTMatchers] Reduce per-matcher, per-node traversal kind overhead (PR #228960)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Nico Weber (nico)

<details>
<summary>Changes</summary>

Give getParentMapContext() and traverseIgnored() inline fast paths, and only 
check if the node is ignored in matchWithFilter() for expressions.

clang-tidy with LLVM's .clang-tidy config on five LLVM files (clang's 
ASTContext.cpp, CGExpr.cpp, ParseDecl.cpp, SemaOverload.cpp, and 
LoopVectorize.cpp), sum of two runs each:

    instructions: 436.4e9 =&gt; 405.8e9, -7.0%
    user time: 32.2 s =&gt; 30.2 s, -6.2%

No behavior change.

---
Full diff: https://github.com/llvm/llvm-project/pull/228960.diff


5 Files Affected:

- (modified) clang/include/clang/AST/ASTContext.h (+6-1) 
- (modified) clang/include/clang/AST/ParentMapContext.h (+8-1) 
- (modified) clang/lib/AST/ASTContext.cpp (+3-3) 
- (modified) clang/lib/AST/ParentMapContext.cpp (+2-1) 
- (modified) clang/lib/ASTMatchers/ASTMatchFinder.cpp (+3-1) 


``````````diff
diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 9d9978e79be51..36aa2a4672b48 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -842,6 +842,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
   clang::PrintingPolicy PrintingPolicy;
   mutable std::unique_ptr<interp::Context> InterpContext;
   std::unique_ptr<ParentMapContext> ParentMapCtx;
+  ParentMapContext &createParentMapContext();
 
   /// Keeps track of the deallocated DeclListNodes for future reuse.
   DeclListNode *ListNodeFreeList = nullptr;
@@ -873,7 +874,11 @@ class ASTContext : public RefCountedBase<ASTContext> {
   };
 
   /// Returns the dynamic AST node parent map context.
-  ParentMapContext &getParentMapContext();
+  ParentMapContext &getParentMapContext() {
+    if (LLVM_LIKELY(ParentMapCtx))
+      return *ParentMapCtx;
+    return createParentMapContext();
+  }
 
   // A traversal scope limits the parts of the AST visible to certain analyses.
   // RecursiveASTVisitor only visits specified children of TranslationUnitDecl.
diff --git a/clang/include/clang/AST/ParentMapContext.h 
b/clang/include/clang/AST/ParentMapContext.h
index 6f79038627d9e..1397f6acbf775 100644
--- a/clang/include/clang/AST/ParentMapContext.h
+++ b/clang/include/clang/AST/ParentMapContext.h
@@ -62,11 +62,18 @@ class ParentMapContext {
 
   const Expr *traverseIgnored(const Expr *E) const;
   Expr *traverseIgnored(Expr *E) const;
-  DynTypedNode traverseIgnored(const DynTypedNode &N) const;
+  DynTypedNode traverseIgnored(const DynTypedNode &N) const {
+    // Fast path: Nothing is ignored in the default traversal mode TK_AsIs.
+    if (Traversal == TK_AsIs)
+      return N;
+    return traverseIgnoredSlow(N);
+  }
 
   class ParentMap;
 
 private:
+  DynTypedNode traverseIgnoredSlow(const DynTypedNode &N) const;
+
   ASTContext &ASTCtx;
   TraversalKind Traversal = TK_AsIs;
   std::unique_ptr<ParentMap> Parents;
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 851c91e076990..59d7a8638c778 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -907,9 +907,9 @@ interp::Context &ASTContext::getInterpContext() const {
   return *InterpContext;
 }
 
-ParentMapContext &ASTContext::getParentMapContext() {
-  if (!ParentMapCtx)
-    ParentMapCtx.reset(new ParentMapContext(*this));
+ParentMapContext &ASTContext::createParentMapContext() {
+  assert(!ParentMapCtx && "already have a ParentMapContext");
+  ParentMapCtx.reset(new ParentMapContext(*this));
   return *ParentMapCtx;
 }
 
diff --git a/clang/lib/AST/ParentMapContext.cpp 
b/clang/lib/AST/ParentMapContext.cpp
index 7138dffb46e19..09c94ed8f6522 100644
--- a/clang/lib/AST/ParentMapContext.cpp
+++ b/clang/lib/AST/ParentMapContext.cpp
@@ -327,7 +327,8 @@ Expr *ParentMapContext::traverseIgnored(Expr *E) const {
   llvm_unreachable("Invalid Traversal type!");
 }
 
-DynTypedNode ParentMapContext::traverseIgnored(const DynTypedNode &N) const {
+DynTypedNode
+ParentMapContext::traverseIgnoredSlow(const DynTypedNode &N) const {
   if (const auto *E = N.get<Expr>()) {
     return DynTypedNode::create(*traverseIgnored(E));
   }
diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp 
b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index d2c024abfe02e..9395b79a7622a 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -1049,13 +1049,15 @@ class MatchASTVisitor : public 
RecursiveASTVisitor<MatchASTVisitor>,
     const bool EnableCheckProfiling = Options.CheckProfiling.has_value();
     TimeBucketRegion Timer;
     auto &Matchers = this->Matchers->DeclOrStmt;
+    // traverseIgnored() only ever replaces expressions.
+    const bool IsExpr = DynNode.get<Expr>() != nullptr;
     for (unsigned short I : Filter) {
       auto &MP = Matchers[I];
       if (EnableCheckProfiling)
         Timer.setBucket(&TimeByBucket[MP.second->getID()]);
       BoundNodesTreeBuilder Builder;
 
-      {
+      if (IsExpr) {
         TraversalKindScope RAII(getASTContext(), MP.first.getTraversalKind());
         if (getASTContext().getParentMapContext().traverseIgnored(DynNode) !=
             DynNode)

``````````

</details>


https://github.com/llvm/llvm-project/pull/228960
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits