From: Samuel Benzaquen Date: Mon, 1 Dec 2014 14:46:14 +0000 (+0000) Subject: Make the function pointer a template argument instead of a runtime value. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0a7a42e99096b810ccdb4b21d72b9d711996afd2;p=clang 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@223029 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/ASTMatchers/ASTMatchersInternal.cpp b/lib/ASTMatchers/ASTMatchersInternal.cpp index ca58d330d8..2c482e38dc 100644 --- a/lib/ASTMatchers/ASTMatchersInternal.cpp +++ b/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 InnerMatchers); + +template class VariadicMatcher : public DynMatcherInterface { public: - typedef bool (*VariadicOperatorFunction)( - const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder, - BoundNodesTreeBuilder *Builder, ArrayRef InnerMatchers); - - VariadicMatcher(VariadicOperatorFunction Func, - std::vector InnerMatchers) - : Func(Func), InnerMatchers(std::move(InnerMatchers)) {} + VariadicMatcher(std::vector 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 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(std::move(InnerMatchers))); + case VO_AnyOf: - Func = AnyOfVariadicOperator; - break; + return DynTypedMatcher( + SupportedKind, RestrictKind, + new VariadicMatcher(std::move(InnerMatchers))); + case VO_EachOf: - Func = EachOfVariadicOperator; - break; + return DynTypedMatcher( + SupportedKind, RestrictKind, + new VariadicMatcher(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(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;