[C++ PATCH] Avoid sharing DECL_UID between different decls (was Re: checked in patch for PR 27793)

Jakub Jelinek jakub@redhat.com
Tue Jun 13 07:18:00 GMT 2006


On Fri, Jun 09, 2006 at 04:02:59PM -0400, Andrew MacLeod wrote:
> On Fri, 2006-06-09 at 15:05 -0400, Jakub Jelinek wrote:
> > On Wed, Jun 07, 2006 at 09:20:25PM -0400, Andrew MacLeod wrote:
> 
> > I have bootstrapped/regtested this on the trunk on i686-linux after
> > reverting revision 114480 (i.e. Andrew's PR middle-end/27793 workaround).
> 
> If this is approved, you can either revert my changes, or I will do it.
> In either case, we should make sure the assert that I mentioned in 27793
> is inserted in it place to catch if this ever shows up anywhere again.
> (assert.diff in the PR, added in comment #13) 

Yes, but your assert.diff is gcc-4_1-branch patch and the trunk is very
different, so I'd prefer if you could handle that part yourself.

FYI, I have also bootstrapped/regtested the following combined patch on 7
linux arches on gcc-4_1-branch (includes my C++ FE workaround, your
assert.diff and reversion of your PR27793 fix), no regressions and verified
the testcases fail when your 2 fixes are reverted, succeeds with current SVN
and succeeds with this patch applied as well:

2006-06-12  Andrew MacLeod  <amacleod@redhat.com>

	PR middle-end/27793
	* tree-dfa.c (add_referenced_var): Assert DECL_UID is unique for
	different decls.

2006-06-12  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/27793
	* cp-tree.h (cxx_int_tree_map): New struct.
	(struct language_function): Add extern_decl_map field.
	* name-lookup.c (pushdecl_maybe_friend): Add x -> t mapping
	to cp_function_chain->extern_decl_map hash table instead of
	copying over DECL_UID.
	* cp-gimplify.c (cxx_int_tree_map_eq, cxx_int_tree_map_hash): New
	functions.
	(cp_genericize_r): Remap DECL_EXTERN local decls using
	cp_function_chain->extern_decl_map hash table.
	* decl.c (finish_function): Clear extern_decl_map.

	Revert:
	2006-06-06  Andrew MacLeod  <amacleod@redhat.com>
	PR middle-end/27793
	* tree-dfa.c (referenced_vars_dup_list): New.  List of duplicate 
	referenced_variables with matching DECL_UID's.
	(find_referenced_vars): Make sure duplicate list is empty to start.
	(add_referenced_var): Add var to duplicate list if required.
	* tree-ssa.c (delete_tree_ssa): Clear var_ann's on duplicates.
	* tree-flow.h (referenced_vars_dup_list): External declaration.

	PR c++/26757
	PR c++/27894
	* g++.dg/tree-ssa/pr26757.C: New test.
	* g++.dg/tree-ssa/pr27894.C: New test.

--- gcc/tree-flow.h	(revision 114458)
+++ gcc/tree-flow.h	(revision 114457)
@@ -420,8 +420,6 @@ typedef struct
 
 /* Array of all variables referenced in the function.  */
 extern GTY((param_is (struct int_tree_map))) htab_t referenced_vars;
-/* List of referenced variables in the function with duplicate UID's.  */
-extern VEC(tree,gc) *referenced_vars_dup_list;
 
 extern tree referenced_var_lookup (unsigned int);
 extern tree referenced_var_lookup_if_exists (unsigned int);
--- gcc/tree-ssa.c	(revision 114458)
+++ gcc/tree-ssa.c	(revision 114457)
@@ -821,7 +821,6 @@ delete_tree_ssa (void)
   block_stmt_iterator bsi;
   referenced_var_iterator rvi;
   tree var;
-  unsigned u;
 
   /* Release any ssa_names still in use.  */
   for (i = 0; i < num_ssa_names; i++)
@@ -856,16 +855,6 @@ delete_tree_ssa (void)
       ggc_free (var->common.ann);
       var->common.ann = NULL;
     }
-
-  /* Remove any referenced variables which had duplicate UID's.  */
-  for (u = 0; u < VEC_length (tree, referenced_vars_dup_list); u++)
-    {
-      var = VEC_index (tree, referenced_vars_dup_list, u);
-      ggc_free (var->common.ann);
-      var->common.ann = NULL;
-    }
-  VEC_free (tree, gc, referenced_vars_dup_list);
-
   htab_delete (referenced_vars);
   referenced_vars = NULL;
 
--- gcc/tree-dfa.c	(revision 114458)
+++ gcc/tree-dfa.c	(revision 114457)
@@ -76,8 +76,6 @@ static void add_referenced_var (tree, bo
 
 /* Array of all variables referenced in the function.  */
 htab_t referenced_vars;
-/* List of referenced variables with duplicate UID's.  */
-VEC(tree,gc) *referenced_vars_dup_list;
 
 
 /*---------------------------------------------------------------------------
@@ -97,7 +95,6 @@ find_referenced_vars (void)
   basic_block bb;
   block_stmt_iterator si;
 
-  gcc_assert (VEC_length (tree, referenced_vars_dup_list) == 0);
   FOR_EACH_BB (bb)
     for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
       {
@@ -613,31 +610,18 @@ static void
 add_referenced_var (tree var, bool always)
 {
   var_ann_t v_ann;
-  tree dup = referenced_var_lookup_if_exists (DECL_UID (var));
+  tree ref;
 
   v_ann = get_var_ann (var);
   gcc_assert (DECL_P (var));
-  
-  /* PRs 26757 and 27793.  Maintain a list of duplicate variable pointers
-     with the same DECL_UID.  There isn't usually very many.  
-     TODO.  Once the C++ front end doesn't create duplicate DECL UID's, this
-     code can be removed.  */
-  if (dup && dup != var)
-    {
-      unsigned u;
-      tree t = NULL_TREE;
 
-      for (u = 0; u < VEC_length (tree, referenced_vars_dup_list); u++)
-	{
-	  t = VEC_index (tree, referenced_vars_dup_list, u);
-	  if (t == var)
-	    break;
-	}
-      if (t != var)
-	VEC_safe_push (tree, gc, referenced_vars_dup_list, var);
-    }
+  ref = referenced_var_lookup_if_exists (DECL_UID (var));  
+
+  /* Catch PRs 26757 and 27793.  If this assert triggers, REF and VAR are
+     two different variables in this function with the same DECL_UID.  */
+  gcc_assert (!ref || ref == var);
 
-  if (always || dup == NULL_TREE)
+  if (always || ref == NULL_TREE)
     {
       /* This is the first time we find this variable, add it to the
          REFERENCED_VARS array and annotate it with attributes that are
--- gcc/cp/name-lookup.c.jj	2006-03-27 14:32:50.000000000 +0200
+++ gcc/cp/name-lookup.c	2006-06-12 13:46:57.000000000 +0200
@@ -669,14 +669,28 @@ pushdecl_maybe_friend (tree x, bool is_f
 	      if (decls_match (x, t))
 		/* The standard only says that the local extern
 		   inherits linkage from the previous decl; in
-		   particular, default args are not shared.  We must
-		   also tell cgraph to treat these decls as the same,
-		   or we may neglect to emit an "unused" static - we
-		   do this by making the DECL_UIDs equal, which should
-		   be viewed as a kludge.  FIXME.  */
+		   particular, default args are not shared.  Add
+		   the decl into a hash table to make sure only
+		   the previous decl in this case is seen by the
+		   middle end.  */
 		{
+		  struct cxx_int_tree_map *h;
+		  void **loc;
+
 		  TREE_PUBLIC (x) = TREE_PUBLIC (t);
-		  DECL_UID (x) = DECL_UID (t);
+
+		  if (cp_function_chain->extern_decl_map == NULL)
+		    cp_function_chain->extern_decl_map
+		      = htab_create_ggc (20, cxx_int_tree_map_hash,
+					 cxx_int_tree_map_eq, NULL);
+
+		  h = GGC_NEW (struct cxx_int_tree_map);
+		  h->uid = DECL_UID (x);
+		  h->to = t;
+		  loc = htab_find_slot_with_hash
+			  (cp_function_chain->extern_decl_map, h,
+			   h->uid, INSERT);
+		  *(struct cxx_int_tree_map **) loc = h;
 		}
 	    }
 	  else if (TREE_CODE (t) == PARM_DECL)
--- gcc/cp/cp-gimplify.c.jj	2006-02-10 21:24:13.000000000 +0100
+++ gcc/cp/cp-gimplify.c	2006-06-12 13:46:57.000000000 +0200
@@ -589,6 +589,24 @@ is_invisiref_parm (tree t)
 	  && DECL_BY_REFERENCE (t));
 }
 
