[Bug target/107515] New: MVE: Generic functions do not accept _Float16 scalars

[Bug target/107515] New: MVE: Generic functions do not accept _Float16 scalars

[kevin.bracey at alifsemi dot com]

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

            Bug ID: 107515
           Summary: MVE: Generic functions do not accept _Float16 scalars
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kevin.bracey at alifsemi dot com
  Target Milestone: ---

Compiling C code, generic functions taking floating point scalars in arm_mve.h
do not accept `_Float16` values.

// Using gcc -mcpu=cortex-m55 -O2
// Uploaded at https://godbolt.org/z/7jrqWWroY

#include <arm_mve.h>

void test(void)
{
    float16x8_t x;

    x = vmulq(x, 0.5); // ok
    x = vmulq(x, 0.5f); // ok
    x = vmulq(x, (__fp16) 0.5); // ok
    x = vmulq(x, 0.15f16); // rejected
    x = vmulq(x, (_Float16) 0.15); // rejected
}

Output:

<source>:10:9: error: '_Generic' selector of type 'int (*)[4][39]' is not
compatible with any association
   10 |     x = vmulq(x, 0.15f16); // rejected
      |         ^~~~~
<source>:11:9: error: '_Generic' selector of type 'int (*)[4][39]' is not
compatible with any association
   11 |     x = vmulq(x, (_Float16) 0.15); // rejected
      |         ^~~~~

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[stammark at gcc dot gnu.org]

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

Stam Markianos-Wright <stammark at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |stammark at gcc dot gnu.org
                 CC|                            |stammark at gcc dot gnu.org
   Last reconfirmed|                            |2022-11-10
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
Also confirmed on trunk and assigning to myself, because I believe I've found a
fix: 
```
diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
index 2d213e12304..ce20a6fcd24 100644
--- a/gcc/config/arm/arm_mve.h
+++ b/gcc/config/arm/arm_mve.h
@@ -35582,6 +35582,9 @@ enum {
        short: __ARM_mve_type_int_n, \
        int: __ARM_mve_type_int_n, \
        long: __ARM_mve_type_int_n, \
+       _Float16: __ARM_mve_type_fp_n, \
+       __fp16: __ARM_mve_type_fp_n, \
+       float: __ARM_mve_type_fp_n, \
        double: __ARM_mve_type_fp_n, \
        long long: __ARM_mve_type_int_n, \
        unsigned char: __ARM_mve_type_int_n, \
```

