From 95636e565a18a4f7ad41f0606d28e2e02c167c57 Mon Sep 17 00:00:00 2001
From: Samuel Benzaquen <sbenza@google.com>
Date: Mon, 1 Dec 2014 14:46:14 +0000
Subject: [PATCH] Make the function pointer a template argument instead of a
 runtime value.

Summary:
Speed up the variadic matchers by removing one indirect call.
Making the function pointer a template arguments allows the compiler to
inline the call instead of doing an runtime call by pointer.
Also, optimize the allOf() case to avoid redundant kind checks.
This speeds up our clang-tidy benchmark by ~2%

Reviewers: klimek

Subscribers: klimek, cfe-commits

Differential Revision: http://reviews.llvm.org/D6424

llvm-svn: 223029
---
 clang/lib/ASTMatchers/ASTMatchersInternal.cpp | 59 +++++++++++++++++----------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index ca58d33..2c482e3 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -50,15 +50,15 @@ void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
 
 namespace {
 
+typedef bool (*VariadicOperatorFunction)(
+    const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder,
+    BoundNodesTreeBuilder *Builder, ArrayRef<DynTypedMatcher> InnerMatchers);
+
+template <VariadicOperatorFunction Func>
 class VariadicMatcher : public DynMatcherInterface {
 public:
-  typedef bool (*VariadicOperatorFunction)(
-      const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder,
-      BoundNodesTreeBuilder *Builder, ArrayRef<DynTypedMatcher> InnerMatchers);
-
-  VariadicMatcher(VariadicOperatorFunction Func,
-                  std::vector<DynTypedMatcher> InnerMatchers)
-      : Func(Func), InnerMatchers(std::move(InnerMatchers)) {}
+  VariadicMatcher(std::vector<DynTypedMatcher> InnerMatchers)
+      : InnerMatchers(std::move(InnerMatchers)) {}
 
   bool dynMatches(const ast_type_traits::DynTypedNode &DynNode,
                   ASTMatchFinder *Finder,
@@ -67,7 +67,6 @@ public:
   }
 
 private:
-  VariadicOperatorFunction Func;
   std::vector<DynTypedMatcher> InnerMatchers;
 };
 
@@ -119,29 +118,45 @@ DynTypedMatcher DynTypedMatcher::constructVariadic(
          }) &&
          "SupportedKind must match!");
 
+  auto SupportedKind = InnerMatchers[0].SupportedKind;
   // We must relax the restrict kind here.
   // The different operators might deal differently with a mismatch.
   // Make it the same as SupportedKind, since that is the broadest type we are
   // allowed to accept.
-  auto SupportedKind = InnerMatchers[0].SupportedKind;
-  VariadicMatcher::VariadicOperatorFunction Func;
+  auto RestrictKind = SupportedKind;
+
   switch (Op) {
   case VO_AllOf:
-    Func = AllOfVariadicOperator;
-    break;
+    // In the case of allOf() we must pass all the checks, so making
+    // RestrictKind the most restrictive can save us time. This way we reject
+    // invalid types earlier and we can elide the kind checks inside the
+    // matcher.
+    for (auto &IM : InnerMatchers) {
+      RestrictKind = ast_type_traits::ASTNodeKind::getMostDerivedType(
+          RestrictKind, IM.RestrictKind);
+    }
+    return DynTypedMatcher(
+        SupportedKind, RestrictKind,
+        new VariadicMatcher<AllOfVariadicOperator>(std::move(InnerMatchers)));
+
   case VO_AnyOf:
-    Func = AnyOfVariadicOperator;
-    break;
+    return DynTypedMatcher(
+        SupportedKind, RestrictKind,
+        new VariadicMatcher<AnyOfVariadicOperator>(std::move(InnerMatchers)));
+
   case VO_EachOf:
-    Func = EachOfVariadicOperator;
-    break;
+    return DynTypedMatcher(
+        SupportedKind, RestrictKind,
+        new VariadicMatcher<EachOfVariadicOperator>(std::move(InnerMatchers)));
+
   case VO_UnaryNot:
-    Func = NotUnaryOperator;
-    break;
+    // FIXME: Implement the Not operator to take a single matcher instead of a
+    // vector.
+    return DynTypedMatcher(
+        SupportedKind, RestrictKind,
+        new VariadicMatcher<NotUnaryOperator>(std::move(InnerMatchers)));
   }
-
-  return DynTypedMatcher(SupportedKind, SupportedKind,
-                         new VariadicMatcher(Func, std::move(InnerMatchers)));
+  llvm_unreachable("Invalid Op value.");
 }
 
 DynTypedMatcher DynTypedMatcher::trueMatcher(
@@ -241,7 +256,7 @@ bool AllOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode,
   // matcher combined with each alternative in the second matcher.
   // Thus, we can reuse the same Builder.
   for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {
-    if (!InnerMatcher.matches(DynNode, Finder, Builder))
+    if (!InnerMatcher.matchesNoKindCheck(DynNode, Finder, Builder))
       return false;
   }
   return true;
-- 
2.7.4