+/* Return true if the uid in both int tree maps are equal.  */
+
+int
+cxx_int_tree_map_eq (const void *va, const void *vb)
+{
+  const struct cxx_int_tree_map *a = (const struct cxx_int_tree_map *) va;
+  const struct cxx_int_tree_map *b = (const struct cxx_int_tree_map *) vb;
+  return (a->uid == b->uid);
+}
+
+/* Hash a UID in a cxx_int_tree_map.  */
+
+unsigned int
+cxx_int_tree_map_hash (const void *item)
+{
+  return ((const struct cxx_int_tree_map *)item)->uid;
+}
+
 /* Perform any pre-gimplification lowering of C++ front end trees to
    GENERIC.  */
 
@@ -608,6 +626,25 @@ cp_genericize_r (tree *stmt_p, int *walk
       return NULL;
     }
 
+  /* Map block scope extern declarations to visible declarations with the
+     same name and type in outer scopes if any.  */
+  if (cp_function_chain->extern_decl_map
+      && (TREE_CODE (stmt) == FUNCTION_DECL || TREE_CODE (stmt) == VAR_DECL)
+      && DECL_EXTERNAL (stmt))
+    {
+      struct cxx_int_tree_map *h, in;
+      in.uid = DECL_UID (stmt);
+      h = (struct cxx_int_tree_map *)
+	  htab_find_with_hash (cp_function_chain->extern_decl_map,
+			       &in, in.uid);
+      if (h)
+	{
+	  *stmt_p = h->to;
+	  *walk_subtrees = 0;
+	  return NULL;
+	}
+    }
+
   /* Other than invisiref parms, don't walk the same tree twice.  */
   if (pointer_set_contains (p_set, stmt))
     {
--- gcc/cp/decl.c.jj	2006-06-08 09:10:47.000000000 +0200
+++ gcc/cp/decl.c	2006-06-12 13:46:57.000000000 +0200
@@ -11004,6 +11004,7 @@ finish_function (int flags)
       f->x_vtt_parm = NULL;
       f->x_return_value = NULL;
       f->bindings = NULL;
+      f->extern_decl_map = NULL;
 
       /* Handle attribute((warn_unused_result)).  Relies on gimple input.  */
       c_warn_unused_result (&DECL_SAVED_TREE (fndecl));
--- gcc/cp/cp-tree.h.jj	2006-06-08 09:10:47.000000000 +0200
+++ gcc/cp/cp-tree.h	2006-06-12 13:46:57.000000000 +0200
@@ -719,6 +719,15 @@ struct saved_scope GTY(())
 
 extern GTY(()) struct saved_scope *scope_chain;
 
+struct cxx_int_tree_map GTY(())
+{
+  unsigned int uid;
+  tree to;
+};
+
+extern unsigned int cxx_int_tree_map_hash (const void *);
+extern int cxx_int_tree_map_eq (const void *, const void *);
+
 /* Global state pertinent to the current function.  */
 
 struct language_function GTY(())
@@ -746,6 +755,7 @@ struct language_function GTY(())
   struct named_label_list *x_named_labels;
   struct cp_binding_level *bindings;
   VEC(tree,gc) *x_local_names;
+  htab_t GTY((param_is (struct cxx_int_tree_map))) extern_decl_map;
 };
 
 /* The current C++-specific per-function global variables.  */
