Re: middle-end/9997: Coelesce stack slots for disjoint scopes.

[Daniel Jacobowitz <drow@mvista.com>]

The following reply was made to PR middle-end/9997; it has been noted by GNATS.

From: Daniel Jacobowitz <drow@mvista.com>
To: rth@redhat.com, gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org
Cc:  
Subject: Re: middle-end/9997: Coelesce stack slots for disjoint scopes.
Date: Sat, 8 Mar 2003 15:22:00 -0500

 On Sat, Mar 08, 2003 at 12:54:06PM -0500, Daniel Jacobowitz wrote:
 > On Sat, Mar 08, 2003 at 12:56:24AM -0000, rth@redhat.com wrote:
 > > 
 > > >Number:         9997
 > > >Category:       middle-end
 > > >Synopsis:       Coelesce stack slots for disjoint scopes.
 > > >Confidential:   no
 > > >Severity:       non-critical
 > > >Priority:       medium
 > > >Responsible:    unassigned
 > > >State:          open
 > > >Class:          change-request
 > > >Submitter-Id:   net
 > > >Arrival-Date:   Sat Mar 08 01:06:00 UTC 2003
 > > >Closed-Date:
 > > >Last-Modified:
 > > >Originator:     Richard Henderson
 > > >Release:        all
 > > >Organization:
 > > >Environment:
 > > any
 > > >Description:
 > > Given
 > > 
 > > struct big { char x[1024]; };
 > > extern void bar(struct big *);
 > > 
 > > void foo(void)
 > > {
 > >   {
 > >     struct big one;
 > >     bar(&one);
 > >   }
 > >   {
 > >     struct big one;
 > >     bar(&one);
 > >   }
 > > }
 > > 
 > > we should allocate only one structure on the stack.
 > 
 > Here's some interesting numbers I got for this testcase.  HEAD+stack is
 > the -fstack-reorg patch posted some time ago.
 > 
 > Compiler	Flags			Stack size
 > 
 > gcc 3.2.2	-O0			2068
 > gcc 3.2.2	-O2			2068
 > gcc HEAD+stack	-O0			2072
 > gcc HEAD+stack	-O2			2072
 > gcc HEAD+stack	-O0 -fstack-reorg	2072
 > gcc HEAD+stack	-O2 -fstack-reorg	2072
 > 
 > Here's a patch I've been playing with:
 > gcc HEAD+patch	-O0			1048
 > gcc HEAD+patch	-O2			1044
 > 
 > The patch bootstrapped on i686-pc-linux-gnu, and I'm running make check
 > now.  I think it's correct; basically, when we close the SCOPE_EXPR,
 > mark all stack slots associated with locals as dead.
 
 It's not quite right, since it re-introduces the failure at
 gcc.c-torture/execute/20020320-1.c.  It probably just needs to be
 tweaked for statement expressions though.
 
 > 
 > -- 
 > Daniel Jacobowitz
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2003-03-08  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* c-semantics.c (genrtl_scope_stmt): Call free_named_temp_slots.
 > 	* expr.c (expand_expr): Update calls to assign_stack_temp_for_type.
 > 	* function.c (struct temp_slot): Add DECL.
 > 	(assign_stack_temp_for_type): Add DECL argument; save it in the
 > 	temp_slot.
 > 	(assign_stack_temp): Update call to assign_stack_temp_for_type.
 > 	(assign_temp): Pass the DECL to assign_stack_temp_for_type.
 > 	(free_named_stack_slots): New function.
 > 	* rtl.h (assign_stack_temp_for_type): Update prototype.
 > 	* tree.h (assign_stack_temp_for_type): Add prototype.
 > 
 > Index: c-semantics.c
 > ===================================================================
 > RCS file: /cvs/gcc/gcc/gcc/c-semantics.c,v
 > retrieving revision 1.53
 > diff -u -p -r1.53 c-semantics.c
 > --- c-semantics.c	22 Feb 2003 00:32:02 -0000	1.53
 > +++ c-semantics.c	8 Mar 2003 17:48:53 -0000
 > @@ -622,7 +622,11 @@ genrtl_scope_stmt (t)
 >        if (SCOPE_BEGIN_P (t))
 >  	expand_start_bindings_and_block (2 * SCOPE_NULLIFIED_P (t), block);
 >        else if (SCOPE_END_P (t))
 > -	expand_end_bindings (NULL_TREE, !SCOPE_NULLIFIED_P (t), 0);
 > +	{
 > +	  expand_end_bindings (NULL_TREE, !SCOPE_NULLIFIED_P (t), 0);
 > +	  if (block)
 > +	    free_named_temp_slots (block);
 > +	}
 >      }
 >    else if (!SCOPE_NULLIFIED_P (t))
 >      {
 > Index: expr.c
 > ===================================================================
 > RCS file: /cvs/gcc/gcc/gcc/expr.c,v
 > retrieving revision 1.510
 > diff -u -p -r1.510 expr.c
 > --- expr.c	5 Mar 2003 00:12:40 -0000	1.510
 > +++ expr.c	8 Mar 2003 17:48:55 -0000
 > @@ -7863,7 +7863,8 @@ expand_expr (exp, target, tmode, modifie
 >  	    target
 >  	      = assign_stack_temp_for_type
 >  		(TYPE_MODE (inner_type),
 > -		 GET_MODE_SIZE (TYPE_MODE (inner_type)), 0, inner_type);
 > +		 GET_MODE_SIZE (TYPE_MODE (inner_type)), 0, inner_type,
 > +		 NULL_TREE);
 >  
 >  	  emit_move_insn (target, op0);
 >  	  op0 = target;
 > @@ -7887,7 +7888,8 @@ expand_expr (exp, target, tmode, modifie
 >  		= MAX (int_size_in_bytes (inner_type),
 >  		       (HOST_WIDE_INT) GET_MODE_SIZE (TYPE_MODE (type)));
 >  	      rtx new = assign_stack_temp_for_type (TYPE_MODE (type),
 > -						    temp_size, 0, type);
 > +						    temp_size, 0, type,
 > +						    NULL_TREE);
 >  	      rtx new_with_op0_mode = adjust_address (new, GET_MODE (op0), 0);
 >  
 >  	      if (TREE_ADDRESSABLE (exp))
 > @@ -9189,7 +9191,8 @@ expand_expr (exp, target, tmode, modifie
 >  		   : int_size_in_bytes (inner_type),
 >  		   1, build_qualified_type (inner_type,
 >  					    (TYPE_QUALS (inner_type)
 > -					     | TYPE_QUAL_CONST)));
 > +					     | TYPE_QUAL_CONST)),
 > +		   NULL_TREE);
 >  
 >  	      if (TYPE_ALIGN_OK (inner_type))
 >  		abort ();
 > Index: function.c
 > ===================================================================
 > RCS file: /cvs/gcc/gcc/gcc/function.c,v
 > retrieving revision 1.405
 > diff -u -p -r1.405 function.c
 > --- function.c	4 Mar 2003 06:20:12 -0000	1.405
 > +++ function.c	8 Mar 2003 17:48:57 -0000
 > @@ -165,7 +165,8 @@ static GTY(()) varray_type sibcall_epilo
 >  
 >     Automatic variables are also assigned temporary slots, at the nesting
 >     level where they are defined.  They are marked a "kept" so that
 > -   free_temp_slots will not free them.  */
 > +   free_temp_slots will not free them, although a decl is associated
 > +   with each so that it can be freed when the decl goes out of scope.  */
 >  
 >  struct temp_slot GTY(())
 >  {
 > @@ -201,6 +202,9 @@ struct temp_slot GTY(())
 >    /* The size of the slot, including extra space for alignment.  This
 >       info is for combine_temp_slots.  */
 >    HOST_WIDE_INT full_size;
 > +  /* The decl associated with this stack slot, for automatic variables.  This
 > +     info is for free_named_temp_slots.  */
 > +  tree decl;
 >  };
 >  \f
 >  /* This structure is used to record MEMs or pseudos used to replace VAR, any
 > @@ -649,14 +653,17 @@ assign_stack_local (mode, size, align)
 >     if we are to allocate something at an inner level to be treated as
 >     a variable in the block (e.g., a SAVE_EXPR).
 >  
 > -   TYPE is the type that will be used for the stack slot.  */
 > +   TYPE is the type that will be used for the stack slot.
 > +
 > +   DECL is the decl that will be associated with the stack slot.  */
 >  
 >  rtx
 > -assign_stack_temp_for_type (mode, size, keep, type)
 > +assign_stack_temp_for_type (mode, size, keep, type, decl)
 >       enum machine_mode mode;
 >       HOST_WIDE_INT size;
 >       int keep;
 >       tree type;
 > +     tree decl;
 >  {
 >    unsigned int align;
 >    struct temp_slot *p, *best_p = 0;
 > @@ -789,6 +796,7 @@ assign_stack_temp_for_type (mode, size, 
 >    p->addr_taken = 0;
 >    p->rtl_expr = seq_rtl_expr;
 >    p->type = type;
 > +  p->decl = decl;
 >  
 >    if (keep == 2)
 >      {
 > @@ -838,7 +846,7 @@ assign_stack_temp (mode, size, keep)
 >       HOST_WIDE_INT size;
 >       int keep;
 >  {
 > -  return assign_stack_temp_for_type (mode, size, keep, NULL_TREE);
 > +  return assign_stack_temp_for_type (mode, size, keep, NULL_TREE, NULL_TREE);
 >  }
 >  \f
 >  /* Assign a temporary.
 > @@ -904,7 +912,7 @@ assign_temp (type_or_decl, keep, memory_
 >  	  size = 1;
 >  	}
 >  
 > -      tmp = assign_stack_temp_for_type (mode, size, keep, type);
 > +      tmp = assign_stack_temp_for_type (mode, size, keep, type, decl);
 >        return tmp;
 >      }
 >  
 > @@ -1177,6 +1185,23 @@ preserve_temp_slots (x)
 >    for (p = temp_slots; p; p = p->next)
 >      if (p->in_use && p->level == temp_slot_level && ! p->keep)
 >        p->level--;
 > +}
 > +
 > +/* For every VAR_DECL in BLOCK, mark the associated temporary slot as dead
 > +   so that it can be reused.  */
 > +
 > +void
 > +free_named_temp_slots (block)
 > +     tree block;
 > +{
 > +  struct temp_slot *p;
 > +  tree decl;
 > +
 > +  for (decl = BLOCK_VARS (block); decl; decl = TREE_CHAIN (decl))
 > +    if (TREE_CODE (decl) == VAR_DECL)
 > +      for (p = temp_slots; p; p = p->next)
 > +	if (p->decl == decl)
 > +	    p->in_use = 0;
 >  }
 >  
 >  /* X is the result of an RTL_EXPR.  If it is a temporary slot associated
 > Index: rtl.h
 > ===================================================================
 > RCS file: /cvs/gcc/gcc/gcc/rtl.h,v
 > retrieving revision 1.386
 > diff -u -p -r1.386 rtl.h
 > --- rtl.h	28 Feb 2003 07:06:33 -0000	1.386
 > +++ rtl.h	8 Mar 2003 17:48:57 -0000
 > @@ -1459,7 +1459,8 @@ extern rtx assign_stack_local		PARAMS ((
 >  extern rtx assign_stack_temp		PARAMS ((enum machine_mode,
 >  					       HOST_WIDE_INT, int));
 >  extern rtx assign_stack_temp_for_type	PARAMS ((enum machine_mode,
 > -						 HOST_WIDE_INT, int, tree));
 > +						 HOST_WIDE_INT, int, tree,
 > +						 tree));
 >  extern rtx assign_temp			PARAMS ((tree, int, int, int));
 >  /* In emit-rtl.c */
 >  extern rtx emit_insn_before		PARAMS ((rtx, rtx));
 > Index: tree.h
 > ===================================================================
 > RCS file: /cvs/gcc/gcc/gcc/tree.h,v
 > retrieving revision 1.385
 > diff -u -p -r1.385 tree.h
 > --- tree.h	2 Mar 2003 21:18:15 -0000	1.385
 > +++ tree.h	8 Mar 2003 17:49:00 -0000
 > @@ -2999,6 +2999,7 @@ extern void pop_temp_slots		PARAMS ((voi
 >  extern void push_temp_slots		PARAMS ((void));
 >  extern void preserve_temp_slots		PARAMS ((rtx));
 >  extern void preserve_rtl_expr_temps	PARAMS ((tree));
 > +extern void free_named_temp_slots	PARAMS ((tree));
 >  extern int aggregate_value_p		PARAMS ((tree));
 >  extern void free_temps_for_rtl_expr	PARAMS ((tree));
 >  extern void instantiate_virtual_regs	PARAMS ((tree, rtx));
 
 -- 
 Daniel Jacobowitz
 MontaVista Software                         Debian GNU/Linux Developer