From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1888) id 8D9483898380; Thu, 6 Oct 2022 14:05:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8D9483898380 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1665065126; bh=I4hZeYz4lP7I6V9i+wwCuvNwiBIVhMOeYBRBqCCwDHE=; h=From:To:Subject:Date:From; b=HzTw2zDgE/OhZvVlAfY7U33Upw6qEHO66D7BA7lyq9xxViHdtzpFwwpdH/MfDu38u XwIJaJjdVUt7ODPpsZ/FyBTI7CPqrvdQiieuO942cO+6eoGe6e973LbsE4WlX1jMaJ Rs9rC7iC6uKbdu8KmrdUs5xJO5mThWlsRzw4rDO0= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Patrick Palka To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-3134] c++: remove optimize_specialization_lookup_p X-Act-Checkin: gcc X-Git-Author: Patrick Palka X-Git-Refname: refs/heads/master X-Git-Oldrev: 3ec926d36fbf7cb3ff45759471139f3a71d1c4de X-Git-Newrev: 09df0d8b14dda66c5159a1b2cf85b73f26282152 Message-Id: <20221006140526.8D9483898380@sourceware.org> Date: Thu, 6 Oct 2022 14:05:26 +0000 (GMT) List-Id: https://gcc.gnu.org/g:09df0d8b14dda66c5159a1b2cf85b73f26282152 commit r13-3134-g09df0d8b14dda66c5159a1b2cf85b73f26282152 Author: Patrick Palka Date: Thu Oct 6 10:04:52 2022 -0400 c++: remove optimize_specialization_lookup_p Roughly speaking, optimize_specialization_lookup_p returns true for a non-template member function of a class template, e.g. template struct A { int f(); }; The idea behind the optimization guarded by this predicate is that if we want to look up the specialization A::f [with T=int], then we can just do a name lookup for f in A and avoid having to add a spec_entry for f in the decl_specializations table. But the benefit of this optimization seems questionable because in order to do the name lookup we first need to look up A [with T=int] in the type_specializations table, which is as expensive as the decl_specializations lookup we're avoiding. And according to some experiments (using stdc++.h, range-v3 and libstdc++ tests) the compiler is slightly (<1%) _faster_ if we disable this optimization. Additionally, this optimization means we won't record an explicit specialization in decl_specializations for such a template either, which is an unfortunate inconsistency that apparently breaks the below modules testcase. So since this optimization doesn't improve performance, and complicates the explicit specialization story which causes issues with modules, this patch proposes to remove it. gcc/cp/ChangeLog: * pt.cc (optimize_specialization_lookup_p): Remove. (retrieve_specialization): Assume the above returns false and simplify accordingly. (register_specialization): Likewise. gcc/testsuite/ChangeLog: * g++.dg/modules/indirect-3_b.C: Expect that the entity foo::TPL<0>::frob is tagged as a specialization instead of as a declaration. * g++.dg/modules/tpl-spec-8_a.H: New test. * g++.dg/modules/tpl-spec-8_b.C: New test. Diff: --- gcc/cp/pt.cc | 157 +++++++--------------------- gcc/testsuite/g++.dg/modules/indirect-3_b.C | 2 +- gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H | 10 ++ gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C | 8 ++ 4 files changed, 59 insertions(+), 118 deletions(-) diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index bf4ae028eb0..5b9fc588a21 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -1170,39 +1170,6 @@ maybe_process_partial_specialization (tree type) return type; } -/* Returns nonzero if we can optimize the retrieval of specializations - for TMPL, a TEMPLATE_DECL. In particular, for such a template, we - do not use DECL_TEMPLATE_SPECIALIZATIONS at all. */ - -static inline bool -optimize_specialization_lookup_p (tree tmpl) -{ - return (DECL_FUNCTION_TEMPLATE_P (tmpl) - && DECL_CLASS_SCOPE_P (tmpl) - /* DECL_CLASS_SCOPE_P holds of T::f even if T is a template - parameter. */ - && CLASS_TYPE_P (DECL_CONTEXT (tmpl)) - /* The optimized lookup depends on the fact that the - template arguments for the member function template apply - purely to the containing class, which is not true if the - containing class is an explicit or partial - specialization. */ - && !CLASSTYPE_TEMPLATE_SPECIALIZATION (DECL_CONTEXT (tmpl)) - && !DECL_MEMBER_TEMPLATE_P (tmpl) - && !DECL_CONV_FN_P (tmpl) - /* It is possible to have a template that is not a member - template and is not a member of a template class: - - template - struct S { friend A::f(); }; - - Here, the friend function is a template, but the context does - not have template information. The optimized lookup relies - on having ARGS be the template arguments for both the class - and the function template. */ - && !DECL_UNIQUE_FRIEND_P (DECL_TEMPLATE_RESULT (tmpl))); -} - /* Make sure ARGS doesn't use any inappropriate typedefs; we should have gone through coerce_template_parms by now. */ @@ -1276,54 +1243,21 @@ retrieve_specialization (tree tmpl, tree args, hashval_t hash) if (lambda_fn_in_template_p (tmpl)) return NULL_TREE; - if (optimize_specialization_lookup_p (tmpl)) - { - /* The template arguments actually apply to the containing - class. Find the class specialization with those - arguments. */ - tree class_template = CLASSTYPE_TI_TEMPLATE (DECL_CONTEXT (tmpl)); - tree class_specialization - = retrieve_specialization (class_template, args, 0); - if (!class_specialization) - return NULL_TREE; + spec_entry elt; + elt.tmpl = tmpl; + elt.args = args; + elt.spec = NULL_TREE; - /* Find the instance of TMPL. */ - tree fns = get_class_binding (class_specialization, DECL_NAME (tmpl)); - for (ovl_iterator iter (fns); iter; ++iter) - { - tree fn = *iter; - if (tree ti = get_template_info (fn)) - if (TI_TEMPLATE (ti) == tmpl - /* using-declarations can bring in a different - instantiation of tmpl as a member of a different - instantiation of tmpl's class. We don't want those - here. */ - && DECL_CONTEXT (fn) == class_specialization) - return fn; - } - return NULL_TREE; - } + spec_hash_table *specializations; + if (DECL_CLASS_TEMPLATE_P (tmpl)) + specializations = type_specializations; else - { - spec_entry *found; - spec_entry elt; - spec_hash_table *specializations; + specializations = decl_specializations; - elt.tmpl = tmpl; - elt.args = args; - elt.spec = NULL_TREE; - - if (DECL_CLASS_TEMPLATE_P (tmpl)) - specializations = type_specializations; - else - specializations = decl_specializations; - - if (hash == 0) - hash = spec_hasher::hash (&elt); - found = specializations->find_with_hash (&elt, hash); - if (found) - return found->spec; - } + if (hash == 0) + hash = spec_hasher::hash (&elt); + if (spec_entry *found = specializations->find_with_hash (&elt, hash)) + return found->spec; return NULL_TREE; } @@ -1567,8 +1501,6 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, hashval_t hash) { tree fn; - spec_entry **slot = NULL; - spec_entry elt; gcc_assert ((TREE_CODE (tmpl) == TEMPLATE_DECL && DECL_P (spec)) || (TREE_CODE (tmpl) == FIELD_DECL @@ -1589,25 +1521,19 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, instantiation unless and until it is actually needed. */ return spec; - if (optimize_specialization_lookup_p (tmpl)) - /* We don't put these specializations in the hash table, but we might - want to give an error about a mismatch. */ - fn = retrieve_specialization (tmpl, args, 0); - else - { - elt.tmpl = tmpl; - elt.args = args; - elt.spec = spec; + spec_entry elt; + elt.tmpl = tmpl; + elt.args = args; + elt.spec = spec; - if (hash == 0) - hash = spec_hasher::hash (&elt); + if (hash == 0) + hash = spec_hasher::hash (&elt); - slot = decl_specializations->find_slot_with_hash (&elt, hash, INSERT); - if (*slot) - fn = (*slot)->spec; - else - fn = NULL_TREE; - } + spec_entry **slot = decl_specializations->find_slot_with_hash (&elt, hash, INSERT); + if (*slot) + fn = (*slot)->spec; + else + fn = NULL_TREE; /* We can sometimes try to re-register a specialization that we've already got. In particular, regenerate_decl_from_template calls @@ -1704,26 +1630,23 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, && !check_specialization_namespace (tmpl)) DECL_CONTEXT (spec) = DECL_CONTEXT (tmpl); - if (slot != NULL /* !optimize_specialization_lookup_p (tmpl) */) - { - spec_entry *entry = ggc_alloc (); - gcc_assert (tmpl && args && spec); - *entry = elt; - *slot = entry; - if ((TREE_CODE (spec) == FUNCTION_DECL && DECL_NAMESPACE_SCOPE_P (spec) - && PRIMARY_TEMPLATE_P (tmpl) - && DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (tmpl)) == NULL_TREE) - || variable_template_p (tmpl)) - /* If TMPL is a forward declaration of a template function, keep a list - of all specializations in case we need to reassign them to a friend - template later in tsubst_friend_function. - - Also keep a list of all variable template instantiations so that - process_partial_specialization can check whether a later partial - specialization would have used it. */ - DECL_TEMPLATE_INSTANTIATIONS (tmpl) - = tree_cons (args, spec, DECL_TEMPLATE_INSTANTIATIONS (tmpl)); - } + spec_entry *entry = ggc_alloc (); + gcc_assert (tmpl && args && spec); + *entry = elt; + *slot = entry; + if ((TREE_CODE (spec) == FUNCTION_DECL && DECL_NAMESPACE_SCOPE_P (spec) + && PRIMARY_TEMPLATE_P (tmpl) + && DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (tmpl)) == NULL_TREE) + || variable_template_p (tmpl)) + /* If TMPL is a forward declaration of a template function, keep a list + of all specializations in case we need to reassign them to a friend + template later in tsubst_friend_function. + + Also keep a list of all variable template instantiations so that + process_partial_specialization can check whether a later partial + specialization would have used it. */ + DECL_TEMPLATE_INSTANTIATIONS (tmpl) + = tree_cons (args, spec, DECL_TEMPLATE_INSTANTIATIONS (tmpl)); return spec; } diff --git a/gcc/testsuite/g++.dg/modules/indirect-3_b.C b/gcc/testsuite/g++.dg/modules/indirect-3_b.C index 5bdfc1d36a8..038b01ecab7 100644 --- a/gcc/testsuite/g++.dg/modules/indirect-3_b.C +++ b/gcc/testsuite/g++.dg/modules/indirect-3_b.C @@ -23,7 +23,7 @@ namespace bar // { dg-final { scan-lang-dump {Lazily binding '::foo@foo:.::TPL'@'foo' section} module } } // { dg-final { scan-lang-dump {Wrote import:-[0-9]* template_decl:'::foo@foo:.::template TPL@foo:.'@foo} module } } -// { dg-final { scan-lang-dump {Cluster members:\n \[0\]=specialization definition '::foo@foo:.::TPL<0x0>'\n \[1\]=specialization declaration '::foo@foo:.::TPL<0x0>::TPL<0x0>'\n( \[.\]=[^\n]* '\n)* \[.\]=decl definition '::foo@foo:.::TPL<0x0>::frob<0x0>'\n} module } } +// { dg-final { scan-lang-dump {Cluster members:\n \[0\]=specialization definition '::foo@foo:.::TPL<0x0>'\n \[1\]=specialization definition '::foo@foo:.::TPL<0x0>::frob<0x0>'\n \[2\]=specialization declaration '::foo@foo:.::TPL<0x0>::TPL<0x0>'} module } } // { dg-final { scan-lang-dump {Cluster members:\n \[0\]=specialization definition '::foo@foo:.::X@foo:.::frob<0x0>'} module } } // { dg-final { scan-lang-dump {Writing:-[0-9]*'s type spec merge key \(specialization\) type_decl:'::foo@foo:.::TPL<0x0>'} module } } diff --git a/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H b/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H new file mode 100644 index 00000000000..5309130dced --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H @@ -0,0 +1,10 @@ +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +template +struct A { + static void f() { T::nonexistent; } +}; + +template<> +inline void A::f() { } diff --git a/gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C b/gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C new file mode 100644 index 00000000000..f23eb370370 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C @@ -0,0 +1,8 @@ +// { dg-additional-options -fmodules-ts } +// { dg-do link } + +import "tpl-spec-8_a.H"; + +int main() { + A::f(); +}