--- gcc/testsuite/g++.dg/tree-ssa/pr27894.C.jj	2006-06-12 14:15:10.000000000 +0200
+++ gcc/testsuite/g++.dg/tree-ssa/pr27894.C	2006-06-12 13:45:41.000000000 +0200
@@ -0,0 +1,82 @@
+// PR c++/27894
+// { dg-do compile }
+// { dg-options "-O" }
+
+class A;
+struct B
+{
+  B (unsigned long);
+  int b2 () const;
+  A *b1 () const;
+};
+
+enum { P = 0 };
+enum O { Q = 75, };
+class C;
+struct D { A *d; };
+struct E
+{
+  B e1 (int) const;
+  A *e2 (const B &) const;
+  D e3[4096];
+};
+
+inline A *
+E::e2 (const B & x) const
+{
+  const D *w = &e3[x.b2 ()];
+  return (A *) w->d;
+}
+
+extern E *e;
+
+inline A *
+B::b1 () const
+{
+  extern E *e;
+  return e->e2 (*this);
+}
+
+template <class T> struct F : public B
+{
+  F (const B &);
+  T *b1 () const;
+};
+
+template < class T > inline T * F <T>::b1 () const
+{
+  return (T *) B::b1 ();
+};
+
+typedef F <C> N;
+
+class G {};
+class H : public G {};
+class I : public H {};
+class J {};
+class K {};
+struct L
+{
+  void l (J *, C *, int, const char *, O);
+};
+class M : public K, public I
+{
+  void m (J &, int, const char *);
+  void m (J &, int, int, const char *, float);
+};
+
+void
+M::m (J &x, int y, const char *z)
+{
+  L *w = new L;
+  N v = e->e1 (y);
+  w->l (&x, v.b1 (), P, z, Q);
+}
+
+void
+M::m (J &x, int y, int s, const char *z, float t)
+{
+  L *w = new L;
+  N v = e->e1 (y);
+  w->l (&x, v.b1 (), s, z, (O) (int) ((t) ? (50 + 20 / (float) t) : 0));
+}
--- gcc/testsuite/g++.dg/tree-ssa/pr26757.C.jj	2006-06-12 14:15:26.000000000 +0200
+++ gcc/testsuite/g++.dg/tree-ssa/pr26757.C	2006-06-12 14:11:17.000000000 +0200
@@ -0,0 +1,44 @@
+// PR c++/26757
+// { dg-do run }
+// { dg-options "-O" }
+
+extern "C" void abort ();
+
+typedef struct A
+{
+  int c;
+  int d;
+} A;
+
+A *b;
+
+void
+foo ()
+{
+  b->c++;
+  extern A *b;
+  b->d++;
+
+}
+
+void
+bar ()
+{
+  if (b->d)
+    b->c++;
+}
+
+
+int
+main ()
+{
+  A a = { 0, 0 };
+  b = &a;
+  foo ();
+  bar ();
+  if (b->c != 2)
+    abort ();
+  if (b->d != 1)
+    abort ();
+  return 0;
+}


	Jakub



More information about the Gcc-patches mailing list