diff mbox series

Simplify types of arrays

Message ID 20181107154134.d2iufzncs3reflrd@kam.mff.cuni.cz
State New
Headers show
Series Simplify types of arrays | expand

Commit Message

Jan Hubicka Nov. 7, 2018, 3:41 p.m. UTC
Hi,
this patch simplfies types of arrays so we don't propagate duplicates
when record/union contains array of pointers.
With this we still miss simplification of pointers to arrays of
structures (where we need to rebuild array same way as we rebuild
pointers) and enumerate types. That should make simplification of types
complete. Neither of those two seems very critical for GCC build,
with the patch we are down to 24 duplicated types in bootstrap, I will
collect data on firefox, but things looks quite good.
(from tens of thousdant week ago).

The patch works, but I am somewhat nervous because modyfing type inplace
affects its type_hash_canon_hash and friends.  There are pre-existing
modifications to function parameters and things seems to just work,
but I wonder if we have any strategy on keeping hashes in tree.c
consitent across free-lang data? Or are all those hashes unused/freed at
this time?

lto-bootstrapped/regtested x86_64-linux.

Honza

	* tree.c (free_lang_data_in_type): Simplify types of arrays.

Comments

Richard Biener Nov. 8, 2018, 8:01 a.m. UTC | #1
On Wed, 7 Nov 2018, Jan Hubicka wrote:

> Hi,
> this patch simplfies types of arrays so we don't propagate duplicates
> when record/union contains array of pointers.
> With this we still miss simplification of pointers to arrays of
> structures (where we need to rebuild array same way as we rebuild
> pointers) and enumerate types. That should make simplification of types
> complete. Neither of those two seems very critical for GCC build,
> with the patch we are down to 24 duplicated types in bootstrap, I will
> collect data on firefox, but things looks quite good.
> (from tens of thousdant week ago).
> 
> The patch works, but I am somewhat nervous because modyfing type inplace
> affects its type_hash_canon_hash and friends.

Indeed - I don't think we want to do this here and this way.  What
you'd need to do is record uses of a type and replace all uses with
a simplified copy ...

> There are pre-existing
> modifications to function parameters and things seems to just work,
> but I wonder if we have any strategy on keeping hashes in tree.c
> consitent across free-lang data? Or are all those hashes unused/freed at
> this time?

They are not freed and I don't see why they should.

> lto-bootstrapped/regtested x86_64-linux.

Please lets not do this and instead keep the array of pointers
thing in mind.

Does it really matter so much?  That is, where are those arrays
with pointer to complete type referenced from?  Global variables?
As fields in records they are already fixed up properly, right?

Richard.

> Honza
> 
> 	* tree.c (free_lang_data_in_type): Simplify types of arrays.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 265877)
> +++ tree.c	(working copy)
> @@ -5320,6 +5320,8 @@ free_lang_data_in_type (tree type, struc
>  	  TREE_PURPOSE (p) = NULL;
>  	}
>      }
> +  else if (TREE_CODE (type) == ARRAY_TYPE)
> +    TREE_TYPE (type) = fld_simplified_type (TREE_TYPE (type), fld);
>    else if (RECORD_OR_UNION_TYPE_P (type))
>      {
>        /* Remove members that are not FIELD_DECLs from the field list
> 
>
Jan Hubicka Nov. 8, 2018, 10:55 a.m. UTC | #2
> On Wed, 7 Nov 2018, Jan Hubicka wrote:
> 
> > Hi,
> > this patch simplfies types of arrays so we don't propagate duplicates
> > when record/union contains array of pointers.
> > With this we still miss simplification of pointers to arrays of
> > structures (where we need to rebuild array same way as we rebuild
> > pointers) and enumerate types. That should make simplification of types
> > complete. Neither of those two seems very critical for GCC build,
> > with the patch we are down to 24 duplicated types in bootstrap, I will
> > collect data on firefox, but things looks quite good.
> > (from tens of thousdant week ago).
> > 
> > The patch works, but I am somewhat nervous because modyfing type inplace
> > affects its type_hash_canon_hash and friends.
> 
> Indeed - I don't think we want to do this here and this way.  What
> you'd need to do is record uses of a type and replace all uses with
> a simplified copy ...
> 
> > There are pre-existing
> > modifications to function parameters and things seems to just work,
> > but I wonder if we have any strategy on keeping hashes in tree.c
> > consitent across free-lang data? Or are all those hashes unused/freed at
> > this time?
> 
> They are not freed and I don't see why they should.
> 
> > lto-bootstrapped/regtested x86_64-linux.
> 
> Please lets not do this and instead keep the array of pointers
> thing in mind.
> 
> Does it really matter so much?  That is, where are those arrays
> with pointer to complete type referenced from?  Global variables?
> As fields in records they are already fixed up properly, right?

If you have record that contains array of pointers or pointer to array
they are not simplified or made incomplete at the moment.
simplified_type_of looks only on the pointers and references at the
moment.  I have patch doing that from simplified_type_of, so I can give
it testing.

Honza
> 
> Richard.
> 
> > Honza
> > 
> > 	* tree.c (free_lang_data_in_type): Simplify types of arrays.
> > Index: tree.c
> > ===================================================================
> > --- tree.c	(revision 265877)
> > +++ tree.c	(working copy)
> > @@ -5320,6 +5320,8 @@ free_lang_data_in_type (tree type, struc
> >  	  TREE_PURPOSE (p) = NULL;
> >  	}
> >      }
> > +  else if (TREE_CODE (type) == ARRAY_TYPE)
> > +    TREE_TYPE (type) = fld_simplified_type (TREE_TYPE (type), fld);
> >    else if (RECORD_OR_UNION_TYPE_P (type))
> >      {
> >        /* Remove members that are not FIELD_DECLs from the field list
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
diff mbox series

Patch

Index: tree.c
===================================================================
--- tree.c	(revision 265877)
+++ tree.c	(working copy)
@@ -5320,6 +5320,8 @@  free_lang_data_in_type (tree type, struc
 	  TREE_PURPOSE (p) = NULL;
 	}
     }
+  else if (TREE_CODE (type) == ARRAY_TYPE)
+    TREE_TYPE (type) = fld_simplified_type (TREE_TYPE (type), fld);
   else if (RECORD_OR_UNION_TYPE_P (type))
     {
       /* Remove members that are not FIELD_DECLs from the field list