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

[Daniel Jacobowitz <drow at mvista dot com>]

On Sat, Mar 08, 2003 at 12:54:06PM -0500, Daniel Jacobowitz wrote:
> On Sat, Mar 08, 2003 at 12:56:24AM -0000, rth at redhat dot 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 at mvista dot 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;
>  };
>  
>  /* 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);
>  }
>  
>  /* 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