Re: Copy debug info properly in the inliner

[Jan Hubicka <jh at suse dot cz>]

> On Fri, Jun 24, 2005 at 03:19:56PM +0200, Jan Hubicka wrote:
> >     fn = id->caller;
> > - #if 1
> >     /* FIXME!  It shouldn't be so hard to manage blocks.  Rebuilding them in
> >        rest_of_compilation is a good start.  */
> 
> FIXME is either not accurate or you havn't done your job.

Hmm, I wasn't particularly fure what this comment referred to, but
definitly I can kill it.
> 
> > + /* Copy the whole locks tree and root it in id->block.  */
> 
> s/ locks/block/
> 
> > !       /* If EXPR has block defined, map it to newly constructed block.
> > ! 	 (if set previously by remap_block, when saving blocks are not remapped
> > ! 	 at all)
> > !          When inlining we want EXPRs without block appear in the block
> > ! 	 of function call.  */
> > !       if (IS_EXPR_CODE_CLASS (TREE_CODE_CLASS (TREE_CODE (*tp))))
> > ! 	{
> > ! 	  splay_tree_node n;
> > ! 
> > ! 	  n = splay_tree_lookup (id->decl_map,
> > ! 	      			 (splay_tree_key) TREE_BLOCK (*tp));
> > ! 	  if (n)
> > ! 	    TREE_BLOCK (*tp) = (tree)n->value;
> > ! 	  else if (id->block)
> > ! 	    TREE_BLOCK (*tp) = id->block;
> > ! 	}
> 
> If the lookup fails, we're putting the stmt into the wrong block.
> Surely it should never fail?  I'm thinking

well, if the original TREE_BLOCK is non-zero, it will never fail as I
copy all of the blocks we have in the calee beforehand. 
> 
> 	tree new_block = id->block;
> 	if (TREE_BLOCK (*tp))
> 	  {
> 	    splay_tree_node n;
> 	    n = splay_tree_lookup (id->decl_map,
> 				   (splay_tree_key) TREE_BLOCK (*tp));
> 	    gcc_assert (n != NULL);
> 	    new_block = (tree) n->value;
> 	  }
> 	TREE_BLOCK (*tp) = new_block;

this should work identically and perhaps is easier to follow, so I will
replace it.
> 
> > +   /* Add local static vars in this inlined callee to caller.  */
> > +   t_step = id->callee_cfun->unexpanded_var_list;
> > +   if (id->callee_cfun->saved_unexpanded_var_list)
> > +     t_step = id->callee_cfun->saved_unexpanded_var_list;
> > +   for (; t_step; t_step = TREE_CHAIN (t_step))
> > +     {
> > +       var = TREE_VALUE (t_step);
> > +       if (TREE_STATIC (var) && !TREE_ASM_WRITTEN (var))
> > + 	cfun->unexpanded_var_list = tree_cons (NULL_TREE, var,
> > + 					       cfun->unexpanded_var_list);
> > +       else if (TREE_CODE (var) == VAR_DECL || TREE_CODE (var) == PARM_DECL)
> > + 	cfun->unexpanded_var_list = tree_cons (NULL_TREE, remap_decl (var, id),
> > + 					       cfun->unexpanded_var_list);
> > +     }
> 
> Comment is no longer accurate, because it's more than local statics.
> 
> > +   /* Add local static vars in this inlined callee to caller.  */
> > +   for (t_step = id.callee_cfun->unexpanded_var_list;
> > +        t_step;
> > +        t_step = TREE_CHAIN (t_step))
> > +     {
> > +       tree var = TREE_VALUE (t_step);
> > +       if (TREE_STATIC (var) && !TREE_ASM_WRITTEN (var))
> > + 	cfun->saved_unexpanded_var_list = tree_cons (NULL_TREE, var,
> > + 					             cfun->saved_unexpanded_var_list);
> > +       else if (TREE_CODE (var) == VAR_DECL || TREE_CODE (var) == PARM_DECL)
> > + 	cfun->saved_unexpanded_var_list = tree_cons (NULL_TREE, remap_decl (var, &id),
> > + 					             cfun->saved_unexpanded_var_list);
> > +     }
> 
> Comment is no longer accurate because it has nothing to do with
> a callee.  Surely here you should be copying the entire list
> without further checks.

Probably yes.  For some reasons unexpanded_var_list contains CONST_DECL
and TYPE_DECLs that seems unused later and throwing them into remap_decl
seemed a bad idea.
Probably proper fix is avoid these being placed into unexpanded_var_list
at first place.
I will send updated patch...

Honzaz
> 
> 
> 
> r~