diff mbox

Prune BLOCK_VARs lists in free_lang_data

Message ID 20160119115352.GE5273@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Jan. 19, 2016, 11:53 a.m. UTC
Hi,
here is updated patch. It has same effect as the former version.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* tree-ssa-live.c (remove_unused_scope_block_p): Also remove
	reudndant typedefs.

Comments

Richard Biener Jan. 19, 2016, 12:25 p.m. UTC | #1
On Tue, 19 Jan 2016, Jan Hubicka wrote:

> Hi,
> here is updated patch. It has same effect as the former version.
> 
> Bootstrapped/regtested x86_64-linux, OK?

But what about the comment?

         We track no
         information on whether given type is used or not, so we have
         to keep them even when not emitting debug information,
         otherwise we may end up remapping variables and their (local)
         types in different orders depending on whether debug
         information is being generated.  */

which suggests that the TYPE_DECLs somehow "order" remapping
of local types and that is somehow important (maybe for VLA
types which refer to locals). OTOH local vars are also
duplicated in order before copying stmts (which may introduce
differences because of seeing debug stmts or not refering to
decls/types).

Richard.

> Honza
> 
> 	* tree-ssa-live.c (remove_unused_scope_block_p): Also remove
> 	reudndant typedefs.
> Index: tree-ssa-live.c
> ===================================================================
> --- tree-ssa-live.c	(revision 232466)
> +++ tree-ssa-live.c	(working copy)
> @@ -470,7 +470,8 @@ remove_unused_scope_block_p (tree scope,
>  	 types in different orders depending on whether debug
>  	 information is being generated.  */
>  
> -      else if (TREE_CODE (*t) == TYPE_DECL
> +      else if ((TREE_CODE (*t) == TYPE_DECL
> +		&& !DECL_IGNORED_P (*t) && !is_redundant_typedef (*t))
>  	       || debug_info_level == DINFO_LEVEL_NORMAL
>  	       || debug_info_level == DINFO_LEVEL_VERBOSE)
>  	;
> 
>
Jan Hubicka Jan. 19, 2016, 12:39 p.m. UTC | #2
> On Tue, 19 Jan 2016, Jan Hubicka wrote:
> 
> > Hi,
> > here is updated patch. It has same effect as the former version.
> > 
> > Bootstrapped/regtested x86_64-linux, OK?
> 
> But what about the comment?
> 
>          We track no
>          information on whether given type is used or not, so we have
>          to keep them even when not emitting debug information,
>          otherwise we may end up remapping variables and their (local)
>          types in different orders depending on whether debug
>          information is being generated.  */
> 
> which suggests that the TYPE_DECLs somehow "order" remapping
> of local types and that is somehow important (maybe for VLA
> types which refer to locals). OTOH local vars are also
> duplicated in order before copying stmts (which may introduce
> differences because of seeing debug stmts or not refering to
> decls/types).

The original patch is here:
https://gcc.gnu.org/ml/gcc-patches/2011-01/msg01344.html
my understand is that it is all about DECL_UID being stable with -g0
and -g. My patch does not change that becuase I drop ignored and redundant
typedefs even with -g.

Honza
Jan Hubicka Jan. 19, 2016, 12:42 p.m. UTC | #3
> > On Tue, 19 Jan 2016, Jan Hubicka wrote:
> > 
> > > Hi,
> > > here is updated patch. It has same effect as the former version.
> > > 
> > > Bootstrapped/regtested x86_64-linux, OK?
> > 
> > But what about the comment?
> > 
> >          We track no
> >          information on whether given type is used or not, so we have
> >          to keep them even when not emitting debug information,
> >          otherwise we may end up remapping variables and their (local)
> >          types in different orders depending on whether debug
> >          information is being generated.  */
> > 
> > which suggests that the TYPE_DECLs somehow "order" remapping
> > of local types and that is somehow important (maybe for VLA
> > types which refer to locals). OTOH local vars are also
> > duplicated in order before copying stmts (which may introduce
> > differences because of seeing debug stmts or not refering to
> > decls/types).
> 
> The original patch is here:
> https://gcc.gnu.org/ml/gcc-patches/2011-01/msg01344.html
> my understand is that it is all about DECL_UID being stable with -g0
> and -g. My patch does not change that becuase I drop ignored and redundant
> typedefs even with -g.

Alexandre, aslo your original mail mentions:
> Anyhow, it seems to me that dropping local type decls from lexical
> scopes doesn't buy us much, and even though it is indeed a sledgehammer,
> as richi put it, this fixes the problem, and I can't envision other
> simpler solutions that wouldn't risk running into the problems mentioned
> above, so...

> Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu.  Ok to install?


> One solution I do envision, which might help, would be to try to figure
> out which types are unused, and discard those.  Say, scan all variables
> within a lexical scope (including nested blocks), deciding which ones
> can be discarded, and then, as we move out of the nesting, we can
> decide, from last to first, which types are unused, and mark as used
> types refereced from retained variables and other types that are used,
> removing those that, when reached during this backward scan, remain
> marked as unused.  Or something along these lines, taking nested
> functions into account, if needed, and anything else I may have missed
> ;-)

dropping the type_decls definitly buys us a memory with LTO and firefox.  How
hard would be to implement the prunning of dead TYPE_DECLs as you suggest?

Honza
> 
> Honza
Jakub Jelinek Jan. 19, 2016, 12:52 p.m. UTC | #4
On Tue, Jan 19, 2016 at 01:42:00PM +0100, Jan Hubicka wrote:
> > One solution I do envision, which might help, would be to try to figure
> > out which types are unused, and discard those.  Say, scan all variables
> > within a lexical scope (including nested blocks), deciding which ones
> > can be discarded, and then, as we move out of the nesting, we can
> > decide, from last to first, which types are unused, and mark as used
> > types refereced from retained variables and other types that are used,
> > removing those that, when reached during this backward scan, remain
> > marked as unused.  Or something along these lines, taking nested
> > functions into account, if needed, and anything else I may have missed
> > ;-)
> 
> dropping the type_decls definitly buys us a memory with LTO and firefox.  How
> hard would be to implement the prunning of dead TYPE_DECLs as you suggest?

