On Jun 5, 2014, at 3:04 PM, Jiong Wang <jiong.w...@arm.com> wrote:

> For AArch64, there may have been an odd num core registers need to be saved.
> 
> This small patch ensure we remain 16 byte aligned for subsequent STP writes 
> of D registers.
> 
> OK for trunk?

Hi Jiong,

How did you test the patch?  You need to run GCC testsuites, and, ideally, 
bootstrap on aarch64-linux-gnu.

Is this patch to fix a correctness problem?  Can you provide a testcase that 
breaks without this patch?

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index aada704..c4abf1e 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -1793,6 +1793,10 @@ aarch64_layout_frame (void)
>       offset += UNITS_PER_WORD;
>        }
>  
> +  /* Align offset to 16-bytes.
> +     There may have been an odd num core registers. Ensure we remain
> +     16 byte aligned for subsequent STP writes of D registers.  */
> +  offset = AARCH64_ROUND_UP (offset, 2 * UNITS_PER_WORD);
>    for (regno = V0_REGNUM; regno <= V31_REGNUM; regno++)
>      if (cfun->machine->frame.reg_offset[regno] == SLOT_REQUIRED)
>        {

It seems that we need to align the offset to 16 bytes only when the V0_REGNUM 
loop stores a register pair, but not a single register.  Since non-pair stores 
would occur only when a single register is stored in V0_REGNUM loop (i.e., 
comparatively rare occurrence), we should be OK to align the offset anyway.

The patch seems OK to me (provided no regressions on the testsuite), but you 
need to get an ACK from an official maintainer.

Thank you,

--
Maxim Kuvyrkov
www.linaro.org



Reply via email to