Make the function pointer a template argument instead of a runtime value.
authorSamuel Benzaquen <sbenza@google.com>
Mon, 1 Dec 2014 14:46:14 +0000 (14:46 +0000)
committerSamuel Benzaquen <sbenza@google.com>
Mon, 1 Dec 2014 14:46:14 +0000 (14:46 +0000)
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

index ca58d33..2c482e3 100644 (file)
@@ -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;