The problem might be if some types are only referenced from unused variables
for which we emit just debug stmts but no other references to them.
Or would we not prune in that case (i.e. only prune types if no variables
before unused locals prunning refer to those types)?

	Jakub
Richard Biener Jan. 19, 2016, 12:57 p.m. UTC | #5
On Tue, 19 Jan 2016, Jan Hubicka wrote:

> > On Tue, 19 Jan 2016, Jan Hubicka wrote:
> > 
> > > Hi,
> > > here is updated patch. It has same effect as the former version.
> > > 
> > > Bootstrapped/regtested x86_64-linux, OK?
> > 
> > But what about the comment?
> > 
> >          We track no
> >          information on whether given type is used or not, so we have
> >          to keep them even when not emitting debug information,
> >          otherwise we may end up remapping variables and their (local)
> >          types in different orders depending on whether debug
> >          information is being generated.  */
> > 
> > which suggests that the TYPE_DECLs somehow "order" remapping
> > of local types and that is somehow important (maybe for VLA
> > types which refer to locals). OTOH local vars are also
> > duplicated in order before copying stmts (which may introduce
> > differences because of seeing debug stmts or not refering to
> > decls/types).
> 
> The original patch is here:
> https://gcc.gnu.org/ml/gcc-patches/2011-01/msg01344.html
> my understand is that it is all about DECL_UID being stable with -g0
> and -g. My patch does not change that becuase I drop ignored and redundant
> typedefs even with -g.

Yes, I see that but the comment suggests there are 2nd-order effects
because those TYPE_DECLs you remove serve as "ordering" point for
refered types and decls during inlining (as we remap blocks and
their decls and refered-to vars first).

The thing that would make the order -g dependent would be remapping
a type or decl when remapping a debug stmt.  Not sure if anything
else is required to refer to them "first" (local vars for example).

Richard.
diff mbox

Patch

Index: tree-ssa-live.c
===================================================================
--- tree-ssa-live.c	(revision 232466)
+++ tree-ssa-live.c	(working copy)
@@ -470,7 +470,8 @@  remove_unused_scope_block_p (tree scope,
 	 types in different orders depending on whether debug
 	 information is being generated.  */
 
-      else if (TREE_CODE (*t) == TYPE_DECL
+      else if ((TREE_CODE (*t) == TYPE_DECL
+		&& !DECL_IGNORED_P (*t) && !is_redundant_typedef (*t))
 	       || debug_info_level == DINFO_LEVEL_NORMAL
 	       || debug_info_level == DINFO_LEVEL_VERBOSE)
 	;