Hi Tamar

On 13/08/18 17:27, Tamar Christina wrote:
Hi Thomas,

Thanks for the review.

I’ll correct the typo before committing if I have no other changes required by 
a maintainer.

Regards,
Tamar.


I am not a maintainer but I would like to point out something in your
patch. I think you test case will fail with -mabi=ilp32

FAIL: gcc.target/aarch64/large_struct_copy_2.c (test for excess errors)
Excess errors:
/work/trunk/src/gcc/gcc/testsuite/gcc.target/aarch64/large_struct_copy_2.c:18:27: warning: overflow in conversion from 'long
long int' to 'long int' changes value from '4073709551611' to
'2080555003' [-Woverflow]

We have had more such recent failures and James gave a very neat
way to make sure the mode comes out what you intend it to here:
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00233.html

I would just ask you to change the data types accordingly and test it
with -mabi=ilp32.

Thanks
Sudi

From: Thomas Preudhomme <thomas.preudho...@linaro.org>
Sent: Monday, August 13, 2018 14:37
To: Tamar Christina <tamar.christ...@arm.com>
Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; James Greenhalgh 
<james.greenha...@arm.com>; Richard Earnshaw <richard.earns...@arm.com>; Marcus Shawcroft 
<marcus.shawcr...@arm.com>
Subject: Re: [PATCH][GCC][AArch64] Limit movmem copies to TImode copies.

Hi Tamar,

Thanks for your patch.

Just one comment about your ChangeLog entry for the testsuiet change: shouldn't 
it mention that it is a new testcase? The patch you attached seems to create 
the file.

Best regards,

Thomas

On Mon, 13 Aug 2018 at 10:33, Tamar Christina 
<tamar.christ...@arm.com<mailto:tamar.christ...@arm.com>> wrote:
Hi All,

On AArch64 we have integer modes larger than TImode, and while we can generate
moves for these they're not as efficient.

So instead make sure we limit the maximum we can copy to TImode.  This means
copying a 16 byte struct will issue 1 TImode copy, which will be done using a
single STP as we expect but an CImode sized copy won't issue CImode operations.

Bootstrapped and regtested on aarch4-none-linux-gnu and no issues.
Crosstested aarch4_be-none-elf and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/
2018-08-13  Tamar Christina  
<tamar.christ...@arm.com<mailto:tamar.christ...@arm.com>>

         * config/aarch64/aarch64.c (aarch64_expand_movmem): Set TImode max.

gcc/testsuite/
2018-08-13  Tamar Christina  
<tamar.christ...@arm.com<mailto:tamar.christ...@arm.com>>

         * gcc.target/aarch64/large_struct_copy_2.c: Add assembler scan.

--


Reply via email to