public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
@ 2023-05-23  8:06 dev at benjarobin dot fr
  2023-05-23  8:13 ` [Bug target/109939] " pinskia at gcc dot gnu.org
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: dev at benjarobin dot fr @ 2023-05-23  8:06 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 109939
           Summary: Invalid return type for __builtin_arm_ssat: Unsigned
                    instead of signed
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dev at benjarobin dot fr
  Target Milestone: ---

The warning is triggered with the following test case:

```
int dbg_ssat_out;
int dbg_ssat_in;

void test_arm_ssat(void)
{
    dbg_ssat_out = __builtin_arm_ssat(dbg_ssat_in, 16);
}
```

Compile it using this command line:
arm-none-eabi-gcc -mcpu=cortex-m4 -Wall -Wconversion -c t.c -o t.o

The following output is generated:
```
t.c: In function 'test_arm_ssat':
t.c:7:20: warning: conversion to 'int' from 'unsigned int' may change the sign
of the result [-Wsign-conversion]
    7 |     dbg_ssat_out = __builtin_arm_ssat(dbg_ssat_in, 16);
      |                    ^~~~~~~~~~~~~~~~~~
```

This is an issue since SSAT assembly instruction returns a signed value and not
an unsigned integer.

Maybe this line is wrong (I am not an expert about GCC):
https://github.com/gcc-mirror/gcc/blob/releases/gcc-13.1.0/gcc/config/arm/arm-builtins.cc#L100

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
@ 2023-05-23  8:13 ` pinskia at gcc dot gnu.org
  2023-05-23  8:21 ` dev at benjarobin dot fr
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-23  8:13 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-05-23
             Status|UNCONFIRMED                 |WAITING

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Does this warning happens with __ssat from arm_acle.h .

This builtin is not for the user to use directly either.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
  2023-05-23  8:13 ` [Bug target/109939] " pinskia at gcc dot gnu.org
@ 2023-05-23  8:21 ` dev at benjarobin dot fr
  2023-05-23  8:52 ` ktkachov at gcc dot gnu.org
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: dev at benjarobin dot fr @ 2023-05-23  8:21 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Benjamin Robin <dev at benjarobin dot fr> ---
New test case with exactly same problem since __ssat() calls
__builtin_arm_ssat():

```
#include <arm_acle.h>

int dbg_ssat_out;
int dbg_ssat_in;

void test_arm_ssat(void)
{
    dbg_ssat_out = __ssat(dbg_ssat_in, 16);
}
```

Build command:
```
arm-none-eabi-gcc -mcpu=cortex-m4 -Wall -Wconversion -c t2.c -o t2.o
t2.c: In function 'test_arm_ssat':
t2.c:8:20: warning: conversion to 'int32_t' {aka 'long int'} from 'unsigned
int' may change the sign of the result [-Wsign-conversion]
    8 |     dbg_ssat_out = __ssat(dbg_ssat_in, 16);
      |                    ^~~~~~
```

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
  2023-05-23  8:13 ` [Bug target/109939] " pinskia at gcc dot gnu.org
  2023-05-23  8:21 ` dev at benjarobin dot fr
@ 2023-05-23  8:52 ` ktkachov at gcc dot gnu.org
  2023-05-24  8:33 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: ktkachov at gcc dot gnu.org @ 2023-05-23  8:52 UTC (permalink / raw)
  To: gcc-bugs

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

ktkachov at gcc dot gnu.org changed:

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

--- Comment #3 from ktkachov at gcc dot gnu.org ---
I think you're right, the qualifier for the return value of
SAT_BINOP_UNSIGNED_IMM should be qualifier_none

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
                   ` (2 preceding siblings ...)
  2023-05-23  8:52 ` ktkachov at gcc dot gnu.org
@ 2023-05-24  8:33 ` cvs-commit at gcc dot gnu.org
  2023-05-24  8:35 ` ktkachov at gcc dot gnu.org
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-24  8:33 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 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:95542a6ec4b350c653b793b7c36a8210b0e9a89d

commit r14-1156-g95542a6ec4b350c653b793b7c36a8210b0e9a89d
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed May 24 09:33:04 2023 +0100

    arm: PR target/109939 Correct signedness of return type of __ssat
intrinsics

    As the PR says we shouldn't be using qualifier_unsigned for the return type
of the __ssat intrinsics.
    UNSIGNED_SAT_BINOP_UNSIGNED_IMM_QUALIFIERS already exists for that.
    This was just a thinko.
    This patch fixes this and the warning with -Wconversion goes away.

    Bootstrapped and tested on arm-none-linux-gnueabihf.

    gcc/ChangeLog:

            PR target/109939
            * config/arm/arm-builtins.cc (SAT_BINOP_UNSIGNED_IMM_QUALIFIERS):
Use
            qualifier_none for the return operand.

    gcc/testsuite/ChangeLog:

            PR target/109939
            * gcc.target/arm/pr109939.c: New test.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
                   ` (3 preceding siblings ...)
  2023-05-24  8:33 ` cvs-commit at gcc dot gnu.org
@ 2023-05-24  8:35 ` ktkachov at gcc dot gnu.org
  2023-06-08  8:31 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: ktkachov at gcc dot gnu.org @ 2023-05-24  8:35 UTC (permalink / raw)
  To: gcc-bugs

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ktkachov at gcc dot gnu.org

