Do not stream TYPE_NEEDS_CONSTRUCTING

Hi,
TYPE_NEEDS_CONSTRUCTING is one of reasons why we get duplicated complete and
incomplete types after my patch because the incoplete type I construct
may have TYPE_NEEDS_CONSTRUCTING set.

I think this flag is useless and can be dropped - the only use in ipa-pure-const
seems confused since we should drop the readonly flag on such variables.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* ipa-pure-const.c (check_decl): Do not test TYPE_NEEDS_CONSTRUCTING.
	* lto-streamer-out.c (hash_tree): Do not hash TYPE_NEEDS_CONSTRUCTING.
	* tree-streamer-in.c (unpack_ts_type_common_value_fields): Do not
	stream TYPE_NEEDS_CONSTRUCTING.
	* tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise.
	* tree.c (free_lang_data_in_type): Clear TYPE_NEEDS_CONSTRUCTING.

On Tue, 6 Nov 2018, Jan Hubicka wrote:

> Hi,
> TYPE_NEEDS_CONSTRUCTING is one of reasons why we get duplicated complete and
> incomplete types after my patch because the incoplete type I construct
> may have TYPE_NEEDS_CONSTRUCTING set.
> 
> I think this flag is useless and can be dropped - the only use in ipa-pure-const
> seems confused since we should drop the readonly flag on such variables.

Did you check that with an assert?

Otherwise OK.

Richard.

> 
> Bootstrapped/regtested x86_64-linux, OK?
> 
> Honza
> 
> 	* ipa-pure-const.c (check_decl): Do not test TYPE_NEEDS_CONSTRUCTING.
> 	* lto-streamer-out.c (hash_tree): Do not hash TYPE_NEEDS_CONSTRUCTING.
> 	* tree-streamer-in.c (unpack_ts_type_common_value_fields): Do not
> 	stream TYPE_NEEDS_CONSTRUCTING.
> 	* tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise.
> 	* tree.c (free_lang_data_in_type): Clear TYPE_NEEDS_CONSTRUCTING.
> Index: ipa-pure-const.c
> ===================================================================
> --- ipa-pure-const.c	(revision 265738)
> +++ ipa-pure-const.c	(working copy)
> @@ -339,7 +339,7 @@ check_decl (funct_state local,
>    if (DECL_EXTERNAL (t) || TREE_PUBLIC (t))
>      {
>        /* Readonly reads are safe.  */
> -      if (TREE_READONLY (t) && !TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (t)))
> +      if (TREE_READONLY (t))
>  	return; /* Read of a constant, do not change the function state.  */
>        else
>  	{
> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c	(revision 265738)
> +++ lto-streamer-out.c	(working copy)
> @@ -1147,7 +1147,6 @@ hash_tree (struct streamer_tree_cache_d
>        hstate.add_flag (TYPE_STRING_FLAG (t));
>        /* TYPE_NO_FORCE_BLK is private to stor-layout and need
>   	 no streaming.  */
> -      hstate.add_flag (TYPE_NEEDS_CONSTRUCTING (t));
>        hstate.add_flag (TYPE_PACKED (t));
>        hstate.add_flag (TYPE_RESTRICT (t));
>        hstate.add_flag (TYPE_USER_ALIGN (t));
> Index: tree-streamer-in.c
> ===================================================================
> --- tree-streamer-in.c	(revision 265738)
> +++ tree-streamer-in.c	(working copy)
> @@ -367,7 +367,6 @@ unpack_ts_type_common_value_fields (stru
>    TYPE_STRING_FLAG (expr) = (unsigned) bp_unpack_value (bp, 1);
>    /* TYPE_NO_FORCE_BLK is private to stor-layout and need
>       no streaming.  */
> -  TYPE_NEEDS_CONSTRUCTING (expr) = (unsigned) bp_unpack_value (bp, 1);
>    TYPE_PACKED (expr) = (unsigned) bp_unpack_value (bp, 1);
>    TYPE_RESTRICT (expr) = (unsigned) bp_unpack_value (bp, 1);
>    TYPE_USER_ALIGN (expr) = (unsigned) bp_unpack_value (bp, 1);
> Index: tree-streamer-out.c
> ===================================================================
> --- tree-streamer-out.c	(revision 265738)
> +++ tree-streamer-out.c	(working copy)
> @@ -314,7 +314,6 @@ pack_ts_type_common_value_fields (struct
>    bp_pack_value (bp, TYPE_STRING_FLAG (expr), 1);
>    /* TYPE_NO_FORCE_BLK is private to stor-layout and need
>       no streaming.  */
> -  bp_pack_value (bp, TYPE_NEEDS_CONSTRUCTING (expr), 1);
>    bp_pack_value (bp, TYPE_PACKED (expr), 1);
>    bp_pack_value (bp, TYPE_RESTRICT (expr), 1);
>    bp_pack_value (bp, TYPE_USER_ALIGN (expr), 1);
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 265738)
> +++ tree.c	(working copy)
> @@ -5254,6 +5254,8 @@ free_lang_data_in_type (tree type)
>    TREE_LANG_FLAG_5 (type) = 0;
>    TREE_LANG_FLAG_6 (type) = 0;
>  
> +  TYPE_NEEDS_CONSTRUCTING (type) = 0;
> +
>    if (TREE_CODE (type) == FUNCTION_TYPE)
>      {
>        /* Remove the const and volatile qualifiers from arguments.  The
> 
>

> On Tue, 6 Nov 2018, Jan Hubicka wrote:
> 
> > Hi,
> > TYPE_NEEDS_CONSTRUCTING is one of reasons why we get duplicated complete and
> > incomplete types after my patch because the incoplete type I construct
> > may have TYPE_NEEDS_CONSTRUCTING set.
> > 
> > I think this flag is useless and can be dropped - the only use in ipa-pure-const
> > seems confused since we should drop the readonly flag on such variables.
> 
> Did you check that with an assert?

You can't assert on this easily because constructor may end up being
optimized out and the variable promoted to readonly for valid reasons.

Honza

On Tue, 6 Nov 2018, Jan Hubicka wrote:

> > On Tue, 6 Nov 2018, Jan Hubicka wrote:
> > 
> > > Hi,
> > > TYPE_NEEDS_CONSTRUCTING is one of reasons why we get duplicated complete and
> > > incomplete types after my patch because the incoplete type I construct
> > > may have TYPE_NEEDS_CONSTRUCTING set.
> > > 
> > > I think this flag is useless and can be dropped - the only use in ipa-pure-const
> > > seems confused since we should drop the readonly flag on such variables.
> > 
> > Did you check that with an assert?
> 
> You can't assert on this easily because constructor may end up being
> optimized out and the variable promoted to readonly for valid reasons.

Hmm, I see.