Hi Maxim,
On 24/07/14 14:16, Maxim Kuvyrkov wrote:
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.
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
