[Bug target/97150] New: [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[Bug target/97150] New: [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[david.spickett at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

            Bug ID: 97150
           Summary: [AArch64] 2nd parameter of unsigned Neon scalar shift
                    intrinsics should be signed
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: david.spickett at linaro dot org
  Target Milestone: ---

The attached test file assigns the following intrinsics to function pointers to
check the argument and return types:
vqshlb_u8 vqshlh_u16 vqshls_u32 vqshld_u64
vqrshlb_u8 vqrshlh_u16 vqrshls_u32 vqrshld_u64
vshld_u64

(for quick reference)
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqshlb_u8
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqshlh_u16
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqshls_u32
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqshld_u64
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqrshlb_u8
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqrshlh_u16
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqrshls_u32
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqrshld_u64
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vshld_u64
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vrshld_u64

The types in the function pointer declaration are taken from the ACLE, so I'd
expect this to compile without issue. Except that the second parameter for
these intrinsics is not signed as it should be.

gcc version: 11.0.0
System: Ubuntu 16.04.7 LTS on x86_64
gcc options:
Configured with:
/tmp/dgboter/bbs/rhev-vm4--rhe6x86_64/buildbot/rhe6x86_64--aarch64-none-elf/build/src/gcc/configure
--target=aarch64-none-elf
--prefix=/tmp/dgboter/bbs/rhev-vm4--rhe6x86_64/buildbot/rhe6x86_64--aarch64-none-elf/build/build-aarch64-none-elf/install//
--with-gmp=/tmp/dgboter/bbs/rhev-vm4--rhe6x86_64/buildbot/rhe6x86_64--aarch64-none-elf/build/build-aarch64-none-elf/host-tools
--with-mpfr=/tmp/dgboter/bbs/rhev-vm4--rhe6x86_64/buildbot/rhe6x86_64--aarch64-none-elf/build/build-aarch64-none-elf/host-tools
--with-mpc=/tmp/dgboter/bbs/rhev-vm4--rhe6x86_64/buildbot/rhe6x86_64--aarch64-none-elf/build/build-aarch64-none-elf/host-tools
--with-isl=/tmp/dgboter/bbs/rhev-vm4--rhe6x86_64/buildbot/rhe6x86_64--aarch64-none-elf/build/build-aarch64-none-elf/host-tools
--disable-shared --disable-nls --disable-threads --disable-tls
--enable-checking=yes --enable-languages=c,c++,fortran --with-newlib
--with-pkgversion=fsf-trunk.2226

Command line to reproduce and output:
$ ./aarch64-none-elf-gcc -march=armv8-a+simd -c -o /dev/null /tmp/test.c -Wall
-Wextra
/tmp/test.c:3:38: warning: initialization of 'uint8_t (*)(uint8_t,  int8_t)'
{aka 'unsigned char (*)(unsigned char,  signed char)'} from incompatible
pointer type 'uint8_t (*)(uint8_t,  uint8_t)' {aka 'unsigned char (*)(unsigned
char,  unsigned char)'} [-Wincompatible-pointer-types]
    3 | uint8_t  (*fp0)(uint8_t, int8_t)   = vqshlb_u8;
      |                                      ^~~~~~~~~
/tmp/test.c:4:38: warning: initialization of 'uint16_t (*)(uint16_t,  int16_t)'
{aka 'short unsigned int (*)(short unsigned int,  short int)'} from
incompatible pointer type 'uint16_t (*)(uint16_t,  uint16_t)' {aka 'short
unsigned int (*)(short unsigned int,  short unsigned int)'}
[-Wincompatible-pointer-types]
    4 | uint16_t (*fp1)(uint16_t, int16_t) = vqshlh_u16;
      |                                      ^~~~~~~~~~
/tmp/test.c:5:38: warning: initialization of 'uint32_t (*)(uint32_t,  int32_t)'
{aka 'unsigned int (*)(unsigned int,  int)'} from incompatible pointer type
'uint32_t (*)(uint32_t,  uint32_t)' {aka 'unsigned int (*)(unsigned int, 
unsigned int)'} [-Wincompatible-pointer-types]
    5 | uint32_t (*fp2)(uint32_t, int32_t) = vqshls_u32;
      |                                      ^~~~~~~~~~
/tmp/test.c:6:38: warning: initialization of 'uint64_t (*)(uint64_t,  int64_t)'
{aka 'long unsigned int (*)(long unsigned int,  long int)'} from incompatible
pointer type 'uint64_t (*)(uint64_t,  uint64_t)' {aka 'long unsigned int
(*)(long unsigned int,  long unsigned int)'} [-Wincompatible-pointer-types]
    6 | uint64_t (*fp3)(uint64_t, int64_t) = vqshld_u64;
      |                                      ^~~~~~~~~~
/tmp/test.c:7:38: warning: initialization of 'uint8_t (*)(uint8_t,  int8_t)'
{aka 'unsigned char (*)(unsigned char,  signed char)'} from incompatible
pointer type 'uint8_t (*)(uint8_t,  uint8_t)' {aka 'unsigned char (*)(unsigned
char,  unsigned char)'} [-Wincompatible-pointer-types]
    7 | uint8_t  (*fp4)(uint8_t, int8_t)   = vqrshlb_u8;
      |                                      ^~~~~~~~~~
/tmp/test.c:8:38: warning: initialization of 'uint16_t (*)(uint16_t,  int16_t)'
{aka 'short unsigned int (*)(short unsigned int,  short int)'} from
incompatible pointer type 'uint16_t (*)(uint16_t,  uint16_t)' {aka 'short
unsigned int (*)(short unsigned int,  short unsigned int)'}
[-Wincompatible-pointer-types]
    8 | uint16_t (*fp5)(uint16_t, int16_t) = vqrshlh_u16;
      |                                      ^~~~~~~~~~~
/tmp/test.c:9:38: warning: initialization of 'uint32_t (*)(uint32_t,  int32_t)'
{aka 'unsigned int (*)(unsigned int,  int)'} from incompatible pointer type
'uint32_t (*)(uint32_t,  uint32_t)' {aka 'unsigned int (*)(unsigned int, 
unsigned int)'} [-Wincompatible-pointer-types]
    9 | uint32_t (*fp6)(uint32_t, int32_t) = vqrshls_u32;
      |                                      ^~~~~~~~~~~
/tmp/test.c:10:38: warning: initialization of 'uint64_t (*)(uint64_t, 
int64_t)' {aka 'long unsigned int (*)(long unsigned int,  long int)'} from
incompatible pointer type 'uint64_t (*)(uint64_t,  uint64_t)' {aka 'long
unsigned int (*)(long unsigned int,  long unsigned int)'}
[-Wincompatible-pointer-types]
   10 | uint64_t (*fp7)(uint64_t, int64_t) = vqrshld_u64;
      |                                      ^~~~~~~~~~~
/tmp/test.c:11:38: warning: initialization of 'uint64_t (*)(uint64_t, 
int64_t)' {aka 'long unsigned int (*)(long unsigned int,  long int)'} from
incompatible pointer type 'uint64_t (*)(uint64_t,  uint64_t)' {aka 'long
unsigned int (*)(long unsigned int,  long unsigned int)'}
[-Wincompatible-pointer-types]
   11 | uint64_t (*fp8)(uint64_t, int64_t) = vshld_u64;
      |                                      ^~~~~~~~~

For what it's worth this also fails to compile with "gcc version 5.4.0 20160609
(Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9)". (so it isn't a new change)

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[david.spickett at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

--- Comment #1 from David Spickett <david.spickett at linaro dot org> ---
Created attachment 49248
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49248&action=edit
reproducer C file

Added test file.

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[david.spickett at linaro dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

--- Comment #2 from David Spickett <david.spickett at linaro dot org> ---
Created attachment 49250
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49250&action=edit
preprocessed test file

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Kyrylo Tkachov <ktkachov@gcc.gnu.org>:

https://gcc.gnu.org/g:2d8fbebdb1eaca8de557ab3052535a8e4b8f8972

commit r11-3548-g2d8fbebdb1eaca8de557ab3052535a8e4b8f8972
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:01:23 2020 +0100

    PR target/97150 AArch64: 2nd parameter of unsigned Neon scalar shift
intrinsics should be signed

    In this PR the second argument to the intrinsics should be signed but we
    use an unsigned one erroneously.
    The corresponding builtins are already using the correct types so it's
    just a matter of correcting the signatures in arm_neon.h

    gcc/
            PR target/97150
            * config/aarch64/arm_neon.h (vqrshlb_u8): Make second argument
            signed.
            (vqrshlh_u16): Likewise.
            (vqrshls_u32): Likewise.
            (vqrshld_u64): Likewise.
            (vqshlb_u8): Likewise.
            (vqshlh_u16): Likewise.
            (vqshls_u32): Likewise.
            (vqshld_u64): Likewise.
            (vshld_u64): Likewise.

    gcc/testsuite/
            PR target/97150
            * gcc.target/aarch64/pr97150.c: New test.

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-09-30
   Target Milestone|---                         |8.5
      Known to fail|                            |10.0, 8.4.1, 9.3.1
           Assignee|unassigned at gcc dot gnu.org      |ktkachov at gcc dot gnu.org
                 CC|                            |ktkachov at gcc dot gnu.org
      Known to work|                            |11.0

--- Comment #4 from ktkachov at gcc dot gnu.org ---
Fixed on trunk. Will backport to the branches later

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Kyrylo Tkachov
<ktkachov@gcc.gnu.org>:

https://gcc.gnu.org/g:11af9c329ae528abb8c69ae8fae96a0f3fc6ed87

commit r10-8869-g11af9c329ae528abb8c69ae8fae96a0f3fc6ed87
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:01:23 2020 +0100

    PR target/97150 AArch64: 2nd parameter of unsigned Neon scalar shift
intrinsics should be signed

    In this PR the second argument to the intrinsics should be signed but we
    use an unsigned one erroneously.
    The corresponding builtins are already using the correct types so it's
    just a matter of correcting the signatures in arm_neon.h

    gcc/
            PR target/97150
            * config/aarch64/arm_neon.h (vqrshlb_u8): Make second argument
            signed.
            (vqrshlh_u16): Likewise.
            (vqrshls_u32): Likewise.
            (vqrshld_u64): Likewise.
            (vqshlb_u8): Likewise.
            (vqshlh_u16): Likewise.
            (vqshls_u32): Likewise.
            (vqshld_u64): Likewise.
            (vshld_u64): Likewise.

    gcc/testsuite/
            PR target/97150
            * gcc.target/aarch64/pr97150.c: New test.

    (cherry picked from commit 2d8fbebdb1eaca8de557ab3052535a8e4b8f8972)

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Kyrylo Tkachov
<ktkachov@gcc.gnu.org>:

https://gcc.gnu.org/g:d5c6ea22fe6db1ee19a178941a8c7f8ff5d0538c

commit r9-8983-gd5c6ea22fe6db1ee19a178941a8c7f8ff5d0538c
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:01:23 2020 +0100

    PR target/97150 AArch64: 2nd parameter of unsigned Neon scalar shift
intrinsics should be signed

    In this PR the second argument to the intrinsics should be signed but we
    use an unsigned one erroneously.
    The corresponding builtins are already using the correct types so it's
    just a matter of correcting the signatures in arm_neon.h

    gcc/
            PR target/97150
            * config/aarch64/arm_neon.h (vqrshlb_u8): Make second argument
            signed.
            (vqrshlh_u16): Likewise.
            (vqrshls_u32): Likewise.
            (vqrshld_u64): Likewise.
            (vqshlb_u8): Likewise.
            (vqshlh_u16): Likewise.
            (vqshls_u32): Likewise.
            (vqshld_u64): Likewise.
            (vshld_u64): Likewise.

    gcc/testsuite/
            PR target/97150
            * gcc.target/aarch64/pr97150.c: New test.

    (cherry picked from commit 2d8fbebdb1eaca8de557ab3052535a8e4b8f8972)
    (cherry picked from commit 11af9c329ae528abb8c69ae8fae96a0f3fc6ed87)

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Kyrylo Tkachov
<ktkachov@gcc.gnu.org>:

https://gcc.gnu.org/g:311183d74e4f3fd5a37749cfbb0960e655e715fb

commit r8-10576-g311183d74e4f3fd5a37749cfbb0960e655e715fb
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:01:23 2020 +0100

    PR target/97150 AArch64: 2nd parameter of unsigned Neon scalar shift
intrinsics should be signed

    In this PR the second argument to the intrinsics should be signed but we
    use an unsigned one erroneously.
    The corresponding builtins are already using the correct types so it's
    just a matter of correcting the signatures in arm_neon.h

    gcc/
            PR target/97150
            * config/aarch64/arm_neon.h (vqrshlb_u8): Make second argument
            signed.
            (vqrshlh_u16): Likewise.
            (vqrshls_u32): Likewise.
            (vqrshld_u64): Likewise.
            (vqshlb_u8): Likewise.
            (vqshlh_u16): Likewise.
            (vqshls_u32): Likewise.
            (vqshld_u64): Likewise.
            (vshld_u64): Likewise.

    gcc/testsuite/
            PR target/97150
            * gcc.target/aarch64/pr97150.c: New test.

    (cherry picked from commit 2d8fbebdb1eaca8de557ab3052535a8e4b8f8972)
    (cherry picked from commit 11af9c329ae528abb8c69ae8fae96a0f3fc6ed87)
    (cherry picked from commit d5c6ea22fe6db1ee19a178941a8c7f8ff5d0538c)

[Bug target/97150] [AArch64] 2nd parameter of unsigned Neon scalar shift intrinsics should be signed

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97150

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from ktkachov at gcc dot gnu.org ---
Fixed on GCC 8.5 onwards.