This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR target/18916; Function arg passing mem align fixes.


Hi Richard,
  Revised version.  As before, plus assign_parm_adjust_stack_rtl and
assign_parm_setup_block changes.  a_p_adjust_stack_rtl now discards
stack_parm as unsuitable when alignment isn't strict enough for the
powerpc backend, and a_p_setup_block makes a new stack slot and handles
copying from the original stack slot if necessary.  Without the
a_p_setup_block change, a BLKmode parm incoming on the stack won't have
a stack_parm made if the original is rejected by a_p_adjust_stack_rtl,
and Nasty Things happen after SET_DECL_RTL (parm, 0).  You'll note that
it wasn't just simply a matter of moving code around:  The PUT_MODE on
the stack slot is only done when the mem mode is the right size.  This
is to handle cases where entry_parm is something like (reg:DI 4) for
a struct parm of say 16 bytes.  Making the mem DImode would wrongly tell
other parts of GCC that the size was 8 bytes.

I chose to put PARM_NEEDS_ALIGN in rs6000.h so that this change would
mainly affect powerpc, as I'm not that familiar with other ABIs.  If
you think it suitable for all targets I'm quite happy to put the change
directly in a_p_adjust_stack_rtl.  If it's OK as is, then I'll document
the new target macro before committing.

Note that the code in a_p_setup_block to create the stack temp still
assigns mem alignment based on the type.  I don't believe that will
cause a problem, but it's not really correct.  The mem alignment should
be left as given by assign_stack_local, I think, but I left that alone
since there are a number of other places in function.c that do the same.
It's probably best to fix them all rather than piecemeal.

	PR target/18916
	* builtins.c (std_gimplify_va_arg_expr): Adjust alignment of *ap.
	* expr.h (struct locate_and_pad_arg_data): Add "boundary".
	* function.c (locate_and_pad_parm): Set new field.
	(assign_parm_find_stack_rtl): Use it instead of FUNCTION_ARG_BOUNDARY.
	Tweak where_pad test to include "none".  Always set mem align for
	stack_parm.
	(PARM_NEEDS_ALIGN): Define.
	(assign_parm_adjust_stack_rtl): Use PARM_NEEDS_ALIGN.
	(assign_parm_setup_block): If stack_parm is zero on entry, always
	make a new stack slot.  Block move old stack parm if necessary to
	new aligned stack slot.
	* calls.c (compute_argument_addresses): Override alignment of stack
	arg calculated from its type with the alignment given by
	FUNCTION_ARG_BOUNDARY.
	(store_one_arg): Likewise.
	* config/rs6000/rs6000.h (PARM_NEEDS_ALIGN): Define.

Bootstrapped and regression tested powerpc-linux, powerpc64-linux and
i686-linux.  OK for mainline?

Index: gcc/builtins.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/builtins.c,v
retrieving revision 1.410
diff -u -p -r1.410 builtins.c
--- gcc/builtins.c	14 Dec 2004 18:04:51 -0000	1.410
+++ gcc/builtins.c	7 Jan 2005 06:43:53 -0000
@@ -3928,6 +3928,18 @@ std_gimplify_va_arg_expr (tree valist, t
 		  build2 (BIT_AND_EXPR, TREE_TYPE (valist), valist_tmp, t));
       gimplify_and_add (t, pre_p);
     }
+  else
+    boundary = align;
+
+  /* If the actual alignment is less than the alignment of the type,
+     adjust the type accordingly so that we don't assume strict alignment
+     when deferencing the pointer.  */
+  boundary *= BITS_PER_UNIT;
+  if (boundary < TYPE_ALIGN (type))
+    {
+      type = build_variant_type_copy (type);
+      TYPE_ALIGN (type) = boundary;
+    }
 
   /* Compute the rounded size of the type.  */
   type_size = size_in_bytes (type);
