This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR 16660, attribute((aligned)) doesn't work for variables on the stack for greater than required alignment
- From: Richard Henderson <rth at redhat dot com>
- To: Andrew_Pinski at PlayStation dot Sony dot com
- Cc: gcc-patches at gcc dot gnu dot org, Trevor_Smigiel at PlayStation dot Sony dot com, Russell_Olsen at PlayStation dot Sony dot com
- Date: Fri, 2 Feb 2007 15:49:10 -0800
- Subject: Re: [PATCH] Fix PR 16660, attribute((aligned)) doesn't work for variables on the stack for greater than required alignment
- References: <OF2591087A.FB5E6096-ON8825726E.001A7ADD-88257273.000DD338@playstation.sony.com>
On Mon, Jan 29, 2007 at 06:31:00PM -0800, Andrew_Pinski@PlayStation.Sony.com wrote:
> + tmp = assign_stack_temp_for_type (mode, size, keep, type);
> + addr = XEXP (tmp, 0);
> + /* CEIL_DIV_EXPR needs to worry about the addition overflowing,
> + but we know it can't. So add ourselves and then do
> + TRUNC_DIV_EXPR. */
> + addr = expand_binop (Pmode, add_optab, addr,
> + GEN_INT (align / BITS_PER_UNIT - 1),
> + NULL_RTX, 1, OPTAB_LIB_WIDEN);
> + addr = expand_divmod (0, TRUNC_DIV_EXPR, Pmode, addr,
> + GEN_INT (align / BITS_PER_UNIT),
> + NULL_RTX, 1);
> + addr = expand_mult (Pmode, addr,
> + GEN_INT (align / BITS_PER_UNIT),
> + NULL_RTX, 1);
...
> +alloc_stack_frame_space (HOST_WIDE_INT size, unsigned HOST_WIDE_INT align)
> {
> HOST_WIDE_INT offset, new_frame_offset;
>
> new_frame_offset = frame_offset;
> +
> + /* Allocate extra space if the alignment required is greater than the
> + stack boundary and then assume the RTL expansion of the variable, gets
> + the correct alignment. */
> + if (align > PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT)
> + {
> + size += align;
> + align = PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT;
I don't like how these two things are split apart. Either cfgexpand.c
should handle both the addition and the rounding, or alloc_stack_frame_space
should.
> + /* When the variable needs additional code to be aligned on the
> + stack we allocate separately just for ease of implementation. */
> + if (get_decl_align_unit (var) > PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT)
> + return false;
Which does suggest it'd be interesting to create a block containing
all of the over-aligned data, create one REG that contains the
rounded address of the base of that block, and all of the data
therein is offsets from that one REG.
Most of the infrastructure for that is already present, what with
the partitioning and sorting that we already do...
> @@ -1892,7 +1900,8 @@ expand_decl (tree decl)
> set_mem_attributes (x, decl, 1);
> SET_DECL_RTL (decl, x);
> }
> - else if (use_register_for_decl (decl))
> + else if (use_register_for_decl (decl)
> + && align <= PREFERRED_STACK_BOUNDARY)
Why do we need to check alignment for register data?
r~