[patch][lto] remove var_decls from the size of structs

[patch][lto] remove var_decls from the size of structs

[Rafael Espindola]

[-- Attachment #1: Type: text/plain, Size: 740 bytes --]

This fixes

void f(int a, struct {int b[a];} c) {}

OK for lto if bootstraps and tests are OK?
	
2008-06-26  Rafael Espindola  <espindola@google.com>

	Based on http://gcc.gnu.org/ml/gcc-patches/2007-11/msg00523.html
	and http://gcc.gnu.org/ml/gcc-patches/2008-03/msg00349.html

	* tree.c (decl_for_uid_map): New.
	(init_ttree): Initialize decl_for_uid_map.
	(make_node_stat): call insert_decl_to_uid_decl_map.
	(insert_decl_to_uid_decl_map): New.
	(reset_type_lang_specific): Handle RECORD_TYPE.
	(reset_lang_specific): New.
	(free_lang_specifics): Call reset_lang_specific.
	
Cheers,
-- 
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047

[-- Attachment #2: struct-size.patch --]
[-- Type: application/octet-stream, Size: 3564 bytes --]

diff --git a/gcc/tree.c b/gcc/tree.c
index f69c593..bcfc89a 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -113,6 +113,12 @@ static GTY(()) int next_type_uid = 1;
 static GTY ((if_marked ("ggc_marked_p"), param_is (union tree_node)))
      htab_t uid2type_map;
 
+/* Mapping from unique DECL_UID to the decl tree node.  */
+static GTY ((if_marked ("ggc_marked_p"), param_is (union tree_node)))
+     htab_t decl_for_uid_map;
+
+static void insert_decl_to_uid_decl_map (tree);
+
 /* Since we cannot rehash a type after it is in the table, we have to
    keep the hash code.  */
 
@@ -269,6 +275,9 @@ init_ttree (void)
   
   int_cst_node = make_node (INTEGER_CST);
 
+  decl_for_uid_map = htab_create_ggc (4093, uid_decl_map_hash,
+				      uid_decl_map_eq, NULL);
+
   uid2type_map = htab_create_ggc (4093, uid_type_map_hash,
 				  uid_type_map_eq, NULL);
 
@@ -637,6 +646,7 @@ make_node_stat (enum tree_code code MEM_STAT_DECL)
 	}
       DECL_SOURCE_LOCATION (t) = input_location;
       DECL_UID (t) = next_decl_uid++;
+      insert_decl_to_uid_decl_map (t);
 
       break;
 
@@ -740,6 +750,7 @@ copy_node_stat (tree node MEM_STAT_DECL)
 	  SET_DECL_RESTRICT_BASE (t, DECL_GET_RESTRICT_BASE (node));
 	  DECL_BASED_ON_RESTRICT_P (t) = 1;
 	}
+      insert_decl_to_uid_decl_map (t);
     }
   else if (TREE_CODE_CLASS (code) == tcc_type)
     {
@@ -3437,6 +3448,30 @@ build_nt_call_list (tree fn, tree arglist)
     CALL_EXPR_ARG (t, i) = TREE_VALUE (arglist);
   return t;
 }
