diff mbox series

Cleanup DECL streaming

Message ID 20180621131850.GG84841@kam.mff.cuni.cz
State New
Headers show
Series Cleanup DECL streaming | expand

Commit Message

Jan Hubicka June 21, 2018, 1:18 p.m. UTC
Hi,
this patch drops DECL_ORIGINAL_TYPE streaming and also logic handling
external decls in blocks since we no longer stream them at all.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* lto-streamer-out.c (DFS::DFS_write_tree_body): Do not
	stream DECL_ORIGINAL_TYPE.
	(DFS::DFS_write_tree_body): Drop hack handling local external decls.
	(hash_tree): Do not walk DECL_ORIGINAL_TYPE.
	* tree-streamer-in.c (lto_input_ts_decl_non_common_tree_pointers):
	Do not walk original type.
	* tree-streamer-out.c (streamer_write_chain): Drop hack handling
	external decls.
	(write_ts_decl_non_common_tree_pointers): Do not stream
	DECL_ORIGINAL_TYPE
	* tree.c (free_lang_data_in_decl): Clear DECL_ORIGINAL_TYPE.
	(find_decls_types_r): Do not walk DEC_ORIGINAL_TYPE.

Comments

Jan Hubicka June 21, 2018, 1:29 p.m. UTC | #1
> Hi,
> this patch drops DECL_ORIGINAL_TYPE streaming and also logic handling
> external decls in blocks since we no longer stream them at all.
> 
> Bootstrapped/regtested x86_64-linux, OK?
Actually the patch dies with lto-bootstrap :(
0x9c44fc gen_member_die
        ../../gcc/dwarf2out.c:24947
0x9c4bb1 gen_struct_or_union_type_die
        ../../gcc/dwarf2out.c:25143
0x9c52dd gen_tagged_type_die
        ../../gcc/dwarf2out.c:25353
0x9c4ffe gen_typedef_die
        ../../gcc/dwarf2out.c:25259
0x9c6e5f gen_decl_die
        ../../gcc/dwarf2out.c:26248
0x9c5557 gen_type_die_with_usage
        ../../gcc/dwarf2out.c:25424
0x9c5bc2 gen_type_die
        ../../gcc/dwarf2out.c:25608
0x9a7756 modified_type_die
        ../../gcc/dwarf2out.c:13396
0x9bd011 add_type_attribute
        ../../gcc/dwarf2out.c:21523
0x9bf854 gen_subprogram_die
        ../../gcc/dwarf2out.c:22835
0x9c6d8c gen_decl_die
        ../../gcc/dwarf2out.c:26222
0x9c7db9 dwarf2out_decl
        ../../gcc/dwarf2out.c:26787
0x9c7e15 dwarf2out_function_decl
        ../../gcc/dwarf2out.c:26802
0xa65bd9 rest_of_handle_final
        ../../gcc/final.c:4704
0xa65d46 execute
        ../../gcc/final.c:4746

I am going to respawn it becuase it seems odd that stage2 built but stage3
failed. Clearly typeefs are determined by existence of original type:

/* Returns true if X is a typedef decl.  */

bool
is_typedef_decl (const_tree x)
{
  return (x && TREE_CODE (x) == TYPE_DECL
          && DECL_ORIGINAL_TYPE (x) != NULL_TREE);
}

/* Returns true iff TYPE is a type variant created for a typedef. */

bool
typedef_variant_p (const_tree type)
{
  return is_typedef_decl (TYPE_NAME (type));
}

however aren't we supposed to not touch these at late builds? We drop most of TYPE_DECLs in favour
of IDENTIFIER_TYPE and thus also throwing away DECL_ORIGINAL_TYPEs.

Honza
> 
> Honza
> 
> 	* lto-streamer-out.c (DFS::DFS_write_tree_body): Do not
> 	stream DECL_ORIGINAL_TYPE.
> 	(DFS::DFS_write_tree_body): Drop hack handling local external decls.
> 	(hash_tree): Do not walk DECL_ORIGINAL_TYPE.
> 	* tree-streamer-in.c (lto_input_ts_decl_non_common_tree_pointers):
> 	Do not walk original type.
> 	* tree-streamer-out.c (streamer_write_chain): Drop hack handling
> 	external decls.
> 	(write_ts_decl_non_common_tree_pointers): Do not stream
> 	DECL_ORIGINAL_TYPE
> 	* tree.c (free_lang_data_in_decl): Clear DECL_ORIGINAL_TYPE.
> 	(find_decls_types_r): Do not walk DEC_ORIGINAL_TYPE.
> 
> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c	(revision 261841)
> +++ lto-streamer-out.c	(working copy)
> @@ -819,12 +819,6 @@ DFS::DFS_write_tree_body (struct output_
>  	DFS_follow_tree_edge (DECL_DEBUG_EXPR (expr));
>      }
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> -    {
> -      if (TREE_CODE (expr) == TYPE_DECL)
> -	DFS_follow_tree_edge (DECL_ORIGINAL_TYPE (expr));
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
>      {
>        /* Make sure we don't inadvertently set the assembler name.  */
> @@ -907,14 +901,13 @@ DFS::DFS_write_tree_body (struct output_
>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
>      {
>        for (tree t = BLOCK_VARS (expr); t; t = TREE_CHAIN (t))
> -	if (VAR_OR_FUNCTION_DECL_P (t)
> -	    && DECL_EXTERNAL (t))
> -	  /* We have to stream externals in the block chain as
> -	     non-references.  See also
> -	     tree-streamer-out.c:streamer_write_chain.  */
> -	  DFS_write_tree (ob, expr_state, t, ref_p, false);
> -	else
> +	{
> +	  /* We would have to stream externals in the block chain as
> +	     non-references but we should have dropped them in
> +	     free-lang-data.  */
> +	  gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
>  	  DFS_follow_tree_edge (t);
> +	}
>  
>        DFS_follow_tree_edge (BLOCK_SUPERCONTEXT (expr));
>  
> @@ -1261,12 +1249,6 @@ hash_tree (struct streamer_tree_cache_d
>           be able to call get_symbol_initial_value.  */
>      }
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> -    {
> -      if (code == TYPE_DECL)
> -	visit (DECL_ORIGINAL_TYPE (t));
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
>      {
>        if (DECL_ASSEMBLER_NAME_SET_P (t))
> Index: tree-streamer-in.c
> ===================================================================
> --- tree-streamer-in.c	(revision 261841)
> +++ tree-streamer-in.c	(working copy)
> @@ -728,11 +721,9 @@ lto_input_ts_decl_common_tree_pointers (
>     file being read.  */
>  
>  static void
> -lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *ib,
> -					    struct data_in *data_in, tree expr)
> +lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *,
> +					    struct data_in *, tree)
>  {
> -  if (TREE_CODE (expr) == TYPE_DECL)
> -    DECL_ORIGINAL_TYPE (expr) = stream_read_tree (ib, data_in);
>  }
>  
>  
> Index: tree-streamer-out.c
> ===================================================================
> --- tree-streamer-out.c	(revision 261841)
> +++ tree-streamer-out.c	(working copy)
> @@ -497,14 +494,10 @@ streamer_write_chain (struct output_bloc
>      {
>        /* We avoid outputting external vars or functions by reference
>  	 to the global decls section as we do not want to have them
> -	 enter decl merging.  This is, of course, only for the call
> -	 for streaming BLOCK_VARS, but other callers are safe.
> -	 See also lto-streamer-out.c:DFS_write_tree_body.  */
> -      if (VAR_OR_FUNCTION_DECL_P (t)
> -	  && DECL_EXTERNAL (t))
> -	stream_write_tree_shallow_non_ref (ob, t, ref_p);
> -      else
> -	stream_write_tree (ob, t, ref_p);
> +	 enter decl merging.  We should not need to do this anymore because
> +	 free_lang_data removes them from block scopes.  */
> +      gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
> +      stream_write_tree (ob, t, ref_p);
>  
>        t = TREE_CHAIN (t);
>      }
> @@ -620,11 +613,8 @@ write_ts_decl_common_tree_pointers (stru
>     pointer fields.  */
>  
>  static void
> -write_ts_decl_non_common_tree_pointers (struct output_block *ob, tree expr,
> -				        bool ref_p)
> +write_ts_decl_non_common_tree_pointers (struct output_block *, tree, bool)
>  {
> -  if (TREE_CODE (expr) == TYPE_DECL)
> -    stream_write_tree (ob, DECL_ORIGINAL_TYPE (expr), ref_p);
>  }
>  
>  
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 261841)
> +++ tree.c	(working copy)
> @@ -5359,6 +5357,7 @@ free_lang_data_in_decl (tree decl)
>        DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
>        DECL_INITIAL (decl) = NULL_TREE;
> +      DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
>      }
>    else if (TREE_CODE (decl) == FIELD_DECL)
>      DECL_INITIAL (decl) = NULL_TREE;
> @@ -5471,10 +5470,6 @@ find_decls_types_r (tree *tp, int *ws, v
>  	  fld_worklist_push (DECL_ARGUMENTS (t), fld);
>  	  fld_worklist_push (DECL_RESULT (t), fld);
>  	}
> -      else if (TREE_CODE (t) == TYPE_DECL)
> -	{
> -	  fld_worklist_push (DECL_ORIGINAL_TYPE (t), fld);
> -	}
>        else if (TREE_CODE (t) == FIELD_DECL)
>  	{
>  	  fld_worklist_push (DECL_FIELD_OFFSET (t), fld);
Richard Biener June 21, 2018, 1:37 p.m. UTC | #2
On Thu, 21 Jun 2018, Jan Hubicka wrote:

> Hi,
> this patch drops DECL_ORIGINAL_TYPE streaming and also logic handling
> external decls in blocks since we no longer stream them at all.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.

Thanks,
Richard.

> Honza
> 
> 	* lto-streamer-out.c (DFS::DFS_write_tree_body): Do not
> 	stream DECL_ORIGINAL_TYPE.
> 	(DFS::DFS_write_tree_body): Drop hack handling local external decls.
> 	(hash_tree): Do not walk DECL_ORIGINAL_TYPE.
> 	* tree-streamer-in.c (lto_input_ts_decl_non_common_tree_pointers):
> 	Do not walk original type.
> 	* tree-streamer-out.c (streamer_write_chain): Drop hack handling
> 	external decls.
> 	(write_ts_decl_non_common_tree_pointers): Do not stream
> 	DECL_ORIGINAL_TYPE
> 	* tree.c (free_lang_data_in_decl): Clear DECL_ORIGINAL_TYPE.
> 	(find_decls_types_r): Do not walk DEC_ORIGINAL_TYPE.
> 
> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c	(revision 261841)
> +++ lto-streamer-out.c	(working copy)
> @@ -819,12 +819,6 @@ DFS::DFS_write_tree_body (struct output_
>  	DFS_follow_tree_edge (DECL_DEBUG_EXPR (expr));
>      }
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> -    {
> -      if (TREE_CODE (expr) == TYPE_DECL)
> -	DFS_follow_tree_edge (DECL_ORIGINAL_TYPE (expr));
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
>      {
>        /* Make sure we don't inadvertently set the assembler name.  */
> @@ -907,14 +901,13 @@ DFS::DFS_write_tree_body (struct output_
>    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
>      {
>        for (tree t = BLOCK_VARS (expr); t; t = TREE_CHAIN (t))
> -	if (VAR_OR_FUNCTION_DECL_P (t)
> -	    && DECL_EXTERNAL (t))
> -	  /* We have to stream externals in the block chain as
> -	     non-references.  See also
> -	     tree-streamer-out.c:streamer_write_chain.  */
> -	  DFS_write_tree (ob, expr_state, t, ref_p, false);
> -	else
> +	{
> +	  /* We would have to stream externals in the block chain as
> +	     non-references but we should have dropped them in
> +	     free-lang-data.  */
> +	  gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
>  	  DFS_follow_tree_edge (t);
> +	}
>  
>        DFS_follow_tree_edge (BLOCK_SUPERCONTEXT (expr));
>  
> @@ -1261,12 +1249,6 @@ hash_tree (struct streamer_tree_cache_d
>           be able to call get_symbol_initial_value.  */
>      }
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> -    {
> -      if (code == TYPE_DECL)
> -	visit (DECL_ORIGINAL_TYPE (t));
> -    }
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
>      {
>        if (DECL_ASSEMBLER_NAME_SET_P (t))
> Index: tree-streamer-in.c
> ===================================================================
> --- tree-streamer-in.c	(revision 261841)
> +++ tree-streamer-in.c	(working copy)
> @@ -728,11 +721,9 @@ lto_input_ts_decl_common_tree_pointers (
>     file being read.  */
>  
>  static void
> -lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *ib,
> -					    struct data_in *data_in, tree expr)
> +lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *,
> +					    struct data_in *, tree)
>  {
> -  if (TREE_CODE (expr) == TYPE_DECL)
> -    DECL_ORIGINAL_TYPE (expr) = stream_read_tree (ib, data_in);
>  }
>  
>  
> Index: tree-streamer-out.c
> ===================================================================
> --- tree-streamer-out.c	(revision 261841)
> +++ tree-streamer-out.c	(working copy)
> @@ -497,14 +494,10 @@ streamer_write_chain (struct output_bloc
>      {
>        /* We avoid outputting external vars or functions by reference
>  	 to the global decls section as we do not want to have them
> -	 enter decl merging.  This is, of course, only for the call
> -	 for streaming BLOCK_VARS, but other callers are safe.
> -	 See also lto-streamer-out.c:DFS_write_tree_body.  */
> -      if (VAR_OR_FUNCTION_DECL_P (t)
> -	  && DECL_EXTERNAL (t))
> -	stream_write_tree_shallow_non_ref (ob, t, ref_p);
> -      else
> -	stream_write_tree (ob, t, ref_p);
> +	 enter decl merging.  We should not need to do this anymore because
> +	 free_lang_data removes them from block scopes.  */
> +      gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
> +      stream_write_tree (ob, t, ref_p);
>  
>        t = TREE_CHAIN (t);
>      }
> @@ -620,11 +613,8 @@ write_ts_decl_common_tree_pointers (stru
>     pointer fields.  */
>  
>  static void
> -write_ts_decl_non_common_tree_pointers (struct output_block *ob, tree expr,
> -				        bool ref_p)
> +write_ts_decl_non_common_tree_pointers (struct output_block *, tree, bool)
>  {
> -  if (TREE_CODE (expr) == TYPE_DECL)
> -    stream_write_tree (ob, DECL_ORIGINAL_TYPE (expr), ref_p);
>  }
>  
>  
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 261841)
> +++ tree.c	(working copy)
> @@ -5359,6 +5357,7 @@ free_lang_data_in_decl (tree decl)
>        DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
>        DECL_VISIBILITY_SPECIFIED (decl) = 0;
>        DECL_INITIAL (decl) = NULL_TREE;
> +      DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
>      }
>    else if (TREE_CODE (decl) == FIELD_DECL)
>      DECL_INITIAL (decl) = NULL_TREE;
> @@ -5471,10 +5470,6 @@ find_decls_types_r (tree *tp, int *ws, v
>  	  fld_worklist_push (DECL_ARGUMENTS (t), fld);
>  	  fld_worklist_push (DECL_RESULT (t), fld);
>  	}
> -      else if (TREE_CODE (t) == TYPE_DECL)
> -	{
> -	  fld_worklist_push (DECL_ORIGINAL_TYPE (t), fld);
> -	}
>        else if (TREE_CODE (t) == FIELD_DECL)
>  	{
>  	  fld_worklist_push (DECL_FIELD_OFFSET (t), fld);
> 
>
Richard Biener June 21, 2018, 1:40 p.m. UTC | #3
On Thu, 21 Jun 2018, Jan Hubicka wrote:

> > Hi,
> > this patch drops DECL_ORIGINAL_TYPE streaming and also logic handling
> > external decls in blocks since we no longer stream them at all.
> > 
> > Bootstrapped/regtested x86_64-linux, OK?
> Actually the patch dies with lto-bootstrap :(
> 0x9c44fc gen_member_die
>         ../../gcc/dwarf2out.c:24947
> 0x9c4bb1 gen_struct_or_union_type_die
>         ../../gcc/dwarf2out.c:25143
> 0x9c52dd gen_tagged_type_die
>         ../../gcc/dwarf2out.c:25353
> 0x9c4ffe gen_typedef_die
>         ../../gcc/dwarf2out.c:25259
> 0x9c6e5f gen_decl_die
>         ../../gcc/dwarf2out.c:26248
> 0x9c5557 gen_type_die_with_usage
>         ../../gcc/dwarf2out.c:25424
> 0x9c5bc2 gen_type_die
>         ../../gcc/dwarf2out.c:25608
> 0x9a7756 modified_type_die
>         ../../gcc/dwarf2out.c:13396
> 0x9bd011 add_type_attribute
>         ../../gcc/dwarf2out.c:21523
> 0x9bf854 gen_subprogram_die
>         ../../gcc/dwarf2out.c:22835
> 0x9c6d8c gen_decl_die
>         ../../gcc/dwarf2out.c:26222
> 0x9c7db9 dwarf2out_decl
>         ../../gcc/dwarf2out.c:26787
> 0x9c7e15 dwarf2out_function_decl
>         ../../gcc/dwarf2out.c:26802
> 0xa65bd9 rest_of_handle_final
>         ../../gcc/final.c:4704
> 0xa65d46 execute
>         ../../gcc/final.c:4746
> 
> I am going to respawn it becuase it seems odd that stage2 built but stage3
> failed. Clearly typeefs are determined by existence of original type:
> 
> /* Returns true if X is a typedef decl.  */
> 
> bool
> is_typedef_decl (const_tree x)
> {
>   return (x && TREE_CODE (x) == TYPE_DECL
>           && DECL_ORIGINAL_TYPE (x) != NULL_TREE);
> }
> 
> /* Returns true iff TYPE is a type variant created for a typedef. */
> 
> bool
> typedef_variant_p (const_tree type)
> {
>   return is_typedef_decl (TYPE_NAME (type));
> }
> 
> however aren't we supposed to not touch these at late builds? We drop most of TYPE_DECLs in favour
> of IDENTIFIER_TYPE and thus also throwing away DECL_ORIGINAL_TYPEs.

We keep quite a bit of TYPE_DECLs around for devirt.

We shouldn't (very often...) end up trying to emit type DIEs late.
Here we're running into

          /* If the prototype had an 'auto' or 'decltype(auto)' return 
type,
             emit the real type on the definition die.  */
          if (is_cxx () && debug_info_level > DINFO_LEVEL_TERSE)
            {
              dw_die_ref die = get_AT_ref (old_die, DW_AT_type);

but that's odd since get_AT_ref shoudn't be able to lookup DW_AT_type
in lto1.

So a testcase would be nice to have...

Richard.


> Honza
> > 
> > Honza
> > 
> > 	* lto-streamer-out.c (DFS::DFS_write_tree_body): Do not
> > 	stream DECL_ORIGINAL_TYPE.
> > 	(DFS::DFS_write_tree_body): Drop hack handling local external decls.
> > 	(hash_tree): Do not walk DECL_ORIGINAL_TYPE.
> > 	* tree-streamer-in.c (lto_input_ts_decl_non_common_tree_pointers):
> > 	Do not walk original type.
> > 	* tree-streamer-out.c (streamer_write_chain): Drop hack handling
> > 	external decls.
> > 	(write_ts_decl_non_common_tree_pointers): Do not stream
> > 	DECL_ORIGINAL_TYPE
> > 	* tree.c (free_lang_data_in_decl): Clear DECL_ORIGINAL_TYPE.
> > 	(find_decls_types_r): Do not walk DEC_ORIGINAL_TYPE.
> > 
> > Index: lto-streamer-out.c
> > ===================================================================
> > --- lto-streamer-out.c	(revision 261841)
> > +++ lto-streamer-out.c	(working copy)
> > @@ -819,12 +819,6 @@ DFS::DFS_write_tree_body (struct output_
> >  	DFS_follow_tree_edge (DECL_DEBUG_EXPR (expr));
> >      }
> >  
> > -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> > -    {
> > -      if (TREE_CODE (expr) == TYPE_DECL)
> > -	DFS_follow_tree_edge (DECL_ORIGINAL_TYPE (expr));
> > -    }
> > -
> >    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
> >      {
> >        /* Make sure we don't inadvertently set the assembler name.  */
> > @@ -907,14 +901,13 @@ DFS::DFS_write_tree_body (struct output_
> >    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
> >      {
> >        for (tree t = BLOCK_VARS (expr); t; t = TREE_CHAIN (t))
> > -	if (VAR_OR_FUNCTION_DECL_P (t)
> > -	    && DECL_EXTERNAL (t))
> > -	  /* We have to stream externals in the block chain as
> > -	     non-references.  See also
> > -	     tree-streamer-out.c:streamer_write_chain.  */
> > -	  DFS_write_tree (ob, expr_state, t, ref_p, false);
> > -	else
> > +	{
> > +	  /* We would have to stream externals in the block chain as
> > +	     non-references but we should have dropped them in
> > +	     free-lang-data.  */
> > +	  gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
> >  	  DFS_follow_tree_edge (t);
> > +	}
> >  
> >        DFS_follow_tree_edge (BLOCK_SUPERCONTEXT (expr));
> >  
> > @@ -1261,12 +1249,6 @@ hash_tree (struct streamer_tree_cache_d
> >           be able to call get_symbol_initial_value.  */
> >      }
> >  
> > -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> > -    {
> > -      if (code == TYPE_DECL)
> > -	visit (DECL_ORIGINAL_TYPE (t));
> > -    }
> > -
> >    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
> >      {
> >        if (DECL_ASSEMBLER_NAME_SET_P (t))
> > Index: tree-streamer-in.c
> > ===================================================================
> > --- tree-streamer-in.c	(revision 261841)
> > +++ tree-streamer-in.c	(working copy)
> > @@ -728,11 +721,9 @@ lto_input_ts_decl_common_tree_pointers (
> >     file being read.  */
> >  
> >  static void
> > -lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *ib,
> > -					    struct data_in *data_in, tree expr)
> > +lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *,
> > +					    struct data_in *, tree)
> >  {
> > -  if (TREE_CODE (expr) == TYPE_DECL)
> > -    DECL_ORIGINAL_TYPE (expr) = stream_read_tree (ib, data_in);
> >  }
> >  
> >  
> > Index: tree-streamer-out.c
> > ===================================================================
> > --- tree-streamer-out.c	(revision 261841)
> > +++ tree-streamer-out.c	(working copy)
> > @@ -497,14 +494,10 @@ streamer_write_chain (struct output_bloc
> >      {
> >        /* We avoid outputting external vars or functions by reference
> >  	 to the global decls section as we do not want to have them
> > -	 enter decl merging.  This is, of course, only for the call
> > -	 for streaming BLOCK_VARS, but other callers are safe.
> > -	 See also lto-streamer-out.c:DFS_write_tree_body.  */
> > -      if (VAR_OR_FUNCTION_DECL_P (t)
> > -	  && DECL_EXTERNAL (t))
> > -	stream_write_tree_shallow_non_ref (ob, t, ref_p);
> > -      else
> > -	stream_write_tree (ob, t, ref_p);
> > +	 enter decl merging.  We should not need to do this anymore because
> > +	 free_lang_data removes them from block scopes.  */
> > +      gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
> > +      stream_write_tree (ob, t, ref_p);
> >  
> >        t = TREE_CHAIN (t);
> >      }
> > @@ -620,11 +613,8 @@ write_ts_decl_common_tree_pointers (stru
> >     pointer fields.  */
> >  
> >  static void
> > -write_ts_decl_non_common_tree_pointers (struct output_block *ob, tree expr,
> > -				        bool ref_p)
> > +write_ts_decl_non_common_tree_pointers (struct output_block *, tree, bool)
> >  {
> > -  if (TREE_CODE (expr) == TYPE_DECL)
> > -    stream_write_tree (ob, DECL_ORIGINAL_TYPE (expr), ref_p);
> >  }
> >  
> >  
> > Index: tree.c
> > ===================================================================
> > --- tree.c	(revision 261841)
> > +++ tree.c	(working copy)
> > @@ -5359,6 +5357,7 @@ free_lang_data_in_decl (tree decl)
> >        DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
> >        DECL_VISIBILITY_SPECIFIED (decl) = 0;
> >        DECL_INITIAL (decl) = NULL_TREE;
> > +      DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> >      }
> >    else if (TREE_CODE (decl) == FIELD_DECL)
> >      DECL_INITIAL (decl) = NULL_TREE;
> > @@ -5471,10 +5470,6 @@ find_decls_types_r (tree *tp, int *ws, v
> >  	  fld_worklist_push (DECL_ARGUMENTS (t), fld);
> >  	  fld_worklist_push (DECL_RESULT (t), fld);
> >  	}
> > -      else if (TREE_CODE (t) == TYPE_DECL)
> > -	{
> > -	  fld_worklist_push (DECL_ORIGINAL_TYPE (t), fld);
> > -	}
> >        else if (TREE_CODE (t) == FIELD_DECL)
> >  	{
> >  	  fld_worklist_push (DECL_FIELD_OFFSET (t), fld);
> 
>
Jan Hubicka June 21, 2018, 1:54 p.m. UTC | #4
> > however aren't we supposed to not touch these at late builds? We drop most of TYPE_DECLs in favour
> > of IDENTIFIER_TYPE and thus also throwing away DECL_ORIGINAL_TYPEs.
> 
> We keep quite a bit of TYPE_DECLs around for devirt.

I know, but we do not keep them systematicaly enough to make them useful for dwarf2out.
So I would say dwarf2out touching them is a bug.

> 
> We shouldn't (very often...) end up trying to emit type DIEs late.
> Here we're running into
> 
>           /* If the prototype had an 'auto' or 'decltype(auto)' return 
> type,
>              emit the real type on the definition die.  */
>           if (is_cxx () && debug_info_level > DINFO_LEVEL_TERSE)
>             {
>               dw_die_ref die = get_AT_ref (old_die, DW_AT_type);
> 
> but that's odd since get_AT_ref shoudn't be able to lookup DW_AT_type
> in lto1.
> 
> So a testcase would be nice to have...

OK, I will try to debug into it.  Testcase is of course easy - apply patch and run
lto bootstrap :)

Honza

> 
> Richard.
> 
> 
> > Honza
> > > 
> > > Honza
> > > 
> > > 	* lto-streamer-out.c (DFS::DFS_write_tree_body): Do not
> > > 	stream DECL_ORIGINAL_TYPE.
> > > 	(DFS::DFS_write_tree_body): Drop hack handling local external decls.
> > > 	(hash_tree): Do not walk DECL_ORIGINAL_TYPE.
> > > 	* tree-streamer-in.c (lto_input_ts_decl_non_common_tree_pointers):
> > > 	Do not walk original type.
> > > 	* tree-streamer-out.c (streamer_write_chain): Drop hack handling
> > > 	external decls.
> > > 	(write_ts_decl_non_common_tree_pointers): Do not stream
> > > 	DECL_ORIGINAL_TYPE
> > > 	* tree.c (free_lang_data_in_decl): Clear DECL_ORIGINAL_TYPE.
> > > 	(find_decls_types_r): Do not walk DEC_ORIGINAL_TYPE.
> > > 
> > > Index: lto-streamer-out.c
> > > ===================================================================
> > > --- lto-streamer-out.c	(revision 261841)
> > > +++ lto-streamer-out.c	(working copy)
> > > @@ -819,12 +819,6 @@ DFS::DFS_write_tree_body (struct output_
> > >  	DFS_follow_tree_edge (DECL_DEBUG_EXPR (expr));
> > >      }
> > >  
> > > -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> > > -    {
> > > -      if (TREE_CODE (expr) == TYPE_DECL)
> > > -	DFS_follow_tree_edge (DECL_ORIGINAL_TYPE (expr));
> > > -    }
> > > -
> > >    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
> > >      {
> > >        /* Make sure we don't inadvertently set the assembler name.  */
> > > @@ -907,14 +901,13 @@ DFS::DFS_write_tree_body (struct output_
> > >    if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
> > >      {
> > >        for (tree t = BLOCK_VARS (expr); t; t = TREE_CHAIN (t))
> > > -	if (VAR_OR_FUNCTION_DECL_P (t)
> > > -	    && DECL_EXTERNAL (t))
> > > -	  /* We have to stream externals in the block chain as
> > > -	     non-references.  See also
> > > -	     tree-streamer-out.c:streamer_write_chain.  */
> > > -	  DFS_write_tree (ob, expr_state, t, ref_p, false);
> > > -	else
> > > +	{
> > > +	  /* We would have to stream externals in the block chain as
> > > +	     non-references but we should have dropped them in
> > > +	     free-lang-data.  */
> > > +	  gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
> > >  	  DFS_follow_tree_edge (t);
> > > +	}
> > >  
> > >        DFS_follow_tree_edge (BLOCK_SUPERCONTEXT (expr));
> > >  
> > > @@ -1261,12 +1249,6 @@ hash_tree (struct streamer_tree_cache_d
> > >           be able to call get_symbol_initial_value.  */
> > >      }
> > >  
> > > -  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
> > > -    {
> > > -      if (code == TYPE_DECL)
> > > -	visit (DECL_ORIGINAL_TYPE (t));
> > > -    }
> > > -
> > >    if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
> > >      {
> > >        if (DECL_ASSEMBLER_NAME_SET_P (t))
> > > Index: tree-streamer-in.c
> > > ===================================================================
> > > --- tree-streamer-in.c	(revision 261841)
> > > +++ tree-streamer-in.c	(working copy)
> > > @@ -728,11 +721,9 @@ lto_input_ts_decl_common_tree_pointers (
> > >     file being read.  */
> > >  
> > >  static void
> > > -lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *ib,
> > > -					    struct data_in *data_in, tree expr)
> > > +lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *,
> > > +					    struct data_in *, tree)
> > >  {
> > > -  if (TREE_CODE (expr) == TYPE_DECL)
> > > -    DECL_ORIGINAL_TYPE (expr) = stream_read_tree (ib, data_in);
> > >  }
> > >  
> > >  
> > > Index: tree-streamer-out.c
> > > ===================================================================
> > > --- tree-streamer-out.c	(revision 261841)
> > > +++ tree-streamer-out.c	(working copy)
> > > @@ -497,14 +494,10 @@ streamer_write_chain (struct output_bloc
> > >      {
> > >        /* We avoid outputting external vars or functions by reference
> > >  	 to the global decls section as we do not want to have them
> > > -	 enter decl merging.  This is, of course, only for the call
> > > -	 for streaming BLOCK_VARS, but other callers are safe.
> > > -	 See also lto-streamer-out.c:DFS_write_tree_body.  */
> > > -      if (VAR_OR_FUNCTION_DECL_P (t)
> > > -	  && DECL_EXTERNAL (t))
> > > -	stream_write_tree_shallow_non_ref (ob, t, ref_p);
> > > -      else
> > > -	stream_write_tree (ob, t, ref_p);
> > > +	 enter decl merging.  We should not need to do this anymore because
> > > +	 free_lang_data removes them from block scopes.  */
> > > +      gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
> > > +      stream_write_tree (ob, t, ref_p);
> > >  
> > >        t = TREE_CHAIN (t);
> > >      }
> > > @@ -620,11 +613,8 @@ write_ts_decl_common_tree_pointers (stru
> > >     pointer fields.  */
> > >  
> > >  static void
> > > -write_ts_decl_non_common_tree_pointers (struct output_block *ob, tree expr,
> > > -				        bool ref_p)
> > > +write_ts_decl_non_common_tree_pointers (struct output_block *, tree, bool)
> > >  {
> > > -  if (TREE_CODE (expr) == TYPE_DECL)
> > > -    stream_write_tree (ob, DECL_ORIGINAL_TYPE (expr), ref_p);
> > >  }
> > >  
> > >  
> > > Index: tree.c
> > > ===================================================================
> > > --- tree.c	(revision 261841)
> > > +++ tree.c	(working copy)
> > > @@ -5359,6 +5357,7 @@ free_lang_data_in_decl (tree decl)
> > >        DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
> > >        DECL_VISIBILITY_SPECIFIED (decl) = 0;
> > >        DECL_INITIAL (decl) = NULL_TREE;
> > > +      DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> > >      }
> > >    else if (TREE_CODE (decl) == FIELD_DECL)
> > >      DECL_INITIAL (decl) = NULL_TREE;
> > > @@ -5471,10 +5470,6 @@ find_decls_types_r (tree *tp, int *ws, v
> > >  	  fld_worklist_push (DECL_ARGUMENTS (t), fld);
> > >  	  fld_worklist_push (DECL_RESULT (t), fld);
> > >  	}
> > > -      else if (TREE_CODE (t) == TYPE_DECL)
> > > -	{
> > > -	  fld_worklist_push (DECL_ORIGINAL_TYPE (t), fld);
> > > -	}
> > >        else if (TREE_CODE (t) == FIELD_DECL)
> > >  	{
> > >  	  fld_worklist_push (DECL_FIELD_OFFSET (t), fld);
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Richard Biener June 21, 2018, 2:03 p.m. UTC | #5
On Thu, 21 Jun 2018, Jan Hubicka wrote:

> > > however aren't we supposed to not touch these at late builds? We drop most of TYPE_DECLs in favour
> > > of IDENTIFIER_TYPE and thus also throwing away DECL_ORIGINAL_TYPEs.
> > 
> > We keep quite a bit of TYPE_DECLs around for devirt.
> 
> I know, but we do not keep them systematicaly enough to make them useful for dwarf2out.
> So I would say dwarf2out touching them is a bug.
> 
> > 
> > We shouldn't (very often...) end up trying to emit type DIEs late.
> > Here we're running into
> > 
> >           /* If the prototype had an 'auto' or 'decltype(auto)' return 
> > type,
> >              emit the real type on the definition die.  */
> >           if (is_cxx () && debug_info_level > DINFO_LEVEL_TERSE)
> >             {
> >               dw_die_ref die = get_AT_ref (old_die, DW_AT_type);
> > 
> > but that's odd since get_AT_ref shoudn't be able to lookup DW_AT_type
> > in lto1.
> > 
> > So a testcase would be nice to have...
> 
> OK, I will try to debug into it.  Testcase is of course easy - apply patch and run
> lto bootstrap :)

:)

Sometimes running the testsuite with -flto -g also pops up such issues.

Richard.
Jan Hubicka June 21, 2018, 3:27 p.m. UTC | #6
Hi
with -flto -g -O2 -r -nostdlib -flinker-output=nolto-rel I get ICE on:
class a;
namespace b {
template <typename> struct c;
struct C {
  typedef a d;
};
void e();
}
template <typename f> class g : f {
public:
  template <typename i> g(i);
};
class a {
  long k;
};
namespace b {
template <> struct c<a> { template <typename j, typename h> static a l(j, h); };
}
template <typename j, typename h> b::C::d b::e(j m, h n) { c<a>::l(m, n); }
void o() {
  g<a> p = 0;
  a r(b::e(r, p));
}
Jan Hubicka June 21, 2018, 3:52 p.m. UTC | #7
Hi,
this problem here seems to be that is_cxx returns true and at the same
time auto_die and decltype_auto_die is not initialized. Here die==NULL
and we end up calling add_type_attribute for random reasons.

I am testing the following but I am not sure it is proper fix.  Are
we supposed to handle auto dies here somehow?

Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 261841)
+++ dwarf2out.c	(working copy)
@@ -22830,7 +22830,8 @@ gen_subprogram_die (tree decl, dw_die_re
 	  if (is_cxx () && debug_info_level > DINFO_LEVEL_TERSE)
 	    {
 	      dw_die_ref die = get_AT_ref (old_die, DW_AT_type);
-	      if (die == auto_die || die == decltype_auto_die)
+	      /* In LTO auto_die and decltype_auto_die may be NULL.  */
+	      if (die && (die == auto_die || die == decltype_auto_die))
 		add_type_attribute (subr_die, TREE_TYPE (TREE_TYPE (decl)),
 				    TYPE_UNQUALIFIED, false, context_die);
 	    }
Richard Biener June 22, 2018, 7:28 a.m. UTC | #8
On Thu, 21 Jun 2018, Jan Hubicka wrote:

> Hi,
> this problem here seems to be that is_cxx returns true and at the same
> time auto_die and decltype_auto_die is not initialized. Here die==NULL
> and we end up calling add_type_attribute for random reasons.
> 
> I am testing the following but I am not sure it is proper fix.  Are
> we supposed to handle auto dies here somehow?

We're not supposed to run into the DW_AT_specification case in late
dwarf, but yes, this explains the issue somewhat but the specification
is supposed to be generated early already and the late dwarf should
refer to it via DW_AT_abstract_origin.

I'll have to dig into the testcase a bit myself.

> Index: dwarf2out.c
> ===================================================================
> --- dwarf2out.c	(revision 261841)
> +++ dwarf2out.c	(working copy)
> @@ -22830,7 +22830,8 @@ gen_subprogram_die (tree decl, dw_die_re
>  	  if (is_cxx () && debug_info_level > DINFO_LEVEL_TERSE)
>  	    {
>  	      dw_die_ref die = get_AT_ref (old_die, DW_AT_type);
> -	      if (die == auto_die || die == decltype_auto_die)
> +	      /* In LTO auto_die and decltype_auto_die may be NULL.  */
> +	      if (die && (die == auto_die || die == decltype_auto_die))
>  		add_type_attribute (subr_die, TREE_TYPE (TREE_TYPE (decl)),
>  				    TYPE_UNQUALIFIED, false, context_die);
>  	    }
Richard Biener June 22, 2018, 7:39 a.m. UTC | #9
On Thu, 21 Jun 2018, Jan Hubicka wrote:

> Hi
> with -flto -g -O2 -r -nostdlib -flinker-output=nolto-rel I get ICE on:

I can't reproduce the ICE on TOT with your patch applied.

> ./xg++ t.ii -B. -g -flto -O2 -r -nostdlib -flinker-output=nolto-rel 
cc1plus: warning: command line option ‘-flinker-output=nolto-rel’ is valid 
for LTO but not for C++
t.ii: In function ‘b::C::d b::e(j, h)’:
t.ii:20:75: warning: no return statement in function returning non-void 
[-Wreturn-type]
 template <typename j, typename h> b::C::d b::e(j m, h n) { c<a>::l(m, n); 
}
                                                                           
^


> class a;
> namespace b {
> template <typename> struct c;
> struct C {
>   typedef a d;
> };
> void e();
> }
> template <typename f> class g : f {
> public:
>   template <typename i> g(i);
> };
> class a {
>   long k;
> };
> namespace b {
> template <> struct c<a> { template <typename j, typename h> static a l(j, h); };
> }
> template <typename j, typename h> b::C::d b::e(j m, h n) { c<a>::l(m, n); }
> void o() {
>   g<a> p = 0;
>   a r(b::e(r, p));
> }
diff mbox series

Patch

Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 261841)
+++ lto-streamer-out.c	(working copy)
@@ -819,12 +819,6 @@  DFS::DFS_write_tree_body (struct output_
 	DFS_follow_tree_edge (DECL_DEBUG_EXPR (expr));
     }
 
-  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
-    {
-      if (TREE_CODE (expr) == TYPE_DECL)
-	DFS_follow_tree_edge (DECL_ORIGINAL_TYPE (expr));
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
     {
       /* Make sure we don't inadvertently set the assembler name.  */
@@ -907,14 +901,13 @@  DFS::DFS_write_tree_body (struct output_
   if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
     {
       for (tree t = BLOCK_VARS (expr); t; t = TREE_CHAIN (t))
-	if (VAR_OR_FUNCTION_DECL_P (t)
-	    && DECL_EXTERNAL (t))
-	  /* We have to stream externals in the block chain as
-	     non-references.  See also
-	     tree-streamer-out.c:streamer_write_chain.  */
-	  DFS_write_tree (ob, expr_state, t, ref_p, false);
-	else
+	{
+	  /* We would have to stream externals in the block chain as
+	     non-references but we should have dropped them in
+	     free-lang-data.  */
+	  gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
 	  DFS_follow_tree_edge (t);
+	}
 
       DFS_follow_tree_edge (BLOCK_SUPERCONTEXT (expr));
 
@@ -1261,12 +1249,6 @@  hash_tree (struct streamer_tree_cache_d
          be able to call get_symbol_initial_value.  */
     }
 
-  if (CODE_CONTAINS_STRUCT (code, TS_DECL_NON_COMMON))
-    {
-      if (code == TYPE_DECL)
-	visit (DECL_ORIGINAL_TYPE (t));
-    }
-
   if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
     {
       if (DECL_ASSEMBLER_NAME_SET_P (t))
Index: tree-streamer-in.c
===================================================================
--- tree-streamer-in.c	(revision 261841)
+++ tree-streamer-in.c	(working copy)
@@ -728,11 +721,9 @@  lto_input_ts_decl_common_tree_pointers (
    file being read.  */
 
 static void
-lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *ib,
-					    struct data_in *data_in, tree expr)
+lto_input_ts_decl_non_common_tree_pointers (struct lto_input_block *,
+					    struct data_in *, tree)
 {
-  if (TREE_CODE (expr) == TYPE_DECL)
-    DECL_ORIGINAL_TYPE (expr) = stream_read_tree (ib, data_in);
 }
 
 
Index: tree-streamer-out.c
===================================================================
--- tree-streamer-out.c	(revision 261841)
+++ tree-streamer-out.c	(working copy)
@@ -497,14 +494,10 @@  streamer_write_chain (struct output_bloc
     {
       /* We avoid outputting external vars or functions by reference
 	 to the global decls section as we do not want to have them
-	 enter decl merging.  This is, of course, only for the call
-	 for streaming BLOCK_VARS, but other callers are safe.
-	 See also lto-streamer-out.c:DFS_write_tree_body.  */
-      if (VAR_OR_FUNCTION_DECL_P (t)
-	  && DECL_EXTERNAL (t))
-	stream_write_tree_shallow_non_ref (ob, t, ref_p);
-      else
-	stream_write_tree (ob, t, ref_p);
+	 enter decl merging.  We should not need to do this anymore because
+	 free_lang_data removes them from block scopes.  */
+      gcc_assert (!VAR_OR_FUNCTION_DECL_P (t) || !DECL_EXTERNAL (t));
+      stream_write_tree (ob, t, ref_p);
 
       t = TREE_CHAIN (t);
     }
@@ -620,11 +613,8 @@  write_ts_decl_common_tree_pointers (stru
    pointer fields.  */
 
 static void
-write_ts_decl_non_common_tree_pointers (struct output_block *ob, tree expr,
-				        bool ref_p)
+write_ts_decl_non_common_tree_pointers (struct output_block *, tree, bool)
 {
-  if (TREE_CODE (expr) == TYPE_DECL)
-    stream_write_tree (ob, DECL_ORIGINAL_TYPE (expr), ref_p);
 }
 
 
Index: tree.c
===================================================================
--- tree.c	(revision 261841)
+++ tree.c	(working copy)
@@ -5359,6 +5357,7 @@  free_lang_data_in_decl (tree decl)
       DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
       DECL_VISIBILITY_SPECIFIED (decl) = 0;
       DECL_INITIAL (decl) = NULL_TREE;
+      DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
     }
   else if (TREE_CODE (decl) == FIELD_DECL)
     DECL_INITIAL (decl) = NULL_TREE;
@@ -5471,10 +5470,6 @@  find_decls_types_r (tree *tp, int *ws, v
 	  fld_worklist_push (DECL_ARGUMENTS (t), fld);
 	  fld_worklist_push (DECL_RESULT (t), fld);
 	}
-      else if (TREE_CODE (t) == TYPE_DECL)
-	{
-	  fld_worklist_push (DECL_ORIGINAL_TYPE (t), fld);
-	}
       else if (TREE_CODE (t) == FIELD_DECL)
 	{
 	  fld_worklist_push (DECL_FIELD_OFFSET (t), fld);