diff mbox series

Clear more useless flags

Message ID 20181107141724.w6azzxooz7hxkr2t@kam.mff.cuni.cz
State New
Headers show
Series Clear more useless flags | expand

Commit Message

Jan Hubicka Nov. 7, 2018, 2:17 p.m. UTC
Hi,
this patch enables bit more merging by clearing more flags that are
unnecesary and differ in practice across different copies of same ODR
types.

lto-bootstrapped/regtested x86_64-linux, OK?
	* tree.c (fld_incomplete_type_of): Clear TREE_ADDRESSABLE flag.
	(free_lang_data_in_decl): Set TREE_ADDRESSABLE for public functions
	and variables; clear DECL_EXTERNAL, TYPE_DECL_SUPPRESS_DEBUG and
	DECL_MODE on TYPE_DECLs.

Comments

Richard Biener Nov. 7, 2018, 2:47 p.m. UTC | #1
On Wed, 7 Nov 2018, Jan Hubicka wrote:

> Hi,
> this patch enables bit more merging by clearing more flags that are
> unnecesary and differ in practice across different copies of same ODR
> types.
> 
> lto-bootstrapped/regtested x86_64-linux, OK?
> 	* tree.c (fld_incomplete_type_of): Clear TREE_ADDRESSABLE flag.
> 	(free_lang_data_in_decl): Set TREE_ADDRESSABLE for public functions
> 	and variables; clear DECL_EXTERNAL, TYPE_DECL_SUPPRESS_DEBUG and
> 	DECL_MODE on TYPE_DECLs.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 265875)
> +++ tree.c	(working copy)
> @@ -5197,6 +5197,7 @@ fld_incomplete_type_of (tree t, struct f
>  	  TYPE_SIZE_UNIT (copy) = NULL;
>  	  TYPE_CANONICAL (copy) = TYPE_CANONICAL (t);
>  	  TYPE_TYPELESS_STORAGE (copy) = 0;
> +	  TREE_ADDRESSABLE (copy) = 0;
>  	  if (AGGREGATE_TYPE_P (t))
>  	    {
>  	      TYPE_FIELDS (copy) = NULL;
> @@ -5496,6 +5497,17 @@ free_lang_data_in_decl (tree decl, struc
>   if (TREE_CODE (decl) == FUNCTION_DECL)
>      {
>        struct cgraph_node *node;
> +      /* Frontends do not set TREE_ADDRESSABLE on public variables even though
> +	 the address may be taken in other unit, so this flag has no practical
> +	 use for middle-end.
> +
> +	 It would make more sense if frontends set TREE_ADDRESSABLE to 0 only
> +	 for public objects that indeed can not be adressed, but it is not
> +	 the case.  Set the flag to true so we do not get merge failures for
> +	 i.e. virtual tables between units that take address of it and
> +	 units that don't.  */
> +      if (TREE_PUBLIC (decl))
> +	TREE_ADDRESSABLE (decl) = true;
>        TREE_TYPE (decl) = fld_simplified_type (TREE_TYPE (decl), fld);
>        if (!(node = cgraph_node::get (decl))
>  	  || (!node->definition && !node->clones))
> @@ -5551,6 +5563,9 @@ free_lang_data_in_decl (tree decl, struc
>      }
>    else if (VAR_P (decl))
>      {
> +      /* See comment above why we set the flag for functions.  */
> +      if (TREE_PUBLIC (decl))
> +	TREE_ADDRESSABLE (decl) = true;
>        if ((DECL_EXTERNAL (decl)
>  	   && (!TREE_STATIC (decl) || !TREE_READONLY (decl)))
>  	  || (decl_function_context (decl) && !TREE_STATIC (decl)))
> @@ -5560,8 +5575,12 @@ free_lang_data_in_decl (tree decl, struc
>      {
>        DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
> +      /* TREE_PUBLIC is used to tell if type is anonymous.  */
> +      DECL_EXTERNAL (decl) = 0;
> +      TYPE_DECL_SUPPRESS_DEBUG (decl) = 0;

DECL_EXTERNAL and TYPE_DECL_SUPPRESS_DEBUG map to the same decl_flag_1 ...
so I'd say you should use TYPE_DECL_SUPPRESS_DEBUG only here.

>        DECL_INITIAL (decl) = NULL_TREE;
>        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> +      DECL_MODE (decl) = VOIDmode;
>        TREE_TYPE (decl) = void_type_node;
>        SET_DECL_ALIGN (decl, 0);
>      }

OK with that change.

Thanks,
Richard.
Jan Hubicka Nov. 7, 2018, 3:08 p.m. UTC | #2
> > +      /* TREE_PUBLIC is used to tell if type is anonymous.  */
> > +      DECL_EXTERNAL (decl) = 0;
> > +      TYPE_DECL_SUPPRESS_DEBUG (decl) = 0;
> 
> DECL_EXTERNAL and TYPE_DECL_SUPPRESS_DEBUG map to the same decl_flag_1 ...
> so I'd say you should use TYPE_DECL_SUPPRESS_DEBUG only here.

I see, print_tree prints both that is how I added both. Probably
something to fix :)

thanks!
Honza
> 
> >        DECL_INITIAL (decl) = NULL_TREE;
> >        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> > +      DECL_MODE (decl) = VOIDmode;
> >        TREE_TYPE (decl) = void_type_node;
> >        SET_DECL_ALIGN (decl, 0);
> >      }
> 
> OK with that change.
> 
> Thanks,
> Richard.
Richard Biener Nov. 7, 2018, 3:14 p.m. UTC | #3
On Wed, 7 Nov 2018, Jan Hubicka wrote:

> > > +      /* TREE_PUBLIC is used to tell if type is anonymous.  */
> > > +      DECL_EXTERNAL (decl) = 0;
> > > +      TYPE_DECL_SUPPRESS_DEBUG (decl) = 0;
> > 
> > DECL_EXTERNAL and TYPE_DECL_SUPPRESS_DEBUG map to the same decl_flag_1 ...
> > so I'd say you should use TYPE_DECL_SUPPRESS_DEBUG only here.
> 
> I see, print_tree prints both that is how I added both. Probably
> something to fix :)

Yeah ;)

> thanks!
> Honza
> > 
> > >        DECL_INITIAL (decl) = NULL_TREE;
> > >        DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> > > +      DECL_MODE (decl) = VOIDmode;
> > >        TREE_TYPE (decl) = void_type_node;
> > >        SET_DECL_ALIGN (decl, 0);
> > >      }
> > 
> > OK with that change.
> > 
> > Thanks,
> > Richard.
> 
>
diff mbox series

Patch

Index: tree.c
===================================================================
--- tree.c	(revision 265875)
+++ tree.c	(working copy)
@@ -5197,6 +5197,7 @@  fld_incomplete_type_of (tree t, struct f
 	  TYPE_SIZE_UNIT (copy) = NULL;
 	  TYPE_CANONICAL (copy) = TYPE_CANONICAL (t);
 	  TYPE_TYPELESS_STORAGE (copy) = 0;
+	  TREE_ADDRESSABLE (copy) = 0;
 	  if (AGGREGATE_TYPE_P (t))
 	    {
 	      TYPE_FIELDS (copy) = NULL;
@@ -5496,6 +5497,17 @@  free_lang_data_in_decl (tree decl, struc
  if (TREE_CODE (decl) == FUNCTION_DECL)
     {
       struct cgraph_node *node;
+      /* Frontends do not set TREE_ADDRESSABLE on public variables even though
+	 the address may be taken in other unit, so this flag has no practical
+	 use for middle-end.
+
+	 It would make more sense if frontends set TREE_ADDRESSABLE to 0 only
+	 for public objects that indeed can not be adressed, but it is not
+	 the case.  Set the flag to true so we do not get merge failures for
+	 i.e. virtual tables between units that take address of it and
+	 units that don't.  */
+      if (TREE_PUBLIC (decl))
+	TREE_ADDRESSABLE (decl) = true;
       TREE_TYPE (decl) = fld_simplified_type (TREE_TYPE (decl), fld);
       if (!(node = cgraph_node::get (decl))
 	  || (!node->definition && !node->clones))
@@ -5551,6 +5563,9 @@  free_lang_data_in_decl (tree decl, struc
     }
   else if (VAR_P (decl))
     {
+      /* See comment above why we set the flag for functions.  */
+      if (TREE_PUBLIC (decl))
+	TREE_ADDRESSABLE (decl) = true;
       if ((DECL_EXTERNAL (decl)
 	   && (!TREE_STATIC (decl) || !TREE_READONLY (decl)))
 	  || (decl_function_context (decl) && !TREE_STATIC (decl)))
@@ -5560,8 +5575,12 @@  free_lang_data_in_decl (tree decl, struc
     {
       DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
       DECL_VISIBILITY_SPECIFIED (decl) = 0;
+      /* TREE_PUBLIC is used to tell if type is anonymous.  */
+      DECL_EXTERNAL (decl) = 0;
+      TYPE_DECL_SUPPRESS_DEBUG (decl) = 0;
       DECL_INITIAL (decl) = NULL_TREE;
       DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
+      DECL_MODE (decl) = VOIDmode;
       TREE_TYPE (decl) = void_type_node;
       SET_DECL_ALIGN (decl, 0);
     }