Message ID | 20160119115352.GE5273@kam.mff.cuni.cz |
---|---|
State | New |
Headers | show |
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) > ; > >
> 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
> > 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
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
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.
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) ;