+
+/* Insert the declaration NODE into the map mapping its unique uid
+   back to the tree.  */
+
+static void
+insert_decl_to_uid_decl_map (tree node)
+{
+  void **slot;
+  struct tree_decl_minimal key;
+
+  key.uid = DECL_UID (node);
+  slot = htab_find_slot_with_hash (decl_for_uid_map,
+				   &key, DECL_UID (node), INSERT);
+
+  /* We should never try to re-insert a decl with the same uid.
+     ???  The C++ frontend breaks this invariant.  Hopefully in a
+     non-fatal way, so just overwrite the slot in this case.  */
+#if 0
+  gcc_assert (!*slot);
+#endif
+
+  *(tree *)slot = node;
+}
+
 \f
 /* Create a DECL_... node of code CODE, name NAME and data type TYPE.
    We do NOT enter this node in any sort of symbol table.
@@ -3784,7 +3819,8 @@ reset_type_lang_specific (void **slot, void *unused ATTRIBUTE_UNUSED)
   tree decl = *(tree*)slot;
   lang_hooks.reset_lang_specifics (decl);
 
-  if (TREE_CODE (decl) == ARRAY_TYPE)
+  if (TREE_CODE (decl) == ARRAY_TYPE
+      || TREE_CODE (decl) == RECORD_TYPE)
     {
       tree unit_size = TYPE_SIZE_UNIT (decl);
       tree size = TYPE_SIZE (decl);
@@ -3817,12 +3853,33 @@ reset_type_lang_specific (void **slot, void *unused ATTRIBUTE_UNUSED)
   return 1;
 }
 
+/* Helper function of free_lang_specifics. */
+static int
+reset_lang_specific (void **slot, void *unused ATTRIBUTE_UNUSED)
+{
+  tree decl = *(tree*)slot;
+  if (TREE_CODE (decl) == PARM_DECL
+      || TREE_CODE (decl) == FIELD_DECL)
+    {
+      tree unit_size = DECL_SIZE_UNIT (decl);
+      tree size = DECL_SIZE (decl);
+      if ((unit_size && TREE_CODE (unit_size) != INTEGER_CST)
+	  || (size && TREE_CODE (size) != INTEGER_CST))
+	{
+	  DECL_SIZE_UNIT (decl) = NULL_TREE;
+	  DECL_SIZE (decl) = NULL_TREE;
+	}
+  }
+  return 1;
+}
+
 /* Free resources that are used by FE but are not needed once they are done. */
 
 void
 free_lang_specifics (void)
 {
   htab_traverse (uid2type_map, reset_type_lang_specific, NULL);
+  htab_traverse (decl_for_uid_map, reset_lang_specific, NULL);
 }
 
 /* Return nonzero if IDENT is a valid name for attribute ATTR,

Re: [patch][lto] remove var_decls from the size of structs

[Diego Novillo]

On Thu, Jun 26, 2008 at 15:08, Rafael Espindola <espindola@google.com> wrote:

> 2008-06-26  Rafael Espindola  <espindola@google.com>
>
>        Based on http://gcc.gnu.org/ml/gcc-patches/2007-11/msg00523.html
>        and http://gcc.gnu.org/ml/gcc-patches/2008-03/msg00349.html
>
>        * tree.c (decl_for_uid_map): New.
>        (init_ttree): Initialize decl_for_uid_map.
>        (make_node_stat): call insert_decl_to_uid_decl_map.
>        (insert_decl_to_uid_decl_map): New.
>        (reset_type_lang_specific): Handle RECORD_TYPE.
>        (reset_lang_specific): New.
>        (free_lang_specifics): Call reset_lang_specific.

OK.

Could you investigate why is the C++ FE creating two DECLs with the
same UID?  I'm hoping that it is a case of a DECL being recycled when
no longer needed, or something along those lines.

In any event, addressing this

+  /* We should never try to re-insert a decl with the same uid.
+     ???  The C++ frontend breaks this invariant.  Hopefully in a
+     non-fatal way, so just overwrite the slot in this case.  */
+#if 0
+  gcc_assert (!*slot);
+#endif

means that we can apply all of Richard's patch in trunk and then
inherit from LTO.  But for the time being, it's fine to have this
subset in the LTO branch.


Diego.

Re: [patch][lto] remove var_decls from the size of structs

[Rafael Espindola]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

> OK.

I needed to pull one extra bit from the Richard's patch:

cp/decl.c (duplicate_decls): Call remove_decl_from_map.
tree.c (remove_decl_from_map): New.

With this addition we bootstrap and regression tests are OK.

OK for the branch?

> Could you investigate why is the C++ FE creating two DECLs with the
> same UID?  I'm hoping that it is a case of a DECL being recycled when
> no longer needed, or something along those lines.
>
> In any event, addressing this
>
> +  /* We should never try to re-insert a decl with the same uid.
> +     ???  The C++ frontend breaks this invariant.  Hopefully in a
> +     non-fatal way, so just overwrite the slot in this case.  */
> +#if 0
> +  gcc_assert (!*slot);
> +#endif
>
> means that we can apply all of Richard's patch in trunk and then
> inherit from LTO.  But for the time being, it's fine to have this
> subset in the LTO branch.

I will try to take a look.

>
> Diego.
>


Cheers,
-- 
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047

[-- Attachment #2: struct-size.patch --]
[-- Type: application/octet-stream, Size: 4796 bytes --]

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0898d5d..b2c983d 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -2151,6 +2151,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
   /* The NEWDECL will no longer be needed.  Because every out-of-class
      declaration of a member results in a call to duplicate_decls,
      freeing these nodes represents in a significant savings.  */
+  remove_decl_from_map (newdecl);
   ggc_free (newdecl);
 
   return olddecl;
diff --git a/gcc/tree.c b/gcc/tree.c
index f69c593..28b58e4 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -113,6 +113,12 @@ static GTY(()) int next_type_uid = 1;
 static GTY ((if_marked ("ggc_marked_p"), param_is (union tree_node)))
      htab_t uid2type_map;
 
+/* Mapping from unique DECL_UID to the decl tree node.  */
+static GTY ((if_marked ("ggc_marked_p"), param_is (union tree_node)))
+     htab_t decl_for_uid_map;
+
+static void insert_decl_to_uid_decl_map (tree);
+
 /* Since we cannot rehash a type after it is in the table, we have to
    keep the hash code.  */
 
@@ -269,6 +275,9 @@ init_ttree (void)
   
   int_cst_node = make_node (INTEGER_CST);
 
+  decl_for_uid_map = htab_create_ggc (4093, uid_decl_map_hash,
+				      uid_decl_map_eq, NULL);
+
   uid2type_map = htab_create_ggc (4093, uid_type_map_hash,
 				  uid_type_map_eq, NULL);
 
@@ -637,6 +646,7 @@ make_node_stat (enum tree_code code MEM_STAT_DECL)
 	}
       DECL_SOURCE_LOCATION (t) = input_location;
       DECL_UID (t) = next_decl_uid++;
