On Mon, 17 Aug 2020 at 11:00, Alex Coplan <alex.cop...@arm.com> wrote:
>
> Hello,
>
> Given the following C function:
>
> double *f(double *p, unsigned x)
> {
> return p + x;
> }
>
> prior to this patch, GCC at -O2 would generate:
>
> f:
> add x0, x0, x1, uxtw 3
> ret
>
> but this add instruction uses architecturally-invalid syntax: the width
> of the third operand conflicts with the width of the extension
> specifier. The third operand is only permitted to be an x register when
> the extension specifier is (u|s)xtx.
>
> This instruction, and analogous insns for adds, sub, subs, and cmp, are
> rejected by clang, but accepted by binutils. Assembling and
> disassembling such an insn with binutils gives the architecturally-valid
> version in the disassembly:
>
> 0: 8b214c00 add x0, x0, w1, uxtw #3
>
> This patch fixes several patterns in the AArch64 backend to use the
> standard syntax as specified in the Arm ARM such that GCC's output can
> be assembled by assemblers other than GAS.
>
> Note that an obvious omission here is that this patch does not touch the
> mult patterns such as *add_<optab><ALLX:mode>_mult_<GPI:mode>. I found
> that I couldn't hit these patterns with C code since multiplications by
> powers of two always get turned into shifts by earlier RTL passes. If
> there's a way to reliably hit these patterns, then perhaps these should
> be updated as well.
>
> Testing:
> * New test which checks for the correct syntax in all updated
> patterns (fails before and passes after the aarch64.md change).
> * New test can be assembled by both GAS and llvm-mc following the
> change.
> * Bootstrapped and regtested on aarch64-none-linux-gnu.
>
> OK for master?
>
> Thanks,
> Alex
>
> ---
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.md
> (*adds_<optab><ALLX:mode>_<GPI:mode>): Ensure extended operand
> agrees with width of extension specifier.
> (*subs_<optab><ALLX:mode>_<GPI:mode>): Likewise.
> (*adds_<optab><ALLX:mode>_shift_<GPI:mode>): Likewise.
> (*subs_<optab><ALLX:mode>_shift_<GPI:mode>): Likewise.
> (*add_<optab><ALLX:mode>_<GPI:mode>): Likewise.
> (*add_<optab><ALLX:mode>_shft_<GPI:mode>): Likewise.
> (*add_uxt<mode>_shift2): Likewise.
> (*sub_<optab><ALLX:mode>_<GPI:mode>): Likewise.
> (*sub_<optab><ALLX:mode>_shft_<GPI:mode>): Likewise.
> (*sub_uxt<mode>_shift2): Likewise.
> (*cmp_swp_<optab><ALLX:mode>_reg<GPI:mode>): Likewise.
> (*cmp_swp_<optab><ALLX:mode>_shft_<GPI:mode>): Likewise.
>
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/adds3.c: Fix test w.r.t. new syntax.
> * gcc.target/aarch64/cmp.c: Likewise.
> * gcc.target/aarch64/subs3.c: Likewise.
> * gcc.target/aarch64/subsp.c: Likewise.
> * gcc.target/aarch64/extend-syntax.c: New test.
>
Hi,
I've noticed some of the new tests fail with -mabi=ilp32:
gcc.target/aarch64/extend-syntax.c check-function-bodies add1
gcc.target/aarch64/extend-syntax.c check-function-bodies add3
gcc.target/aarch64/extend-syntax.c check-function-bodies sub2
gcc.target/aarch64/extend-syntax.c check-function-bodies sub3
gcc.target/aarch64/extend-syntax.c scan-assembler-times
subs\tx[0-9]+, x[0-9]+, w[0-9]+, sxtw 3 1
gcc.target/aarch64/subsp.c scan-assembler sub\tsp, sp, w[0-9]*, sxtw 4\n
Christophe
