r223029 - Make the function pointer a template argument instead of a runtime value.

[Samuel Benzaquen]

Author: sbenza
Date: Mon Dec  1 08:46:14 2014
New Revision: 223029

URL: http://llvm.org/viewvc/llvm-project?rev=223029&view=rev
Log:
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

Modified:
    cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=223029&r1=223028&r2=223029&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Mon Dec  1 08:46:14 2014
@@ -50,15 +50,15 @@ void BoundNodesTreeBuilder::visitMatches
 
 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::constru
          }) &&
          "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_typ
   // 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;