[PATCH] c++: remove optimize_specialization_lookup_p

[Patrick Palka <ppalka@redhat.com>]

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!

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.

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();
+}
-- 
2.38.0.rc2