[Bug target/96313] New: [AArch64] vqmovun* return types should be unsigned

[Bug target/96313] New: [AArch64] vqmovun* return types should be unsigned

[evan@coeus-group.com]

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

            Bug ID: 96313
           Summary: [AArch64] vqmovun* return types should be unsigned
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: evan@coeus-group.com
  Target Milestone: ---

GCC has signed return values for the vqmovun* functions, but they should be
unsigned.  See
https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqmovun

Trivial test case:

  #include <arm_neon.h>

  uint16_t foo(int32_t v) {
    return vqmovuns_s32(v);
  }

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[rearnsha at gcc dot gnu.org]

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

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2020-07-27
     Ever confirmed|0                           |1

--- Comment #1 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
In what way, exactly, do you think the output is incorrect?  Don't forget that
your testcase returns a uint16_t and the ABI says that it is the caller's
responsibility to do any widening, so any bits beyond bit 15 in the result
register are simply unspecified.

Secondly, you haven't stated which version of gcc you were using, or how it was
configured.

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[jgreenhalgh at gcc dot gnu.org]

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

James Greenhalgh <jgreenhalgh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jgreenhalgh at gcc dot gnu.org
             Status|WAITING                     |NEW

--- Comment #2 from James Greenhalgh <jgreenhalgh at gcc dot gnu.org> ---
Confirmed by inspection; types in arm_neon.h are:

  int8_t vqmovunh_s16 (int16_t __a)
  int16_t vqmovuns_s32 (int32_t __a)
  int32_t vqmovund_s64 (int64_t __a)

Types in the documentation are:

  uint8_t vqmovunh_s16 (int16_t a)
  uint16_t vqmovuns_s32 (int32_t a)
  uint32_t vqmovund_s64 (int64_t a)

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[david.spickett at linaro dot org]

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

David Spickett <david.spickett at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |david.spickett at linaro dot org

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

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[david.spickett at linaro dot org]

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

--- Comment #4 from David Spickett <david.spickett at linaro dot org> ---
I've been fixing this for clang so here is a test file in case it's any help.

gcc version 11.0.0 20200609 (experimental)
Ubuntu 16.04.7 LTS running on x86_64

$ ./aarch64-none-elf-g++ -v
Using built-in specs.
COLLECT_GCC=./aarch64-none-elf-g++
COLLECT_LTO_WRAPPER=/arm/pdtl/builds/fsf-trunk.2226/installed/rhe6x86_64/aarch64-none-elf/bin/../libexec/gcc/aarch64-none-elf/11.0.0/lto-wrapper
Target: aarch64-none-elf
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
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 11.0.0 20200609 (experimental) (fsf-trunk.2226)

/tmp/test.cpp:
#include <arm_neon.h>

uint32_t (*fp3)(int64_t) = vqmovund_s64;
uint8_t (*fp4)(int16_t) = vqmovunh_s16;
uint16_t (*fp5)(int32_t) = vqmovuns_s32;

Compile with:
$ ./aarch64-none-elf-g++ -march=armv8.1-a+simd -Wall -Wextra -c -o /tmp/foo
/tmp/test.cpp -save-temps

Output:
/tmp/test.cpp:3:28: error: invalid conversion from 'int32_t (*)(int64_t)' {aka
'int (*)(long int)'} to 'uint32_t (*)(int64_t)' {aka 'unsigned int (*)(long
int)'} [-fpermissive]
    3 | uint32_t (*fp3)(int64_t) = vqmovund_s64;
      |                            ^~~~~~~~~~~~
      |                            |
      |                            int32_t (*)(int64_t) {aka int (*)(long int)}
