From patchwork Thu Jun 17 18:18:15 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Fix DECL_ALIGN during expand with dynamic stack realignment (PR
 target/44542, take 2)
From: Jakub Jelinek <jakub@redhat.com>
X-Patchwork-Id: 56076
Message-Id: <20100617181815.GA7811@tyan-ft48-01.lab.bos.redhat.com>
To: Michael Matz <matz@suse.de>
Cc: gcc-patches@gcc.gnu.org
Date: Thu, 17 Jun 2010 20:18:15 +0200

On Thu, Jun 17, 2010 at 04:52:38PM +0200, Michael Matz wrote:
> > +      else if (DECL_HAS_VALUE_EXPR_P (var)
> > +	       || (DECL_RTL_SET_P (var)
> > +		   && MEM_P (DECL_RTL (var))
> > +		   && (XEXP (DECL_RTL (var), 0) == virtual_stack_vars_rtx
> > +		       || (GET_CODE (XEXP (DECL_RTL (var), 0)) == PLUS
> > +			   && XEXP (XEXP (DECL_RTL (var), 0), 0)
> > +			      == virtual_stack_vars_rtx
> > +			   && CONST_INT_P (XEXP (XEXP (DECL_RTL (var), 0), 1))))))
> 
> ... this reads uneasy to me (that's the reason I bothered with the 
> really_expand suggestion).  Why do you have to test the form of the MEM, 
> shouldn't "DECL_RTL_SET_P (var) && MEM_P (DECL_RTL (var))" be enough, or 
> even without testing for MEM_P?

During testing with assert of this I found that Ada passes in through
some VAR_DECLs with (mem (sp)), but those shouldn't matter for dynamic stack
realignment purposes.

So here is an updated patch (also with the align == 0 -> max_align stuff
H.J. mentioned), bootstrapped/regtested on x86_64-linux and i686-linux.

Ok for trunk?

2010-06-17  Jakub Jelinek  <jakub@redhat.com>

	PR target/44542
	* cfgexpand.c (expand_one_stack_var_at): Limit align to maximum
	of max_used_stack_slot_alignment and PREFERRED_STACK_BOUNDARY
	instead of MAX_SUPPORTED_STACK_ALIGNMENT.
	(expand_one_var): Don't consider DECL_ALIGN for variables for
	which expand_one_stack_var_at has been already called.



	Jakub


--- gcc/cfgexpand.c.jj	2010-06-17 17:01:11.964198458 +0200
+++ gcc/cfgexpand.c	2010-06-17 18:25:18.940335757 +0200
@@ -706,7 +706,7 @@ static void
 expand_one_stack_var_at (tree decl, HOST_WIDE_INT offset)
 {
   /* Alignment is unsigned.   */
-  unsigned HOST_WIDE_INT align;
+  unsigned HOST_WIDE_INT align, max_align;
   rtx x;
 
   /* If this fails, we've overflowed the stack frame.  Error nicely?  */
@@ -723,10 +723,10 @@ expand_one_stack_var_at (tree decl, HOST
       offset -= frame_phase;
       align = offset & -offset;
       align *= BITS_PER_UNIT;
-      if (align == 0)
-	align = STACK_BOUNDARY;
-      else if (align > MAX_SUPPORTED_STACK_ALIGNMENT)
-	align = MAX_SUPPORTED_STACK_ALIGNMENT;
+      max_align = MAX (crtl->max_used_stack_slot_alignment,
+		       PREFERRED_STACK_BOUNDARY);
+      if (align == 0 || align > max_align)
+	align = max_align;
 
       DECL_ALIGN (decl) = align;
       DECL_USER_ALIGN (decl) = 0;
@@ -931,6 +931,13 @@ expand_one_var (tree var, bool toplevel,
 	align = MINIMUM_ALIGNMENT (TREE_TYPE (var),
 				   TYPE_MODE (TREE_TYPE (var)),
 				   TYPE_ALIGN (TREE_TYPE (var)));
+      else if (DECL_HAS_VALUE_EXPR_P (var)
+	       || (DECL_RTL_SET_P (var) && MEM_P (DECL_RTL (var))))
+	/* Don't consider debug only variables with DECL_HAS_VALUE_EXPR_P set
+	   or variables which were assigned a stack slot already by
+	   expand_one_stack_var_at - in the latter case DECL_ALIGN has been
+	   changed from the offset chosen to it.  */
+	align = crtl->stack_alignment_estimated;
       else
 	align = MINIMUM_ALIGNMENT (var, DECL_MODE (var), DECL_ALIGN (var));