--- Comment #5 from ktkachov at gcc dot gnu.org ---
Fixed for GCC 14. It should be a very low risk patch to backport to the
branches as it fixes an inconsistency with the spec. Will do so after some time
for testing on trunk.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
                   ` (4 preceding siblings ...)
  2023-05-24  8:35 ` ktkachov at gcc dot gnu.org
@ 2023-06-08  8:31 ` cvs-commit at gcc dot gnu.org
  2023-06-08  8:54 ` cvs-commit at gcc dot gnu.org
  2024-06-04 12:49 ` ktkachov at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-08  8:31 UTC (permalink / raw)
  To: gcc-bugs

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

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

https://gcc.gnu.org/g:8f170995aac3fd568652eb208eca62e3937d0cf1

commit r13-7428-g8f170995aac3fd568652eb208eca62e3937d0cf1
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed May 24 09:33:04 2023 +0100

    arm: PR target/109939 Correct signedness of return type of __ssat
intrinsics

    As the PR says we shouldn't be using qualifier_unsigned for the return type
of the __ssat intrinsics.
    UNSIGNED_SAT_BINOP_UNSIGNED_IMM_QUALIFIERS already exists for that.
    This was just a thinko.
    This patch fixes this and the warning with -Wconversion goes away.

    Bootstrapped and tested on arm-none-linux-gnueabihf.

    gcc/ChangeLog:

            PR target/109939
            * config/arm/arm-builtins.cc (SAT_BINOP_UNSIGNED_IMM_QUALIFIERS):
Use
            qualifier_none for the return operand.

    gcc/testsuite/ChangeLog:

            PR target/109939
            * gcc.target/arm/pr109939.c: New test.

    (cherry picked from commit 95542a6ec4b350c653b793b7c36a8210b0e9a89d)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
                   ` (5 preceding siblings ...)
  2023-06-08  8:31 ` cvs-commit at gcc dot gnu.org
@ 2023-06-08  8:54 ` cvs-commit at gcc dot gnu.org
  2024-06-04 12:49 ` ktkachov at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-08  8:54 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

commit r12-9683-ga620451032abb28343c31438a4e779ea5d2e1bbf
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed May 24 09:33:04 2023 +0100

    arm: PR target/109939 Correct signedness of return type of __ssat
intrinsics

    As the PR says we shouldn't be using qualifier_unsigned for the return type
of the __ssat intrinsics.
    UNSIGNED_SAT_BINOP_UNSIGNED_IMM_QUALIFIERS already exists for that.
    This was just a thinko.
    This patch fixes this and the warning with -Wconversion goes away.

    Bootstrapped and tested on arm-none-linux-gnueabihf.

    gcc/ChangeLog:

            PR target/109939
            * config/arm/arm-builtins.cc (SAT_BINOP_UNSIGNED_IMM_QUALIFIERS):
Use
            qualifier_none for the return operand.

    gcc/testsuite/ChangeLog:

            PR target/109939
            * gcc.target/arm/pr109939.c: New test.

    (cherry picked from commit 95542a6ec4b350c653b793b7c36a8210b0e9a89d)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug target/109939] Invalid return type for __builtin_arm_ssat: Unsigned instead of signed
  2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
                   ` (6 preceding siblings ...)
  2023-06-08  8:54 ` cvs-commit at gcc dot gnu.org
@ 2024-06-04 12:49 ` ktkachov at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: ktkachov at gcc dot gnu.org @ 2024-06-04 12:49 UTC (permalink / raw)
  To: gcc-bugs

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

ktkachov at gcc dot gnu.org changed:

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

--- Comment #8 from ktkachov at gcc dot gnu.org ---
This has been fixed some time ago.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-06-04 12:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-23  8:06 [Bug target/109939] New: Invalid return type for __builtin_arm_ssat: Unsigned instead of signed dev at benjarobin dot fr
2023-05-23  8:13 ` [Bug target/109939] " pinskia at gcc dot gnu.org
2023-05-23  8:21 ` dev at benjarobin dot fr
2023-05-23  8:52 ` ktkachov at gcc dot gnu.org
2023-05-24  8:33 ` cvs-commit at gcc dot gnu.org
2023-05-24  8:35 ` ktkachov at gcc dot gnu.org
2023-06-08  8:31 ` cvs-commit at gcc dot gnu.org
2023-06-08  8:54 ` cvs-commit at gcc dot gnu.org
2024-06-04 12:49 ` ktkachov at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).