Patchwork C++ PATCHes for c++/48481 (overload resolution memory hog)

login
register
mail settings
Submitter Jason Merrill
Date April 8, 2011, 6:08 a.m.
Message ID <4D9EA66A.8030206@redhat.com>
Download mbox | patch
Permalink /patch/90264/
State New
Headers show

Comments

Jason Merrill - April 8, 2011, 6:08 a.m.
48481 involves a huge function involving calls to functions taking class 
arguments.  Detailed memory statistics pointed to three primary sources 
of garbage:

1) BASELINKs from lookup of constructors for considering a user-defined 
conversion sequence.  We can just avoid creating these in the first 
place, since we always find constructors in the type itself.

2) VEC(tree)s created in build_user_type_conversion_1 and then not 
freed.  So we release the vec if we don't actually end up needing it.

3) OVERLOADs from argument-dependent lookup.  We allocate some of these 
whenever we do argument-dependent lookup, which is pretty wasteful in 
the common case that unqualified lookup finds all the interesting 
functions, because we end up looking at every function twice -- once 
from unqualified lookup, once from argument-dependent.  We used to try 
to avoid this in the common case, but stopped to get 2-phase name lookup 
right.  It seems to me that we could still do the optimization in the 
non-template case, but that will be a later patch.

This patch marks the OVERLOADs as coming from arg-dependent lookup so 
that we can free them again once we're done with the call.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit fe4fd71bfe5c968261c332c2792047dcfe36da3c
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Apr 6 15:09:09 2011 -0400

    	PR c++/48481
    	* call.c (build_user_type_conversion_1): Use lookup_fnfields_slot.
    	Release unused vector.

Patch

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index f283bd1..096fbbc 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -3249,7 +3249,9 @@  build_user_type_conversion_1 (tree totype, tree expr, int flags)
 	      || !DERIVED_FROM_P (totype, fromtype));
 
   if (MAYBE_CLASS_TYPE_P (totype))
-    ctors = lookup_fnfields (totype, complete_ctor_identifier, 0);
+    /* Use lookup_fnfields_slot instead of lookup_fnfields to avoid
+       creating a garbage BASELINK; constructors can't be inherited.  */
+    ctors = lookup_fnfields_slot (totype, complete_ctor_identifier);
 
   if (MAYBE_CLASS_TYPE_P (fromtype))
     {
@@ -3281,7 +3283,6 @@  build_user_type_conversion_1 (tree totype, tree expr, int flags)
   if (ctors)
     {
       int ctorflags = flags;
-      ctors = BASELINK_FUNCTIONS (ctors);
 
       first_arg = build_int_cst (build_pointer_type (totype), 0);
 
@@ -3389,7 +3390,11 @@  build_user_type_conversion_1 (tree totype, tree expr, int flags)
 
   candidates = splice_viable (candidates, pedantic, &any_viable_p);
   if (!any_viable_p)
-    return NULL;
+    {
+      if (args)
+	release_tree_vector (args);
+      return NULL;
+    }
 
   cand = tourney (candidates);
   if (cand == 0)

commit 88c1fe424f19e08870fb6d2b5e54eee9d354095f
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Apr 7 11:48:09 2011 -0400

    	PR c++/48481
    	* cp-tree.h (OVL_ARG_DEPENDENT): New.
    	* name-lookup.c (add_function): Set it.
    	* semantics.c (finish_call_expr): Free OVERLOADs if it's set.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ea251a8..885b31c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -317,6 +317,9 @@  typedef struct ptrmem_cst * ptrmem_cst_t;
    This is not to confuse with being used somewhere, which
    is not important for this node.  */
 #define OVL_USED(NODE)		TREE_USED (NODE)
+/* If set, this OVERLOAD was created for argument-dependent lookup
+   and can be freed afterward.  */
+#define OVL_ARG_DEPENDENT(NODE) TREE_LANG_FLAG_0 (OVERLOAD_CHECK (NODE))
 
 struct GTY(()) tree_overload {
   struct tree_common common;
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 18e3441..696a8f5 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4725,7 +4725,11 @@  add_function (struct arg_lookup *k, tree fn)
   else if (fn == k->functions)
     ;
   else
-    k->functions = build_overload (fn, k->functions);
+    {
+      k->functions = build_overload (fn, k->functions);
+      if (TREE_CODE (k->functions) == OVERLOAD)
+	OVL_ARG_DEPENDENT (k->functions) = true;
+    }
 
   return false;
 }
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 30175af..2184a53 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2160,6 +2160,25 @@  finish_call_expr (tree fn, VEC(tree,gc) **args, bool disallow_virtual,
       result = convert_from_reference (result);
     }
 
+  if (koenig_p)
+    {
+      /* Free garbage OVERLOADs from arg-dependent lookup.  */
+      tree next = NULL_TREE;
+      for (fn = orig_fn;
+	   fn && TREE_CODE (fn) == OVERLOAD && OVL_ARG_DEPENDENT (fn);
+	   fn = next)
+	{
+	  if (processing_template_decl)
+	    /* In a template, we'll re-use them at instantiation time.  */
+	    OVL_ARG_DEPENDENT (fn) = false;
+	  else
+	    {
+	      next = OVL_CHAIN (fn);
+	      ggc_free (fn);
+	    }
+	}
+    }
+
   return result;
 }
 

commit e57283fb9a55c107cc924ed9c3afa1866adcbe5e
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Apr 7 12:05:45 2011 -0400

    	PR c++/48481
    	* tree.c (build_overload): Allow an unwrapped FUNCTION_DECL
    	at the end of the chain.
    	* pt.c (dependent_template_p): Use OVL_CURRENT/NEXT.
    	(iterative_hash_template_arg): Likewise.

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 696a8f5..2136df6 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5139,8 +5139,8 @@  arg_assoc (struct arg_lookup *k, tree n)
     }
   else if (TREE_CODE (n) == OVERLOAD)
     {
-      for (; n; n = OVL_CHAIN (n))
-	if (arg_assoc_type (k, TREE_TYPE (OVL_FUNCTION (n))))
+      for (; n; n = OVL_NEXT (n))
+	if (arg_assoc_type (k, TREE_TYPE (OVL_CURRENT (n))))
 	  return true;
     }
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 4edd404..86274e9 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -1516,8 +1516,8 @@  iterative_hash_template_arg (tree arg, hashval_t val)
       return val;
 
     case OVERLOAD:
-      for (; arg; arg = OVL_CHAIN (arg))
-	val = iterative_hash_template_arg (OVL_FUNCTION (arg), val);
+      for (; arg; arg = OVL_NEXT (arg))
+	val = iterative_hash_template_arg (OVL_CURRENT (arg), val);
       return val;
 
     case CONSTRUCTOR:
@@ -18591,9 +18591,9 @@  dependent_template_p (tree tmpl)
     {
       while (tmpl)
 	{
-	  if (dependent_template_p (OVL_FUNCTION (tmpl)))
+	  if (dependent_template_p (OVL_CURRENT (tmpl)))
 	    return true;
-	  tmpl = OVL_CHAIN (tmpl);
+	  tmpl = OVL_NEXT (tmpl);
 	}
       return false;
     }
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 014986d..3594ae4 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1461,8 +1461,6 @@  build_overload (tree decl, tree chain)
 {
   if (! chain && TREE_CODE (decl) != TEMPLATE_DECL)
     return decl;
-  if (chain && TREE_CODE (chain) != OVERLOAD)
-    chain = ovl_cons (chain, NULL_TREE);
   return ovl_cons (decl, chain);
 }