------- Additional Comments From fjahanian at apple dot com 2004-12-21 01:25
-------
My last patch also had problems, in that it changed alignment of local vector
variables on stack.
This alignment cannot be changed because AltiVec intrincics expect 128bit
alignment. So,
I conclude that only tempoaries with expected 128bit or more alignments are not
aligned
properly. The safest fix would be to simply change the alignment at the rtl
level when
temporaries of 128bit alignment need be generated. This requires change to the
middle end.
Following patch shows the concept and is not an FSF ready patch (which requires
target-hook or
some such). This patch essentially says that if 128 alignment of local
temporaries on stack can not
be guaranteed (or changed), then set the alignment value in the rtl to what can
be guaranteed.
With this patch, emit_block_move will not generate lvx/stvx for these cases.
Index: expr.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.761
diff -c -p -r1.761 expr.c
*** expr.c 18 Dec 2004 14:38:31 -0000 1.761
--- expr.c 21 Dec 2004 01:23:27 -0000
*************** emit_push_insn (rtx x, enum machine_mode
*** 3457,3463 ****
to record the alignment of the stack slot. */
/* ALIGN may well be better aligned than TYPE, e.g. due to
PARM_BOUNDARY. Assume the caller isn't lying. */
! set_mem_align (target, align);
emit_block_move (target, xinner, size, BLOCK_OP_CALL_PARM);
}
--- 3457,3469 ----
to record the alignment of the stack slot. */
/* ALIGN may well be better aligned than TYPE, e.g. due to
PARM_BOUNDARY. Assume the caller isn't lying. */
! /* powerpc-darwin currently does not enforce 128 bit alignment of
! temporaries on the stack. To do so, requires changes which will
break
! ABI compatibility. On the other hand, Leaving this unchanged
generates
! incorrect code in cases where block move is implemented using
! AltiVec instructions whose src and dest must be 128 bit aligned
! (expand_block_move implementation in rs6000.c). */
! set_mem_align (target, align >= 128 ? PARM_BOUNDARY : align);
emit_block_move (target, xinner, size, BLOCK_OP_CALL_PARM);
}
*************** store_expr (tree exp, rtx target, int ca
*** 4206,4214 ****
emit_group_load (target, temp, TREE_TYPE (exp),
int_size_in_bytes (TREE_TYPE (exp)));
else if (GET_MODE (temp) == BLKmode)
! emit_block_move (target, temp, expr_size (exp),
! (call_param_p
! ? BLOCK_OP_CALL_PARM : BLOCK_OP_NORMAL));
else
{
temp = force_operand (temp, target);
--- 4212,4224 ----
emit_group_load (target, temp, TREE_TYPE (exp),
int_size_in_bytes (TREE_TYPE (exp)));
else if (GET_MODE (temp) == BLKmode)
! {
! /* See previous comment. */
! set_mem_align (temp, MEM_ALIGN (temp) >= 128 ? PARM_BOUNDARY :
MEM_ALIGN (temp));
! emit_block_move (target, temp, expr_size (exp),
! (call_param_p
! ? BLOCK_OP_CALL_PARM : BLOCK_OP_NORMAL));
! }
else
{
temp = force_operand (temp, target);
(In reply to comment #6)
> And this is the patch that I had in mind. Can this break ABI compatibily? My
> limited testing shows
> that it does not.
>
> Index: rs6000.c
>
===============================================================
> ====
> RCS file: /cvs/gcc/gcc/gcc/config/rs6000/rs6000.c,v
> retrieving revision 1.332.2.46.2.84
> diff -c -p -r1.332.2.46.2.84 rs6000.c
> *** rs6000.c 16 Dec 2004 03:23:30 -0000 1.332.2.46.2.84
> --- rs6000.c 18 Dec 2004 01:44:28 -0000
> *************** function_arg_boundary (enum machine_mode
> *** 5190,5195 ****
> --- 5190,5201 ----
> || (type && TREE_CODE (type) == VECTOR_TYPE
> && int_size_in_bytes (type) >= 16))
> return 128;
> + else if (DEFAULT_ABI == ABI_DARWIN && mode == BLKmode
> + && TYPE_ALIGN (type) >= 128)
> + {
> + TYPE_ALIGN (type) = PARM_BOUNDARY;
> + return PARM_BOUNDARY;
> + }
> else
> return PARM_BOUNDARY;
> }
>
--
What |Removed |Added
----------------------------------------------------------------------------
CC| |dalej at apple dot com
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18916
