Re: [PATCH] Improve alloca alignment

[Eric Botcazou <ebotcazou at adacore dot com>]

> No clue, but ISTM that it should.  Eric, can you try that and see if it
> addresses these problems?  I'd really like to get this wrapped up, but I
> don't have access to any sparc systems to test it myself.

Yes, the INIT_EXPANDERS trick works for SPARC (but this has nothing to do with 
SPARC_STACK_BIAS) and avoid hardcoding the bogus alignment assumption in the 
get_dynamic_stack_size function.  As a matter of fact, this was the approach 
originally used by Dominik Vogt last year.

Of course this doesn't address the same potential issue on other targets but 
you don't seem to care much about that, so who am I to do it after all? ;-)

Tested on x86_64-suse-linux and SPARC/Solaris, applied on the mainline.


2017-12-13  Eric Botcazou  <ebotcazou@adacore.com>
            Dominik Vogt  <vogt@linux.vnet.ibm.com>

	PR middle-end/78468
	* emit-rtl.c (init_emit): Remove ??? comment.
	* explow.c (get_dynamic_stack_size): Take known alignment of stack
	pointer + STACK_DYNAMIC_OFFSET into account in lieu of STACK_BOUNDARY
	* config/sparc/sparc.h (INIT_EXPANDERS): In 32-bit mode, lower the
	alignment of 3 virtual registers to BITS_PER_WORD.

	* config/sparc/sparc.c (sparc_compute_frame_size): Simplify.

-- 
Eric Botcazou

Index: emit-rtl.c
===================================================================
--- emit-rtl.c	(revision 255578)
+++ emit-rtl.c	(working copy)
@@ -5764,8 +5764,6 @@ init_emit (void)
   REGNO_POINTER_ALIGN (HARD_FRAME_POINTER_REGNUM) = STACK_BOUNDARY;
   REGNO_POINTER_ALIGN (ARG_POINTER_REGNUM) = STACK_BOUNDARY;
 
-  /* ??? These are problematic (for example, 3 out of 4 are wrong on
-     32-bit SPARC and cannot be all fixed because of the ABI).  */
   REGNO_POINTER_ALIGN (VIRTUAL_INCOMING_ARGS_REGNUM) = STACK_BOUNDARY;
   REGNO_POINTER_ALIGN (VIRTUAL_STACK_VARS_REGNUM) = STACK_BOUNDARY;
   REGNO_POINTER_ALIGN (VIRTUAL_STACK_DYNAMIC_REGNUM) = STACK_BOUNDARY;
Index: explow.c
===================================================================
--- explow.c	(revision 255578)
+++ explow.c	(working copy)
@@ -1206,7 +1206,6 @@ get_dynamic_stack_size (rtx *psize, unsi
 			unsigned required_align,
 			HOST_WIDE_INT *pstack_usage_size)
 {
-  unsigned extra = 0;
   rtx size = *psize;
 
   /* Ensure the size is in the proper mode.  */
@@ -1242,16 +1241,16 @@ get_dynamic_stack_size (rtx *psize, unsi
      example), so we must preventively align the value.  We leave space
      in SIZE for the hole that might result from the alignment operation.  */
 
-  /* Since the stack is presumed to be aligned before this allocation,
-     we only need to increase the size of the allocation if the required
-     alignment is more than the stack alignment.  */
-  if (required_align > STACK_BOUNDARY)
+  unsigned known_align = REGNO_POINTER_ALIGN (VIRTUAL_STACK_DYNAMIC_REGNUM);
+  if (known_align == 0)
+    known_align = BITS_PER_UNIT;
+  if (required_align > known_align)
     {
-      extra = (required_align - STACK_BOUNDARY) / BITS_PER_UNIT;
+      unsigned extra = (required_align - known_align) / BITS_PER_UNIT;
       size = plus_constant (Pmode, size, extra);
       size = force_operand (size, NULL_RTX);
-      if (size_align > STACK_BOUNDARY)
-	size_align = STACK_BOUNDARY;
+      if (size_align > known_align)
+	size_align = known_align;
 
       if (flag_stack_usage_info && pstack_usage_size)
 	*pstack_usage_size += extra;
Index: config/sparc/sparc.c
===================================================================
--- config/sparc/sparc.c	(revision 255578)
+++ config/sparc/sparc.c	(working copy)
@@ -5483,10 +5483,8 @@ sparc_compute_frame_size (HOST_WIDE_INT
     frame_size = apparent_frame_size = 0;
   else
     {
-      /* We subtract TARGET_STARTING_FRAME_OFFSET, remember it's negative.  */
-      apparent_frame_size
-	= ROUND_UP (size - targetm.starting_frame_offset (), 8);
-      apparent_frame_size += n_global_fp_regs * 4;
+      /* Start from the apparent frame size.  */
+      apparent_frame_size = ROUND_UP (size, 8) + n_global_fp_regs * 4;
 
       /* We need to add the size of the outgoing argument area.  */
       frame_size = apparent_frame_size + ROUND_UP (args_size, 8);
Index: config/sparc/sparc.h
===================================================================
--- config/sparc/sparc.h	(revision 255578)
+++ config/sparc/sparc.h	(working copy)
@@ -771,13 +771,29 @@ extern enum cmodel sparc_cmodel;
 /* The soft frame pointer does not have the stack bias applied.  */
 #define FRAME_POINTER_REGNUM 101
 
-/* Given the stack bias, the stack pointer isn't actually aligned.  */
 #define INIT_EXPANDERS							 \
   do {									 \
-    if (crtl->emit.regno_pointer_align && SPARC_STACK_BIAS)	 \
+    if (crtl->emit.regno_pointer_align)					 \
       {									 \
-	REGNO_POINTER_ALIGN (STACK_POINTER_REGNUM) = BITS_PER_UNIT;	 \
-	REGNO_POINTER_ALIGN (HARD_FRAME_POINTER_REGNUM) = BITS_PER_UNIT; \
+	/* The biased stack pointer is only aligned on BITS_PER_UNIT.  */\
+	if (SPARC_STACK_BIAS)						 \
+	  {								 \
+	    REGNO_POINTER_ALIGN (STACK_POINTER_REGNUM)			 \
+	      = BITS_PER_UNIT;	 					 \
+	    REGNO_POINTER_ALIGN (HARD_FRAME_POINTER_REGNUM)		 \
+	      = BITS_PER_UNIT;						 \
+	  }								 \
+									 \
+	/* In 32-bit mode, not everything is double-word aligned.  */	 \
+	if (TARGET_ARCH32)						 \
+	  {								 \
+	    REGNO_POINTER_ALIGN (VIRTUAL_INCOMING_ARGS_REGNUM)		 \
+	      = BITS_PER_WORD;						 \
+	    REGNO_POINTER_ALIGN (VIRTUAL_STACK_DYNAMIC_REGNUM)		 \
+	      = BITS_PER_WORD;						 \
+	    REGNO_POINTER_ALIGN (VIRTUAL_OUTGOING_ARGS_REGNUM)		 \
+	      = BITS_PER_WORD;						 \
+	  }								 \
       }									 \
   } while (0)