Still untested, though, and I'll likely send it to the mailing list within the
next week or two (we've got a couple more arm_mve.h changes in the pipeline, so
I'll test them all together).

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[kevin.bracey at alifsemi dot com]

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

--- Comment #2 from Kevin Bracey <kevin.bracey at alifsemi dot com> ---
I've just spotted another apparent generic selection problem in my reproducer
for  bug 107714 - should I create a new issue for it?

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[stammark at gcc dot gnu.org]

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

Stam Markianos-Wright <stammark at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #3 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
(In reply to Kevin Bracey from comment #2)
> I've just spotted another apparent generic selection problem in my
> reproducer for  bug 107714 - should I create a new issue for it?

Our patch series has gone up to the mailing list! For _Float16 see:

https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606587.html

For `vmuq` this:

https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606575.html

works on my side, so if it gets merged and works for your case, too, then I
guess we don't have to worry about a new bug report :)

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[kevin.bracey at alifsemi dot com]

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

--- Comment #4 from Kevin Bracey <kevin.bracey at alifsemi dot com> ---
Yes, looking at them it seems clear those patches address what I'm seeing with
the `vmulq(x, 6)` issue.

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrea Corallo <akrl@gcc.gnu.org>:

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

commit r13-4332-g2fefb8931d566cc8a4cbba81601972b0d2002f95
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Thu Nov 10 15:06:47 2022 +0000

    arm: Explicitly specify other float types for _Generic overloading
[PR107515]

    This patch adds explicit references to other float types
    to __ARM_mve_typeid in arm_mve.h.  Resolves PR 107515:
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515

    gcc/ChangeLog:
            PR target/107515
            * config/arm/arm_mve.h (__ARM_mve_typeid): Add float types.

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[kevin.bracey at alifsemi dot com]

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

--- Comment #6 from Kevin Bracey <kevin.bracey at alifsemi dot com> ---
Retesting the Godbolt on trunk, it's now worse - every line produces multiple
not-very-informative errors:

source>:7:9: error: '_Generic' specifies two compatible types
    7 |     x = vmulq(x, 0.5); // ok
      |         ^~~~~
<source>:7:9: note: compatible type is here
    7 |     x = vmulq(x, 0.5); // ok
      |         ^~~~~

(repeated 6 times per source line)

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[stammark at gcc dot gnu.org]

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

--- Comment #7 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
(In reply to Kevin Bracey from comment #6)
> Retesting the Godbolt on trunk, it's now worse - every line produces
> multiple not-very-informative errors:
> 
> source>:7:9: error: '_Generic' specifies two compatible types
>     7 |     x = vmulq(x, 0.5); // ok
>       |         ^~~~~
> <source>:7:9: note: compatible type is here
>     7 |     x = vmulq(x, 0.5); // ok
>       |         ^~~~~
> 
> (repeated 6 times per source line)

Interesting... Thanks for spotting this, I didn't see this in my testing,
because it doesn't seem to happen on baremetal `arm-none-eabi` (and I still
can't replicate it there), but I do see this on the linux target (let me know
if you are seeing anything different). I am investigating further!

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[kevin.bracey at alifsemi dot com]

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

--- Comment #8 from Kevin Bracey <kevin.bracey at alifsemi dot com> ---
I'm only testing on the Linux trunk because it's what Godbolt has. If it has
bare-metal, I'm not seeing it.

Actual real development system is bare-metal using Arm's embedded GCC releases,
and I don't have a set-up to test a trunk GCC build on it at the moment.

Clearly Helium+Linux on Godbolt is a bit confused because it's always using
non-existent registers Q8 upwards. There may be a fundamental config error
leading to all sorts of strange results.

(Mostly reproduces my bare-metal findings though.)

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[stammark at gcc dot gnu.org]

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

--- Comment #9 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
> Clearly Helium+Linux on Godbolt is a bit confused 

Yea, I agree -- it still shouldn't be an unintuitive front-end type clash
error, though!

I've posted another patch:
https://gcc.gnu.org/pipermail/gcc-patches/2022-December/607675.html (see there
for what the error was --- interestingly it was coming from `__ARM_mve_coerce3`
and it didn't directly have anything to do with the float types)

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Stam Markianos-Wright
<stammark@gcc.gnu.org>:

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

commit r13-5200-g8a1360e72d6c6056606aa5edd8c906c50f26de59
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Mon Jan 16 11:40:40 2023 +0000

    arm: Split up MVE _Generic associations to prevent type clashes [PR107515]

    With these previous patches:
    https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606586.html
    https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606587.html
    we enabled the MVE overloaded _Generic associations to handle more
    scalar types, however at PR 107515 we found a new regression that
    wasn't detected in our testing:

    With glibc's posix/types.h:
    ```
    typedef signed int __int32_t;
    ...
    typedef __int32_t int32_t;
    ```
    We would get a `error: '_Generic' specifies two compatible types`
    from `__ARM_mve_coerce3` because of `type: param`, when `type` is
    `int` and `int32_t: param` both being the same under the hood.

    The same did not happen with Newlib's header sys/_stdint.h:
    ```
    typedef long int __int32_t;
    ...
    typedef __int32_t int32_t ;
    ```
    which worked fine, because it uses `long int`.

    The same could feasibly happen in `__ARM_mve_coerce2` between
    `__fp16` and `float16_t`.

    The solution here is to break the _Generic down so that the similar
    types don't appear at the same level, as is done in `__ARM_mve_typeid`

    gcc/ChangeLog:
            PR target/96795
            PR target/107515
            * config/arm/arm_mve.h (__ARM_mve_coerce2): Split types.
            (__ARM_mve_coerce3): Likewise.

    gcc/testsuite/ChangeLog:
            PR target/96795
            PR target/107515
            * gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c:
New test.
            * gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c:
New test.

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Stam Markianos-Wright
<stammark@gcc.gnu.org>:

https://gcc.gnu.org/g:79431d45e2c25dc8608c8ddc6d2798dbcb79650a

commit r12-9559-g79431d45e2c25dc8608c8ddc6d2798dbcb79650a
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Thu Nov 10 15:06:47 2022 +0000

    arm: Explicitly specify other float types for _Generic overloading
[PR107515]

    This patch adds explicit references to other float types
    to __ARM_mve_typeid in arm_mve.h.  Resolves PR 107515:
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515

    gcc/ChangeLog:
            PR target/107515
            * config/arm/arm_mve.h (__ARM_mve_typeid): Add float types.

    (cherry picked from commit 2fefb8931d566cc8a4cbba81601972b0d2002f95)

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Stam Markianos-Wright
<stammark@gcc.gnu.org>:

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

commit r12-9608-gc7c4dfb5989e80ffc8e8439a8d9a9ed654612b90
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Mon Jan 16 11:40:40 2023 +0000

    arm: Split up MVE _Generic associations to prevent type clashes [PR107515]

    With these previous patches:
    https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606586.html
    https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606587.html
    we enabled the MVE overloaded _Generic associations to handle more
    scalar types, however at PR 107515 we found a new regression that
    wasn't detected in our testing:

    With glibc's posix/types.h:
    ```
    typedef signed int __int32_t;
    ...
    typedef __int32_t int32_t;
    ```
    We would get a `error: '_Generic' specifies two compatible types`
    from `__ARM_mve_coerce3` because of `type: param`, when `type` is
    `int` and `int32_t: param` both being the same under the hood.

    The same did not happen with Newlib's header sys/_stdint.h:
    ```
    typedef long int __int32_t;
    ...
    typedef __int32_t int32_t ;
    ```
    which worked fine, because it uses `long int`.

    The same could feasibly happen in `__ARM_mve_coerce2` between
    `__fp16` and `float16_t`.

    The solution here is to break the _Generic down so that the similar
    types don't appear at the same level, as is done in `__ARM_mve_typeid`

    gcc/ChangeLog:
            PR target/96795
            PR target/107515
            * config/arm/arm_mve.h (__ARM_mve_coerce2): Split types.
            (__ARM_mve_coerce3): Likewise.

    gcc/testsuite/ChangeLog:
            PR target/96795
            PR target/107515
            * gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c:
New test.
            * gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c:
New test.

    (cherry picked from commit 8a1360e72d6c6056606aa5edd8c906c50f26de59)

[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars

[stammark at gcc dot gnu.org]

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

Stam Markianos-Wright <stammark at gcc dot gnu.org> changed:

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

--- Comment #13 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
Fixed on GCC12 onwards.