+      insert_decl_to_uid_decl_map (t);
 
       break;
 
@@ -740,6 +750,7 @@ copy_node_stat (tree node MEM_STAT_DECL)
 	  SET_DECL_RESTRICT_BASE (t, DECL_GET_RESTRICT_BASE (node));
 	  DECL_BASED_ON_RESTRICT_P (t) = 1;
 	}
+      insert_decl_to_uid_decl_map (t);
     }
   else if (TREE_CODE_CLASS (code) == tcc_type)
     {
@@ -3437,6 +3448,42 @@ build_nt_call_list (tree fn, tree arglist)
     CALL_EXPR_ARG (t, i) = TREE_VALUE (arglist);
   return t;
 }
+
+/* Insert the declaration NODE into the map mapping its unique uid
+   back to the tree.  */
+
+static void
+insert_decl_to_uid_decl_map (tree node)
+{
+  void **slot;
+  struct tree_decl_minimal key;
+
+  key.uid = DECL_UID (node);
+  slot = htab_find_slot_with_hash (decl_for_uid_map,
+				   &key, DECL_UID (node), INSERT);
+
+  /* We should never try to re-insert a decl with the same uid.
+     ???  The C++ frontend breaks this invariant.  Hopefully in a
+     non-fatal way, so just overwrite the slot in this case.  */
+#if 0
+  gcc_assert (!*slot);
+#endif
+
+  *(tree *)slot = node;
+}
+
+void
+remove_decl_from_map (tree decl)
+{
+    struct tree_decl_minimal key;
+
+    key.uid = DECL_UID (decl);
+#if ENABLE_CHECKING
+    gcc_assert (decl == htab_find_with_hash (decl_for_uid_map, &key, key.uid));
+#endif
+    htab_remove_elt_with_hash (decl_for_uid_map, &key, key.uid);
+}
+
 \f
 /* Create a DECL_... node of code CODE, name NAME and data type TYPE.
    We do NOT enter this node in any sort of symbol table.
@@ -3784,7 +3831,8 @@ reset_type_lang_specific (void **slot, void *unused ATTRIBUTE_UNUSED)
   tree decl = *(tree*)slot;
   lang_hooks.reset_lang_specifics (decl);
 
-  if (TREE_CODE (decl) == ARRAY_TYPE)
+  if (TREE_CODE (decl) == ARRAY_TYPE
+      || TREE_CODE (decl) == RECORD_TYPE)
     {
       tree unit_size = TYPE_SIZE_UNIT (decl);
       tree size = TYPE_SIZE (decl);
@@ -3817,12 +3865,33 @@ reset_type_lang_specific (void **slot, void *unused ATTRIBUTE_UNUSED)
   return 1;
 }
 
+/* Helper function of free_lang_specifics. */
+static int
+reset_lang_specific (void **slot, void *unused ATTRIBUTE_UNUSED)
+{
+  tree decl = *(tree*)slot;
+  if (TREE_CODE (decl) == PARM_DECL
+      || TREE_CODE (decl) == FIELD_DECL)
+    {
+      tree unit_size = DECL_SIZE_UNIT (decl);
+      tree size = DECL_SIZE (decl);
+      if ((unit_size && TREE_CODE (unit_size) != INTEGER_CST)
+	  || (size && TREE_CODE (size) != INTEGER_CST))
+	{
+	  DECL_SIZE_UNIT (decl) = NULL_TREE;
+	  DECL_SIZE (decl) = NULL_TREE;
+	}
+  }
+  return 1;
+}
+
 /* Free resources that are used by FE but are not needed once they are done. */
 
 void
 free_lang_specifics (void)
 {
   htab_traverse (uid2type_map, reset_type_lang_specific, NULL);
+  htab_traverse (decl_for_uid_map, reset_lang_specific, NULL);
 }
 
 /* Return nonzero if IDENT is a valid name for attribute ATTR,
diff --git a/gcc/tree.h b/gcc/tree.h
index c78c2e6..031838d 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5249,6 +5249,10 @@ struct tree_int_map GTY(())
 #define tree_int_map_hash tree_map_base_hash
 #define tree_int_map_marked_p tree_map_base_marked_p
 
+/* Map from a DECL_UID to the decl tree.  */
+
+extern void remove_decl_from_map (tree);
+
 /* Map from a tree to initialization/finalization priorities.  */
 
 struct tree_priority_map GTY(())

Re: [patch][lto] remove var_decls from the size of structs

[Diego Novillo]

On Fri, Jun 27, 2008 at 08:40, Rafael Espindola <espindola@google.com> wrote:

> OK for the branch?

Yes, with:

> +void
> +remove_decl_from_map (tree decl)
> +{

Needs comment.


Diego.