This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR target/18916; Function arg passing mem align fixes.
On Sat, Jan 08, 2005 at 01:55:51PM -0800, Richard Henderson wrote:
> On Sat, Jan 08, 2005 at 10:00:04AM +1030, Alan Modra wrote:
> > 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.
>
> I think that it's definitely suitable for all targets. More or less
> by definition -- it's exactly the condition that we're providing.
>
> > + stack_parm = assign_stack_local (BLKmode, size_stored, 0);
>
> Don't you know the alignment that you require?
>
> I believe that you need similar changes to assign_parm_setup_stack.
OK, I changed assign_parm_setup_stack to handle mis-aligned stack slots,
and tweaked some assign_stack_local calls to specify alignment.
The testsuite changes I found useful when debugging this problem.
gcc/
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.
(assign_parm_adjust_stack_rtl): Discard stack_parm if alignment
not sufficient for type.
(assign_parm_setup_block): If stack_parm is zero on entry, always
make a new stack local. Block move old stack parm if necessary
to new aligned stack local.
(assign_parm_setup_stack): Use a block move to handle
potentially misaligned entry_parm.
(assign_parms_unsplit_complex): Specify required alignment when
creating stack local. Don't override with alignment of type.
* config/rs6000/rs6000.h (PARM_NEEDS_ALIGN): Define.
* 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.
gcc/testsuite/
* g++.dg/ext/altivec-3.C (baz, baz2): Check one component of structs
at a time.
* ext/altivec_check.h: Support compiling as C.
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.411
diff -u -p -r1.411 builtins.c
--- gcc/builtins.c 7 Jan 2005 09:00:50 -0000 1.411
+++ gcc/builtins.c 10 Jan 2005 00:34:01 -0000
@@ -3894,6 +3894,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/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 10 Jan 2005 00:34:06 -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 10 Jan 2005 00:34:10 -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,7 +2491,6 @@ 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. */
-
static void
assign_parm_adjust_stack_rtl (struct assign_parm_data_one *data)
{
@@ -2501,8 +2499,12 @@ 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
+ && ((STRICT_ALIGNMENT
+ && GET_MODE_ALIGNMENT (data->nominal_mode) > MEM_ALIGN (stack_parm))
+ || (data->nominal_type
+ && TYPE_ALIGN (data->nominal_type) > MEM_ALIGN (stack_parm)
+ && MEM_ALIGN (stack_parm) < PREFERRED_STACK_BOUNDARY)))
stack_parm = NULL;
/* If parm was passed in memory, and we need to convert it on entry,
@@ -2548,6 +2550,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,30 +2597,34 @@ 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,
+ TYPE_ALIGN (data->passed_type));
+ 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.
So we have to be careful to ensure that we allocate an
- integral number of words. We do this below in the
+ integral number of words. We do this above when we call
assign_stack_local if space was not allocated in the argument
list. If it was, this will not work if PARM_BOUNDARY is not
a multiple of BITS_PER_WORD. It isn't clear how to fix this
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);
}
@@ -2903,33 +2915,37 @@ assign_parm_setup_stack (struct assign_p
/* ??? This may need a big-endian conversion on sparc64. */
data->stack_parm
= adjust_address (data->stack_parm, data->nominal_mode, 0);
-
- all->conversion_insns = get_insns ();
- end_sequence ();
}
if (data->entry_parm != data->stack_parm)
{
+ rtx src, dest;
+
if (data->stack_parm == 0)
{
data->stack_parm
= assign_stack_local (GET_MODE (data->entry_parm),
GET_MODE_SIZE (GET_MODE (data->entry_parm)),
- 0);
+ TYPE_ALIGN (data->passed_type));
set_mem_attributes (data->stack_parm, parm, 1);
}
- if (data->promoted_mode != data->nominal_mode)
- {
- push_to_sequence (all->conversion_insns);
- emit_move_insn (validize_mem (data->stack_parm),
- validize_mem (data->entry_parm));
- all->conversion_insns = get_insns ();
- end_sequence ();
- }
+ dest = validize_mem (data->stack_parm);
+ src = validize_mem (data->entry_parm);
+
+ if (MEM_P (src))
+ /* Use a block move to handle potentially misaligned entry_parm. */
+ emit_block_move (dest, src,
+ GEN_INT (int_size_in_bytes (data->passed_type)),
+ BLOCK_OP_NORMAL);
else
- emit_move_insn (validize_mem (data->stack_parm),
- validize_mem (data->entry_parm));
+ emit_move_insn (dest, src);
+ }
+
+ if (data->promoted_mode != data->nominal_mode)
+ {
+ all->conversion_insns = get_insns ();
+ end_sequence ();
}
SET_DECL_RTL (parm, data->stack_parm);
@@ -2967,7 +2983,8 @@ assign_parms_unsplit_complex (struct ass
/* split_complex_arg put the real and imag parts in
pseudos. Move them to memory. */
- tmp = assign_stack_local (DECL_MODE (parm), size, 0);
+ tmp = assign_stack_local (DECL_MODE (parm), size,
+ TYPE_ALIGN (TREE_TYPE (parm)));
set_mem_attributes (tmp, parm, 1);
rmem = adjust_address_nv (tmp, inner, 0);
imem = adjust_address_nv (tmp, inner, GET_MODE_SIZE (inner));
@@ -3411,6 +3428,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/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 10 Jan 2005 00:34:03 -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/testsuite/g++.dg/ext/altivec-3.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.dg/ext/altivec-3.C,v
retrieving revision 1.3
diff -u -p -r1.3 altivec-3.C
--- gcc/testsuite/g++.dg/ext/altivec-3.C 8 Jun 2004 18:32:15 -0000 1.3
+++ gcc/testsuite/g++.dg/ext/altivec-3.C 10 Jan 2005 00:34:51 -0000
@@ -75,11 +75,15 @@ void baz (int i, ... )
CHECK_INVARIANT (vec_all_eq (v_i, v_g));
CHECK_INVARIANT (j == i_1);
- CHECK_INVARIANT (vx.x == vx_g.x && vec_all_eq(vx.v, vx_g.v) && vx.y == vx_g.y);
+ CHECK_INVARIANT (vx.x == vx_g.x);
+ CHECK_INVARIANT (vec_all_eq (vx.v, vx_g.v));
+ CHECK_INVARIANT (vx.y == vx_g.y);
CHECK_INVARIANT (k == i_1);
CHECK_INVARIANT (vec_all_eq (v2_i, v2_g));
CHECK_INVARIANT (l == i_1);
- CHECK_INVARIANT (vx2.x == vx2_g.x && vec_all_eq(vx2.v, vx2_g.v) && vx2.y == vx2_g.y);
+ CHECK_INVARIANT (vx2.x == vx2_g.x);
+ CHECK_INVARIANT (vec_all_eq (vx2.v, vx2_g.v));
+ CHECK_INVARIANT (vx2.y == vx2_g.y);
}
void quux (int i, ... )
@@ -110,7 +114,9 @@ void baz2 (int i, ... )
va_end(ap);
vxi.v = vx.v;
- CHECK_INVARIANT (vx.x == vx_g.x && vec_all_eq(vx.v, vx_g.v) && vx.y == vx_g.y);
+ CHECK_INVARIANT (vx.x == vx_g.x);
+ CHECK_INVARIANT (vec_all_eq (vx.v, vx_g.v));
+ CHECK_INVARIANT (vx.y == vx_g.y);
CHECK_INVARIANT (vec_all_eq (vxi.v, vx_g.v));
}
Index: gcc/testsuite/g++.dg/ext/altivec_check.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.dg/ext/altivec_check.h,v
retrieving revision 1.2
diff -u -p -r1.2 altivec_check.h
--- gcc/testsuite/g++.dg/ext/altivec_check.h 5 Aug 2004 05:31:30 -0000 1.2
+++ gcc/testsuite/g++.dg/ext/altivec_check.h 10 Jan 2005 00:34:51 -0000
@@ -2,7 +2,11 @@
/* Contributed by Ziemowit Laski <zlaski@apple.com> */
#include <signal.h>
-extern "C" void exit(int);
+extern
+#ifdef __cplusplus
+"C"
+#endif
+void exit(int);
void
sig_ill_handler (int sig)
--
Alan Modra
IBM OzLabs - Linux Technology Centre