/tmp/test.cpp:4:27: error: invalid conversion from 'int8_t (*)(int16_t)' {aka
'signed char (*)(short int)'} to 'uint8_t (*)(int16_t)' {aka 'unsigned char
(*)(short int)'} [-fpermissive]
    4 | uint8_t (*fp4)(int16_t) = vqmovunh_s16;
      |                           ^~~~~~~~~~~~
      |                           |
      |                           int8_t (*)(int16_t) {aka signed char
(*)(short int)}
/tmp/test.cpp:5:28: error: invalid conversion from 'int16_t (*)(int32_t)' {aka
'short int (*)(int)'} to 'uint16_t (*)(int32_t)' {aka 'short unsigned int
(*)(int)'} [-fpermissive]
    5 | uint16_t (*fp5)(int32_t) = vqmovuns_s32;
      |                            ^~~~~~~~~~~~
      |                            |
      |                            int16_t (*)(int32_t) {aka short int
(*)(int)}

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:135b043196b5575c690ef1e07bcbb49bf037c3a2

commit r11-3547-g135b043196b5575c690ef1e07bcbb49bf037c3a2
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:00:20 2020 +0100

    PR target/96313 AArch64: vqmovun* return types should be unsigned

    In this PR we have the wrong return type for some intrinsics. It should
    be unsigned, but we implement it as signed.
    Fix this by adjusting the type qualifiers used when creating the
    builtins and fixing the type in the arm_neon.h intrinsic.
    With the adjustment in qualifiers we now don't need to cast the result
    when returning.

    Bootstrapped and tested on aarch64-none-linux-gnu.

    gcc/
            PR target/96313
            * config/aarch64/aarch64-simd-builtins.def (sqmovun): Use UNOPUS
            qualifiers.
            * config/aarch64/arm_neon.h (vqmovun_s16): Adjust builtin call.
            Remove unnecessary result cast.
            (vqmovun_s32): Likewise.
            (vqmovun_s64): Likewise.
            (vqmovunh_s16): Likewise.  Fix return type.
            (vqmovuns_s32): Likewise.
            (vqmovund_s64): Likewise.

    gcc/testsuite/
            PR target/96313
            * gcc.target/aarch64/pr96313.c: New test.
            * gcc.target/aarch64/scalar_intrinsics.c (test_vqmovunh_s16):
            Adjust return type.
            (test_vqmovuns_s32): Likewise.
            (test_vqmovund_s64): Likewise.

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |ktkachov at gcc dot gnu.org
             Status|NEW                         |ASSIGNED
                 CC|                            |ktkachov at gcc dot gnu.org
      Known to work|                            |11.0
   Target Milestone|---                         |8.5
      Known to fail|                            |10.0, 8.4.1, 9.3.1

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

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:7d177b142dcbe841124fe023cb592914d4200e57

commit r10-8868-g7d177b142dcbe841124fe023cb592914d4200e57
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:00:20 2020 +0100

    PR target/96313 AArch64: vqmovun* return types should be unsigned

    In this PR we have the wrong return type for some intrinsics. It should
    be unsigned, but we implement it as signed.
    Fix this by adjusting the type qualifiers used when creating the
    builtins and fixing the type in the arm_neon.h intrinsic.
    With the adjustment in qualifiers we now don't need to cast the result
    when returning.

    Bootstrapped and tested on aarch64-none-linux-gnu.

    gcc/
            PR target/96313
            * config/aarch64/aarch64-simd-builtins.def (sqmovun): Use UNOPUS
            qualifiers.
            * config/aarch64/arm_neon.h (vqmovun_s16): Adjust builtin call.
            Remove unnecessary result cast.
            (vqmovun_s32): Likewise.
            (vqmovun_s64): Likewise.
            (vqmovunh_s16): Likewise.  Fix return type.
            (vqmovuns_s32): Likewise.
            (vqmovund_s64): Likewise.

    gcc/testsuite/
            PR target/96313
            * gcc.target/aarch64/pr96313.c: New test.
            * gcc.target/aarch64/scalar_intrinsics.c (test_vqmovunh_s16):
            Adjust return type.
            (test_vqmovuns_s32): Likewise.
            (test_vqmovund_s64): Likewise.

    (cherry picked from commit 135b043196b5575c690ef1e07bcbb49bf037c3a2)

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 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:18d980d94f8d7187ce30bf23ddd365fa54189c36

commit r9-8982-g18d980d94f8d7187ce30bf23ddd365fa54189c36
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:00:20 2020 +0100

    PR target/96313 AArch64: vqmovun* return types should be unsigned

    In this PR we have the wrong return type for some intrinsics. It should
    be unsigned, but we implement it as signed.
    Fix this by adjusting the type qualifiers used when creating the
    builtins and fixing the type in the arm_neon.h intrinsic.
    With the adjustment in qualifiers we now don't need to cast the result
    when returning.

    Bootstrapped and tested on aarch64-none-linux-gnu.

    gcc/
            PR target/96313
            * config/aarch64/aarch64-simd-builtins.def (sqmovun): Use UNOPUS
            qualifiers.
            * config/aarch64/arm_neon.h (vqmovun_s16): Adjust builtin call.
            Remove unnecessary result cast.
            (vqmovun_s32): Likewise.
            (vqmovun_s64): Likewise.
            (vqmovunh_s16): Likewise.  Fix return type.
            (vqmovuns_s32): Likewise.
            (vqmovund_s64): Likewise.

    gcc/testsuite/
            PR target/96313
            * gcc.target/aarch64/pr96313.c: New test.
            * gcc.target/aarch64/scalar_intrinsics.c (test_vqmovunh_s16):
            Adjust return type.
            (test_vqmovuns_s32): Likewise.
            (test_vqmovund_s64): Likewise.

    (cherry picked from commit 135b043196b5575c690ef1e07bcbb49bf037c3a2)
    (cherry picked from commit 7d177b142dcbe841124fe023cb592914d4200e57)

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 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:7f8115b305f1a1a2ddec4f59bc08a3415359dda6

commit r8-10575-g7f8115b305f1a1a2ddec4f59bc08a3415359dda6
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Sep 30 12:00:20 2020 +0100

    PR target/96313 AArch64: vqmovun* return types should be unsigned

    In this PR we have the wrong return type for some intrinsics. It should
    be unsigned, but we implement it as signed.
    Fix this by adjusting the type qualifiers used when creating the
    builtins and fixing the type in the arm_neon.h intrinsic.
    With the adjustment in qualifiers we now don't need to cast the result
    when returning.

    Bootstrapped and tested on aarch64-none-linux-gnu.

    gcc/
            PR target/96313
            * config/aarch64/aarch64-simd-builtins.def (sqmovun): Use UNOPUS
            qualifiers.
            * config/aarch64/arm_neon.h (vqmovun_s16): Adjust builtin call.
            Remove unnecessary result cast.
            (vqmovun_s32): Likewise.
            (vqmovun_s64): Likewise.
            (vqmovunh_s16): Likewise.  Fix return type.
            (vqmovuns_s32): Likewise.
            (vqmovund_s64): Likewise.

    gcc/testsuite/
            PR target/96313
            * gcc.target/aarch64/pr96313.c: New test.
            * gcc.target/aarch64/scalar_intrinsics.c (test_vqmovunh_s16):
            Adjust return type.
            (test_vqmovuns_s32): Likewise.
            (test_vqmovund_s64): Likewise.

    (cherry picked from commit 135b043196b5575c690ef1e07bcbb49bf037c3a2)
    (cherry picked from commit 7d177b142dcbe841124fe023cb592914d4200e57)
    (cherry picked from commit 18d980d94f8d7187ce30bf23ddd365fa54189c36)

[Bug target/96313] [AArch64] vqmovun* return types should be unsigned

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

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

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