Index: gcc/calls.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/calls.c,v
retrieving revision 1.375
diff -u -p -r1.375 calls.c
--- gcc/calls.c	30 Dec 2004 03:07:34 -0000	1.375
+++ gcc/calls.c	7 Jan 2005 06:43:54 -0000
@@ -1357,6 +1357,7 @@ compute_argument_addresses (struct arg_d
 	  rtx offset = ARGS_SIZE_RTX (args[i].locate.offset);
 	  rtx slot_offset = ARGS_SIZE_RTX (args[i].locate.slot_offset);
 	  rtx addr;
+	  unsigned int align, boundary;
 
 	  /* Skip this parm if it will not be passed on the stack.  */
 	  if (! args[i].pass_on_stack && args[i].reg != 0)
@@ -1369,9 +1370,18 @@ compute_argument_addresses (struct arg_d
 
 	  addr = plus_constant (addr, arg_offset);
 	  args[i].stack = gen_rtx_MEM (args[i].mode, addr);
-	  set_mem_align (args[i].stack, PARM_BOUNDARY);
 	  set_mem_attributes (args[i].stack,
 			      TREE_TYPE (args[i].tree_value), 1);
+	  align = BITS_PER_UNIT;
+	  boundary = args[i].locate.boundary;
+	  if (args[i].locate.where_pad != downward)
+	    align = boundary;
+	  else if (GET_CODE (offset) == CONST_INT)
+	    {
+	      align = INTVAL (offset) * BITS_PER_UNIT | boundary;
+	      align = align & -align;
+	    }
+	  set_mem_align (args[i].stack, align);
 
 	  if (GET_CODE (slot_offset) == CONST_INT)
 	    addr = plus_constant (arg_reg, INTVAL (slot_offset));
@@ -1380,9 +1390,9 @@ compute_argument_addresses (struct arg_d
 
 	  addr = plus_constant (addr, arg_offset);
 	  args[i].stack_slot = gen_rtx_MEM (args[i].mode, addr);
-	  set_mem_align (args[i].stack_slot, PARM_BOUNDARY);
 	  set_mem_attributes (args[i].stack_slot,
 			      TREE_TYPE (args[i].tree_value), 1);
+	  set_mem_align (args[i].stack_slot, args[i].locate.boundary);
 
 	  /* Function incoming arguments may overlap with sibling call
 	     outgoing arguments and we cannot allow reordering of reads
@@ -4119,9 +4129,7 @@ store_one_arg (struct arg_data *arg, rtx
 				  NULL_RTX, TYPE_MODE (sizetype), 0);
 	}
 
-      /* Some types will require stricter alignment, which will be
-	 provided for elsewhere in argument layout.  */
-      parm_align = MAX (PARM_BOUNDARY, TYPE_ALIGN (TREE_TYPE (pval)));
+      parm_align = arg->locate.boundary;
 
       /* When an argument is padded down, the block is aligned to
 	 PARM_BOUNDARY, but the actual argument isn't.  */
Index: gcc/expr.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.h,v
retrieving revision 1.184
diff -u -p -r1.184 expr.h
--- gcc/expr.h	30 Dec 2004 03:07:36 -0000	1.184
+++ gcc/expr.h	7 Jan 2005 06:43:54 -0000
@@ -116,6 +116,8 @@ struct locate_and_pad_arg_data
   struct args_size alignment_pad;
   /* Which way we should pad this arg.  */
   enum direction where_pad;
+  /* slot_offset is at least this aligned.  */
+  unsigned int boundary;
 };
 
 /* Add the value of the tree INC to the `struct args_size' TO.  */
Index: gcc/function.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/function.c,v
retrieving revision 1.599
diff -u -p -r1.599 function.c
--- gcc/function.c	2 Jan 2005 03:46:21 -0000	1.599
+++ gcc/function.c	7 Jan 2005 23:11:12 -0000
@@ -2405,22 +2405,21 @@ assign_parm_find_stack_rtl (tree parm, s
 
   set_mem_attributes (stack_parm, parm, 1);
 
-  boundary = FUNCTION_ARG_BOUNDARY (data->promoted_mode, data->passed_type);
-  align = 0;
+  boundary = data->locate.boundary;
+  align = BITS_PER_UNIT;
 
   /* If we're padding upward, we know that the alignment of the slot
      is FUNCTION_ARG_BOUNDARY.  If we're using slot_offset, we're
      intentionally forcing upward padding.  Otherwise we have to come
      up with a guess at the alignment based on OFFSET_RTX.  */
-  if (data->locate.where_pad == upward || data->entry_parm)
+  if (data->locate.where_pad != downward || data->entry_parm)
     align = boundary;
   else if (GET_CODE (offset_rtx) == CONST_INT)
     {
       align = INTVAL (offset_rtx) * BITS_PER_UNIT | boundary;
       align = align & -align;
     }
-  if (align > 0)
-    set_mem_align (stack_parm, align);
+  set_mem_align (stack_parm, align);
 
   if (data->entry_parm)
     set_reg_attrs_for_parm (data->entry_parm, stack_parm);
@@ -2492,6 +2491,11 @@ assign_parm_adjust_entry_rtl (struct ass
 /* A subroutine of assign_parms.  Adjust DATA->STACK_RTL such that it's
    always valid and properly aligned.  */
 
+#ifndef PARM_NEEDS_ALIGN
+#define PARM_NEEDS_ALIGN(DATA, HAVE_ALIGN) \
+  (STRICT_ALIGNMENT							\
+   && GET_MODE_ALIGNMENT (DATA->nominal_mode) > HAVE_ALIGN)
+#endif
 
 static void
 assign_parm_adjust_stack_rtl (struct assign_parm_data_one *data)
@@ -2501,8 +2505,7 @@ assign_parm_adjust_stack_rtl (struct ass
   /* If we can't trust the parm stack slot to be aligned enough for its
      ultimate type, don't use that slot after entry.  We'll make another
      stack slot, if we need one.  */
-  if (STRICT_ALIGNMENT && stack_parm
-      && GET_MODE_ALIGNMENT (data->nominal_mode) > MEM_ALIGN (stack_parm))
+  if (stack_parm && PARM_NEEDS_ALIGN (data, MEM_ALIGN (stack_parm)))
     stack_parm = NULL;
 
   /* If parm was passed in memory, and we need to convert it on entry,
@@ -2548,6 +2551,8 @@ assign_parm_setup_block (struct assign_p
 {
   rtx entry_parm = data->entry_parm;
   rtx stack_parm = data->stack_parm;
+  HOST_WIDE_INT size;
+  HOST_WIDE_INT size_stored;
 
   if (GET_CODE (entry_parm) == PARALLEL)
     entry_parm = emit_group_move_into_temps (entry_parm);
@@ -2593,12 +2598,20 @@ assign_parm_setup_block (struct assign_p
       return;
     }
 
+  size = int_size_in_bytes (data->passed_type);
+  size_stored = CEIL_ROUND (size, UNITS_PER_WORD);
+  if (stack_parm == 0)
+    {
+      stack_parm = assign_stack_local (BLKmode, size_stored, 0);
+      if (GET_MODE_SIZE (GET_MODE (entry_parm)) == size)
+	PUT_MODE (stack_parm, GET_MODE (entry_parm));
+      set_mem_attributes (stack_parm, parm, 1);
+    }
+
   /* If a BLKmode arrives in registers, copy it to a stack slot.  Handle
      calls that pass values in multiple non-contiguous locations.  */
   if (REG_P (entry_parm) || GET_CODE (entry_parm) == PARALLEL)
     {
-      HOST_WIDE_INT size = int_size_in_bytes (data->passed_type);
-      HOST_WIDE_INT size_stored = CEIL_ROUND (size, UNITS_PER_WORD);
       rtx mem;
 
       /* Note that we will be storing an integral number of words.
@@ -2610,13 +2623,8 @@ assign_parm_setup_block (struct assign_p
 	 if it becomes a problem.  Exception is when BLKmode arrives
 	 with arguments not conforming to word_mode.  */
 
-      if (stack_parm == 0)
-	{
-	  stack_parm = assign_stack_local (BLKmode, size_stored, 0);
-	  data->stack_parm = stack_parm;
-	  PUT_MODE (stack_parm, GET_MODE (entry_parm));
-	  set_mem_attributes (stack_parm, parm, 1);
-	}
+      if (data->stack_parm == 0)
+	;
       else if (GET_CODE (entry_parm) == PARALLEL)
 	;
       else
@@ -2686,7 +2694,11 @@ assign_parm_setup_block (struct assign_p
 	move_block_from_reg (REGNO (entry_parm), mem,
 			     size_stored / UNITS_PER_WORD);
     }
+  else if (data->stack_parm == 0)
+    emit_block_move (stack_parm, data->entry_parm, GEN_INT (size),
+		     BLOCK_OP_NORMAL);
 
+  data->stack_parm = stack_parm;
   SET_DECL_RTL (parm, stack_parm);
 }
 
@@ -3411,6 +3423,7 @@ locate_and_pad_parm (enum machine_mode p
   where_pad = FUNCTION_ARG_PADDING (passed_mode, type);
   boundary = FUNCTION_ARG_BOUNDARY (passed_mode, type);
   locate->where_pad = where_pad;
+  locate->boundary = boundary;
 
 #ifdef ARGS_GROW_DOWNWARD
   locate->slot_offset.constant = -initial_offset_ptr->constant;
Index: gcc/config/rs6000/rs6000.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.h,v
retrieving revision 1.350
diff -u -p -r1.350 rs6000.h
--- gcc/config/rs6000/rs6000.h	30 Dec 2004 03:08:05 -0000	1.350
+++ gcc/config/rs6000/rs6000.h	7 Jan 2005 10:17:31 -0000
@@ -800,6 +800,13 @@ extern const char *rs6000_warn_altivec_l
    || (((MODE) == SFmode || (MODE) == DFmode || (MODE) == TFmode	\
 	|| (MODE) == DImode)						\
        && (ALIGN) < 32))
+
+#define PARM_NEEDS_ALIGN(DATA, HAVE_ALIGN) \
+  ((STRICT_ALIGNMENT							\
+    && GET_MODE_ALIGNMENT (DATA->nominal_mode) > HAVE_ALIGN)		\
+   || (DATA->nominal_type						\
+       && TYPE_ALIGN (DATA->nominal_type) > HAVE_ALIGN			\
+       && HAVE_ALIGN < PREFERRED_STACK_BOUNDARY))
 
 /* Standard register usage.  */
 

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]