[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 Stam Markianos-Wright changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #13 from Stam Markianos-Wright --- Fixed on GCC12 onwards.
[Bug target/107515] MVE: Generic functions do not accept _Float16 scalars
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #12 from CVS Commits --- The releases/gcc-12 branch has been updated by Stam Markianos-Wright : https://gcc.gnu.org/g:c7c4dfb5989e80ffc8e8439a8d9a9ed654612b90 commit r12-9608-gc7c4dfb5989e80ffc8e8439a8d9a9ed654612b90 Author: Stam Markianos-Wright Date: Mon Jan 16 11:40:40 2023 + 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #11 from CVS Commits --- The releases/gcc-12 branch has been updated by Stam Markianos-Wright : https://gcc.gnu.org/g:79431d45e2c25dc8608c8ddc6d2798dbcb79650a commit r12-9559-g79431d45e2c25dc8608c8ddc6d2798dbcb79650a Author: Stam Markianos-Wright Date: Thu Nov 10 15:06:47 2022 + 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #10 from CVS Commits --- The master branch has been updated by Stam Markianos-Wright : https://gcc.gnu.org/g:8a1360e72d6c6056606aa5edd8c906c50f26de59 commit r13-5200-g8a1360e72d6c6056606aa5edd8c906c50f26de59 Author: Stam Markianos-Wright Date: Mon Jan 16 11:40:40 2023 + 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #9 from Stam Markianos-Wright --- > 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #8 from Kevin Bracey --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #7 from Stam Markianos-Wright --- (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 > | ^ > :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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #6 from Kevin Bracey --- 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 | ^ :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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #5 from CVS Commits --- The trunk branch has been updated by Andrea Corallo : https://gcc.gnu.org/g:2fefb8931d566cc8a4cbba81601972b0d2002f95 commit r13-4332-g2fefb8931d566cc8a4cbba81601972b0d2002f95 Author: Stam Markianos-Wright Date: Thu Nov 10 15:06:47 2022 + 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #4 from Kevin Bracey --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 Stam Markianos-Wright changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #3 from Stam Markianos-Wright --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515 --- Comment #2 from Kevin Bracey --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107515
Stam Markianos-Wright 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 ---
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).
