Message ID | 20221006020226.3629040-1-ppalka@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: remove optimize_specialization_lookup_p | expand |
On 10/5/22 22:02, Patrick Palka wrote: > Roughly speaking, optimize_specialization_lookup_p returns true > for a non-template member function of a class template, e.g. > > template<class T> struct A { int f(); }; > > The idea behind the optimization in question seems to be that if we want > to look up the specialization A<T>::f [with T=int], then we can just do > a name lookup for f in A<int> and avoid having to maintain a spec_entry > for it in the decl_specializations table. > > However, the benefit of this optimization seems questionable because in > order to do the name lookup we need to look up A<T> [with T=int] in the > type_specializations table, which is just as expensive as looking up an > entry in decl_specializations. And according to my experiments using > stdc++.h, range-v3 and libstdc++ tests, the compiler is slightly (<1%) > _faster_ after disabling this optimization! Yeah, probably should have done this when we switched to hash tables. > Additionally, this optimization means that an explicit specialization > won't get recorded in decl_specializations for such a template, which > is a rather arbitrary invariant violation, and apparently breaks the > below modules testcase. > > So since this optimization doesn't seem to give a performance benefit, > complicates the explicit specialization story, and affects modules, this > patch proposes to remove it. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? Patch generated with -w to suppress noisy whitespace changes. OK. > 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 has is tagged as a specialization and > not just a declaration. > * g++.dg/modules/tpl-spec-8_a.H: New test. > * g++.dg/modules/tpl-spec-8_b.C: New test. > --- > gcc/cp/pt.cc | 85 +-------------------- > gcc/testsuite/g++.dg/modules/indirect-3_b.C | 2 +- > gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H | 9 +++ > gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C | 8 ++ > 4 files changed, 22 insertions(+), 82 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-spec-8_b.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index bce2a777922..c079bbd4268 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 <typename T> > - 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,43 +1243,12 @@ 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; > - > - /* 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; > - } > - else > - { > - spec_entry *found; > spec_entry elt; > - spec_hash_table *specializations; > - > elt.tmpl = tmpl; > elt.args = args; > elt.spec = NULL_TREE; > > + spec_hash_table *specializations; > if (DECL_CLASS_TEMPLATE_P (tmpl)) > specializations = type_specializations; > else > @@ -1320,10 +1256,8 @@ retrieve_specialization (tree tmpl, tree args, hashval_t hash) > > if (hash == 0) > hash = spec_hasher::hash (&elt); > - found = specializations->find_with_hash (&elt, hash); > - if (found) > + 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,12 +1521,7 @@ 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 > - { > + spec_entry elt; > elt.tmpl = tmpl; > elt.args = args; > elt.spec = spec; > @@ -1602,12 +1529,11 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, > if (hash == 0) > hash = spec_hasher::hash (&elt); > > - slot = decl_specializations->find_slot_with_hash (&elt, hash, INSERT); > + 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,8 +1630,6 @@ 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<spec_entry> (); > gcc_assert (tmpl && args && spec); > *entry = elt; > @@ -1723,7 +1647,6 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, > 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..5f85556d7d7 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H > @@ -0,0 +1,9 @@ > +// { dg-additional-options -fmodule-header } > +// { dg-module-cmi {} } > + > +template<class T> > +struct A { > + static void f() { T::nonexistent; } > +}; > + > +template<> inline void A<int>::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<int>::f(); > +}
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index bce2a777922..c079bbd4268 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 <typename T> - 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,43 +1243,12 @@ 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; - - /* 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; - } - else - { - spec_entry *found; spec_entry elt; - spec_hash_table *specializations; - elt.tmpl = tmpl; elt.args = args; elt.spec = NULL_TREE; + spec_hash_table *specializations; if (DECL_CLASS_TEMPLATE_P (tmpl)) specializations = type_specializations; else @@ -1320,10 +1256,8 @@ retrieve_specialization (tree tmpl, tree args, hashval_t hash) if (hash == 0) hash = spec_hasher::hash (&elt); - found = specializations->find_with_hash (&elt, hash); - if (found) + 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,12 +1521,7 @@ 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 - { + spec_entry elt; elt.tmpl = tmpl; elt.args = args; elt.spec = spec; @@ -1602,12 +1529,11 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, if (hash == 0) hash = spec_hasher::hash (&elt); - slot = decl_specializations->find_slot_with_hash (&elt, hash, INSERT); + 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,8 +1630,6 @@ 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<spec_entry> (); gcc_assert (tmpl && args && spec); *entry = elt; @@ -1723,7 +1647,6 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, 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..5f85556d7d7 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-spec-8_a.H @@ -0,0 +1,9 @@ +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +template<class T> +struct A { + static void f() { T::nonexistent; } +}; + +template<> inline void A<int>::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<int>::f(); +}