Re: [PATCH] split tree_type, a.k.a. tuplifying types
On 05/23/2011 05:04 PM, Nathan Froyd wrote:
On 05/22/2011 02:24 PM, Tom de Vries wrote:
Now that struct tree_type does not exist anymore, 'sizeof (struct tree_type)'
generates an error in the following assert in fold_checksum_tree:
...
gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree)
= sizeof (struct tree_function_decl))
sizeof (struct tree_type) = sizeof (struct
tree_function_decl));
...
This error is triggered with -enable-checking=fold.
Doh. Thanks for the report.
The easy fix is s/tree_type/tree_type_non_common/. But I don't see why the
assert has to even care about tree_type; doesn't:
gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree)
= sizeof (union tree_node));
accomplish the same thing?
-Nathan
I don't know for sure what the assert is trying to check, but I'm guessing it's
trying to check that the memcpys are save. A naive implementation would be:
Index: fold-const.c
===
--- fold-const.c(revision 173703)
+++ fold-const.c(working copy)
@@ -13792,6 +13789,7 @@ recursive_label:
DECL_ASSEMBLER_NAME_SET_P (expr))
{
/* Allow DECL_ASSEMBLER_NAME to be modified. */
+ gcc_assert (tree_size (expr) = sizeof (buf));
memcpy ((char *) buf, expr, tree_size (expr));
SET_DECL_ASSEMBLER_NAME ((tree)buf, NULL);
expr = (tree) buf;
@@ -13805,6 +13803,7 @@ recursive_label:
{
/* Allow these fields to be modified. */
tree tmp;
+ gcc_assert (tree_size (expr) = sizeof (buf));
memcpy ((char *) buf, expr, tree_size (expr));
expr = tmp = (tree) buf;
TYPE_CONTAINS_PLACEHOLDER_INTERNAL (tmp) = 0;
But that turns it into a runtime check.
On the other hand, I'm not sure the original assert still makes sense. Neither
tcc_type nor tcc_declaration have variable size, so the '5 * sizeof (tree)' does
not seem applicable anymore.
If we want checks cheaper than the naive, but more maintainable than the
current, we would want something like:
+ gcc_assert (tree_class_max_size (tcc_declaration) = sizeof (buf));
+ gcc_assert (tree_class_max_size (tcc_type) = sizeof (buf));
We would want those checks moved out of the hot path, and we would need to
implement and maintain tree_class_max_size alongside tree_size and
tree_code_size. But I'm not sure it's worth the effort.
Thanks,
- Tom
Re: [PATCH] split tree_type, a.k.a. tuplifying types
On 05/22/2011 02:24 PM, Tom de Vries wrote: Now that struct tree_type does not exist anymore, 'sizeof (struct tree_type)' generates an error in the following assert in fold_checksum_tree: ... gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree) = sizeof (struct tree_function_decl)) sizeof (struct tree_type) = sizeof (struct tree_function_decl)); ... This error is triggered with -enable-checking=fold. Doh. Thanks for the report. The easy fix is s/tree_type/tree_type_non_common/. But I don't see why the assert has to even care about tree_type; doesn't: gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree) = sizeof (union tree_node)); accomplish the same thing? -Nathan
Re: [PATCH] split tree_type, a.k.a. tuplifying types
Hi, Now that struct tree_type does not exist anymore, 'sizeof (struct tree_type)' generates an error in the following assert in fold_checksum_tree: ... gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree) = sizeof (struct tree_function_decl)) sizeof (struct tree_type) = sizeof (struct tree_function_decl)); ... This error is triggered with -enable-checking=fold. Thanks, - Tom
Re: [PATCH] split tree_type, a.k.a. tuplifying types
On Tue, May 10, 2011 at 7:50 PM, Nathan Froyd froy...@codesourcery.com wrote:
On Tue, May 10, 2011 at 02:28:06PM -0300, Diego Novillo wrote:
On Tue, May 10, 2011 at 13:15, Nathan Froyd froy...@codesourcery.com wrote:
Other types can of course be shrunk, but the memory savings from doing
so will be negligible
Have you done any measurements on the potential savings?
Only back-of-the-envelope. I will try to get some numbers after we
start saving memory. :)
+static void
+lto_input_ts_type_common_tree_pointers (struct lto_input_block *ib,
+ struct data_in *data_in, tree expr)
+{
+ TYPE_SIZE (expr) = lto_input_tree (ib, data_in);
+ TYPE_SIZE_UNIT (expr) = lto_input_tree (ib, data_in);
+ TYPE_ATTRIBUTES (expr) = lto_input_tree (ib, data_in);
+ TYPE_NAME (expr) = lto_input_tree (ib, data_in);
+ /* Do not stream TYPE_POINTER_TO or TYPE_REFERENCE_TO. */
Add some wording as to why not? This was copied from existing
comments, but I do not remember why we were doing this. Not too
critical, anyway.
I'm not entirely sure; I'm not intimately familiar with how LTO
streaming works. lto.c's lto_ft_type and lto_ft_common purport to
recreate TYPE_{POINTER,REFERENCE}_TO, but I don't immediately see how
that's supposed to work. I can imagine that we ought to be able to
recreate those fields after reading everything in, and that's why don't
stream them; I just don't know where that's done.
Yes, we're re-creating them to avoid streaming all pointer types that might
be unused before streaming.
One nit:
+struct GTY(()) tree_type_non_common {
+ struct tree_type_with_lang_specific common;
shouldn't that field be named w_lang_specific or something like that,
instead of re-using common?
Richard.
-Nathan
Re: [PATCH] split tree_type, a.k.a. tuplifying types
I'm checking this in to fix build with --enable-gather-detailed-mem-stats. Jason commit 091ac850cf8830722c9bae83232f0d95ccd45b2a Author: Jason Merrill ja...@redhat.com Date: Wed May 11 21:36:09 2011 -0400 * tree.c (type_hash_canon): Use struct tree_type_non_common. diff --git a/gcc/tree.c b/gcc/tree.c index c38d24b..3357d84 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -6217,7 +6217,7 @@ type_hash_canon (unsigned int hashcode, tree type) #ifdef GATHER_STATISTICS tree_code_counts[(int) TREE_CODE (type)]--; tree_node_counts[(int) t_kind]--; - tree_node_sizes[(int) t_kind] -= sizeof (struct tree_type); + tree_node_sizes[(int) t_kind] -= sizeof (struct tree_type_non_common); #endif return t1; }
[PATCH] split tree_type, a.k.a. tuplifying types
tree_type is a fairly big structure, and several of its fields are overloaded. This means we waste some space, depending on the type, and it's also somewhat confusing as to who uses what. The patch below implements the first step towards tuplifying types: those pieces which all types use have been split out into a tree_type_common structure and those pieces which are overloaded live in a tree_type_non_common structure. I choose to implement the lang-specific bit as a separate substructure, on the off-chance that some types are fortunate enough to not have langage-specific data attached to them by the FEs--I have not gone through and made a detailed check of this, but a skimming shows that nearly every interesting type that we'd care about shrinking has TYPE_LANG_SPECIFIC set on it by some FE. There are several miscellaneous places that need to be fixed up after the removal of tree_type. The changes to the Ada, Java, ObjC/C++, and C++ FEs mainly have to do with direct access to the fields of tree_type, rather than going through accessor macros. ggc-page.c creates a bucket size for sizeof (struct tree_type); I changed it to use 'struct tree_type_non_common' instead. There are a couple of places in the compiler where fields are accessed directly--stor-layout.c and lto/lto.c--I chose to leave those alone. After this reorganization, the most that we can shrink most types will be 2-3 words (possibly more, if we moved TYPE_SIZE around so not all types needed it). According to PR 45375, the types on which we should focus are: - RECORD_TYPE - FUNCTION_TYPE - METHOD_TYPE - POINTER_TYPE - INTEGER_TYPE - REFERENCE_TYPE RECORD_TYPE is not straightforwardly shrinkable. FUNCTION_TYPE and METHOD_TYPE could probably be shrunk by a word or two; likewise for POINTER_TYPE and REFERENCE_TYPE. (POINTER_TYPE and REFERENCE_TYPE could also be made smaller by not allocating an entire TREE_VEC for the one cached value they put in TYPE_CACHED_VALUES...) INTEGER_TYPE's binfo field is used by Ada; INTEGER_TYPE could be shrunk by one word were it not for this overloading. Other types can of course be shrunk, but the memory savings from doing so will be negligible This patch also opens up the possibility of using language-specific trees for types as well as decls; the C++ FE's tree_size langhook makes an attempt to do this, but we never call the tree_size langhook for types. I haven't included the refactoring of types yet; I'll let this patch sit on trunk for a couple of days before trying that. Testing with all languages in progress on x86_64-unknown-linux-gnu; ObjC/Go/Ada tests have completed with no regressions; C/C++/Fortran are looking good as well. OK to commit? -Nathan gcc/ada/ * gcc-interface/ada-tree.h (TYPE_OBJECT_RECORD_TYPE): Use TYPE_MINVAL. (TYPE_GCC_MIN_VALUE): Use TYPE_MINVAL. (TYPE_GCC_MAX_VALUE): Use TYPE_MAXVAL. gcc/cp/ * cp-tree.h (TYPENAME_TYPE_FULLNAME, TYPEOF_TYPE_EXPR): Use TYPE_VALUES_RAW. (UNDERLYING_TYPE_TYPE, DECLTYPE_TYPE_EXPR): Likewise. (DECLTYPE_TYPE_ID_EXPR_OR_MEMBER_ACCESS_P): Likewise. (TEMPLATE_TYPE_PARM_INDEX): Likewise. gcc/ * ggc-page.c (extra_order_size_table): Use struct tree_type_non_common. * lto-streamer-in.c (unpack_ts_type_value_fields): Rename to... (unpack_ts_type_common_value_fields): ...this. Update comment. (unpack_value_fields): Adjust for renaming. (lto_input_ts_type_tree_pointers): Split into... (lto_input_ts_type_common_tree_pointer): ...this and... (lto_input_ts_type_non_common_tree_pointers): ...this. (lto_input_tree_pointers): Adjust for above split. * lto-streamer-out.c (pack_ts_type_value_fields): Rename to... (pack_ts_type_common_value_fields): ...this. Update comment. (lto_output_ts_type_tree_pointers): Split into... (lto_output_ts_type_common_tree_pointers): ...this and... (lto_output_ts_type_non_common_tree_pointers): ...this. (lto_output_tree_pointers): Adjust for above split. * lto-streamer.c (check_handled_ts_structures): Mark TS_TYPE_COMMON, TS_TYPE_WITH_LANG_SPECIFIC, and TS_TYPE_NON_COMMON as handled. * stor-layout.c (vector_type_mode): Adjust location of mode field. * tree.h (MARK_TS_TYPE_COMMON, MARK_TS_TYPE_WITH_LANG_SPECIFIC): Define. (struct tree_type): Split into... (struct tree_type_common: ...this and... (struct tree_type_with_lang_specific): ...this and... (struct tree_type_non_common): ...this. Adjust accessor macros accordingly. (TYPE_VALUES_RAW): Define. (union tree_node): Update for above changes. * tree.c (tree_node_structure_for_code) [tcc_type]: Return TS_TYPE_NON_COMMON. (initialize_tree_contains_struct) [TS_TYPE]: Use TS_TYPE_COMMON. Add TS_TYPE_WITH_LANG_SPECIFIC and TS_TYPE_NON_COMMON. (tree_code_size)
Re: [PATCH] split tree_type, a.k.a. tuplifying types
On May 10, 2011, at 9:15 AM, Nathan Froyd froy...@codesourcery.com wrote: The patch below implements the first step towards tuplifying types: OK to commit? Ok for Objective parts...
Re: [PATCH] split tree_type, a.k.a. tuplifying types
On Tue, May 10, 2011 at 13:15, Nathan Froyd froy...@codesourcery.com wrote:
Other types can of course be shrunk, but the memory savings from doing
so will be negligible
Have you done any measurements on the potential savings?
+static void
+lto_input_ts_type_common_tree_pointers (struct lto_input_block *ib,
+ struct data_in *data_in, tree expr)
+{
+ TYPE_SIZE (expr) = lto_input_tree (ib, data_in);
+ TYPE_SIZE_UNIT (expr) = lto_input_tree (ib, data_in);
+ TYPE_ATTRIBUTES (expr) = lto_input_tree (ib, data_in);
+ TYPE_NAME (expr) = lto_input_tree (ib, data_in);
+ /* Do not stream TYPE_POINTER_TO or TYPE_REFERENCE_TO. */
Add some wording as to why not? This was copied from existing
comments, but I do not remember why we were doing this. Not too
critical, anyway.
OK, otherwise. Thanks for doing this!
Diego.
Re: [PATCH] split tree_type, a.k.a. tuplifying types
Nathan == Nathan Froyd froy...@codesourcery.com writes: Nathan gcc/java/ Nathan * java-tree.h (TYPE_ARGUMENT_SIGNATURE): Use TYPE_MINVAL. This is ok. Tom
Re: [PATCH] split tree_type, a.k.a. tuplifying types
Hi,
On Tue, 10 May 2011, Nathan Froyd wrote:
+ /* Do not stream TYPE_POINTER_TO or TYPE_REFERENCE_TO. */
Add some wording as to why not? This was copied from existing
comments, but I do not remember why we were doing this. Not too
critical, anyway.
I'm not entirely sure; I'm not intimately familiar with how LTO
streaming works. lto.c's lto_ft_type and lto_ft_common purport to
recreate TYPE_{POINTER,REFERENCE}_TO, but I don't immediately see how
that's supposed to work. I can imagine that we ought to be able to
recreate those fields after reading everything in, and that's why don't
stream them; I just don't know where that's done.
That is correct. As soon as we read in a POINTER_TYPE or REFERENCE_TYPE
we'll reconstruct the target type's TYPE_{POINTER,REFERENCE}_TO fields as
being the trees we just process. Type merging would have to overwrite
them anyway, so it actually saves time and space to not stream or
reconstruct them, just to have them overwritten during type merging.
Ciao,
Michael.
