[AArch64] Make sure start callee-save offset for D registers aligned

Jiong Wang jiong.wang@arm.com
Thu Jul 24 13:41:00 GMT 2014


Hi Maxim,

On 24/07/14 14:16, Maxim Kuvyrkov wrote:
> On Jun 5, 2014, at 3:04 PM, Jiong Wang <jiong.wang@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.
thanks for review.

sorry for haven't make a clearer statement. actually, this patch has 
pass aarch64 bare metal full test and no regression.

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

This patch was trying to fix a hidding performance problem. but later I 
found there maybe something wrong with this patch.

I'd re-investigate and re-base this patch  then update it with testcase.

thanks.

-- Jiong

>
>> 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
>
>
>
>
>
>




More information about the Gcc-patches mailing list