This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v3] [aarch64] Correct the maximum shift amount for shifted operands.
- From: Philipp Tomsich <philipp dot tomsich at theobroma-systems dot com>
- To: Sam Tebbs <Sam dot Tebbs at arm dot com>
- Cc: Christoph Muellner <christoph dot muellner at theobroma-systems dot com>, Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, James Greenhalgh <James dot Greenhalgh at arm dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Andrew Pinksi <pinskia at gmail dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Tue, 27 Nov 2018 14:18:35 +0100
- Subject: Re: [PATCH v3] [aarch64] Correct the maximum shift amount for shifted operands.
- References: <20181126195017.1718-1-christoph.muellner@theobroma-systems.com> <b611544e-b4f7-5b15-351d-56ed9b228938@arm.com>
Sam,
> On 27.11.2018, at 14:06, Sam Tebbs <Sam.Tebbs@arm.com> wrote:
>
>
> On 11/26/18 7:50 PM, Christoph Muellner wrote:
>> The aarch64 ISA specification allows a left shift amount to be applied
>> after extension in the range of 0 to 4 (encoded in the imm3 field).
>>
>> This is true for at least the following instructions:
>>
>> * ADD (extend register)
>> * ADDS (extended register)
>> * SUB (extended register)
>>
>> The result of this patch can be seen, when compiling the following code:
>>
>> uint64_t myadd(uint64_t a, uint64_t b)
>> {
>> return a+(((uint8_t)b)<<4);
>> }
>>
>> Without the patch the following sequence will be generated:
>>
>> 0000000000000000 <myadd>:
>> 0: d37c1c21 ubfiz x1, x1, #4, #8
>> 4: 8b000020 add x0, x1, x0
>> 8: d65f03c0 ret
>>
>> With the patch the ubfiz will be merged into the add instruction:
>>
>> 0000000000000000 <myadd>:
>> 0: 8b211000 add x0, x0, w1, uxtb #4
>> 4: d65f03c0 ret
>
> Hi Christoph,
>
> Thanks for this, I'm not a maintainer but this looks good to me. A good
> point to mention would be how it affects shifts smaller than 4 bits,
> since I don't see any testing for that in the files you have changed.
>
>> Tested with "make check" and no regressions found.
> Could you perhaps elaborate on your testing? So what triplets you
> tested, if you bootstrapped successfully etc.
Just one bit of background info...
We’ve had this change in production since 2014 both in AppliedMicro’s
(now Ampere’s) compiler branch as well as in Ubuntu PPA for various
workloads.
Thanks,
Philipp.