public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] arm: Split up MVE _Generic associations to prevent type clashes [PR107515]
@ 2022-12-01 18:19 Stam Markianos-Wright
  2023-01-10 14:07 ` [PING][PATCH] " Stam Markianos-Wright
  2023-01-13 16:47 ` [PATCH] " Richard Earnshaw
  0 siblings, 2 replies; 3+ messages in thread
From: Stam Markianos-Wright @ 2022-12-01 18:19 UTC (permalink / raw)
  To: gcc-patches; +Cc: Kyrylo Tkachov

[-- Attachment #1: Type: text/plain, Size: 6750 bytes --]

Hi all,

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`.

Ok for trunk?

Thanks,
Stam Markianos-Wright

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.


=========== Inline Ctrl+C, Ctrl+V or patch ===========

diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
index 
09167ec118ed3310c5077145e119196f29d83cac..70003653db65736fcfd019e83d9f18153be650dc 
100644
--- a/gcc/config/arm/arm_mve.h
+++ b/gcc/config/arm/arm_mve.h
@@ -35659,9 +35659,9 @@ extern void *__ARM_undef;
  #define __ARM_mve_coerce1(param, type) \
      _Generic(param, type: param, const type: param, default: *(type 
*)__ARM_undef)
  #define __ARM_mve_coerce2(param, type) \
-    _Generic(param, type: param, float16_t: param, float32_t: param, 
default: *(type *)__ARM_undef)
+    _Generic(param, type: param, __fp16: param, default: _Generic 
(param, _Float16: param, float16_t: param, float32_t: param, default: 
*(type *)__ARM_undef))
  #define __ARM_mve_coerce3(param, type) \
-    _Generic(param, type: param, int8_t: param, int16_t: param, 
int32_t: param, int64_t: param, uint8_t: param, uint16_t: param, 
uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef)
+    _Generic(param, type: param, default: _Generic (param, int8_t: 
param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param, 
uint16_t: param, uint32_t: param, uint64_t: param, default: *(type 
*)__ARM_undef))

  #if (__ARM_FEATURE_MVE & 2) /* MVE Floating point.  */

diff --git 
a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c 
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
new file mode 100644
index 
0000000000000000000000000000000000000000..427dcacb5ff59b53d5eab1f1582ef6460da3f2f3
--- /dev/null
+++ 
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
@@ -0,0 +1,65 @@
+/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
+/* { dg-add-options arm_v8_1m_mve_fp } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+#include "arm_mve.h"
+
+float f1;
+double f2;
+float16_t f3;
+float32_t f4;
+__fp16 f5;
+_Float16 f6;
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+float16x8_t floatvec;
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+
+    /* Test a few different supported ways of passing a float value.  */
+    floatvec = vmulq(floatvec, 0.5);
+    floatvec = vmulq(floatvec, 0.5f);
+    floatvec = vmulq(floatvec, (__fp16) 0.5);
+    floatvec = vmulq(floatvec, f1);
+    floatvec = vmulq(floatvec, f2);
+    floatvec = vmulq(floatvec, f3);
+    floatvec = vmulq(floatvec, f4);
+    floatvec = vmulq(floatvec, f5);
+    floatvec = vmulq(floatvec, f6);
+    floatvec = vmulq(floatvec, 0.15f16);
+    floatvec = vmulq(floatvec, (_Float16) 0.15);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file
diff --git 
a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c 
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
new file mode 100644
index 
0000000000000000000000000000000000000000..d0e3b3eb30f46cb8327e7b976713990721305c9b
--- /dev/null
+++ 
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
@@ -0,0 +1,45 @@
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+
+#include "arm_mve.h"
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file

[-- Attachment #2: rb16663.patch --]
[-- Type: text/x-patch, Size: 4954 bytes --]

diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
index 09167ec118ed3310c5077145e119196f29d83cac..70003653db65736fcfd019e83d9f18153be650dc 100644
--- a/gcc/config/arm/arm_mve.h
+++ b/gcc/config/arm/arm_mve.h
@@ -35659,9 +35659,9 @@ extern void *__ARM_undef;
 #define __ARM_mve_coerce1(param, type) \
     _Generic(param, type: param, const type: param, default: *(type *)__ARM_undef)
 #define __ARM_mve_coerce2(param, type) \
-    _Generic(param, type: param, float16_t: param, float32_t: param, default: *(type *)__ARM_undef)
+    _Generic(param, type: param, __fp16: param, default: _Generic (param, _Float16: param, float16_t: param, float32_t: param, default: *(type *)__ARM_undef))
 #define __ARM_mve_coerce3(param, type) \
-    _Generic(param, type: param, int8_t: param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param, uint16_t: param, uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef)
+    _Generic(param, type: param, default: _Generic (param, int8_t: param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param, uint16_t: param, uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef))
 
 #if (__ARM_FEATURE_MVE & 2) /* MVE Floating point.  */
 
diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
new file mode 100644
index 0000000000000000000000000000000000000000..427dcacb5ff59b53d5eab1f1582ef6460da3f2f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
@@ -0,0 +1,65 @@
+/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
+/* { dg-add-options arm_v8_1m_mve_fp } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+#include "arm_mve.h"
+
+float f1;
+double f2;
+float16_t f3;
+float32_t f4;
+__fp16 f5;
+_Float16 f6;
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+float16x8_t floatvec;
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+
+    /* Test a few different supported ways of passing a float value.  */
+    floatvec = vmulq(floatvec, 0.5);
+    floatvec = vmulq(floatvec, 0.5f);
+    floatvec = vmulq(floatvec, (__fp16) 0.5);
+    floatvec = vmulq(floatvec, f1);
+    floatvec = vmulq(floatvec, f2);
+    floatvec = vmulq(floatvec, f3);
+    floatvec = vmulq(floatvec, f4);
+    floatvec = vmulq(floatvec, f5);
+    floatvec = vmulq(floatvec, f6);
+    floatvec = vmulq(floatvec, 0.15f16);
+    floatvec = vmulq(floatvec, (_Float16) 0.15);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
new file mode 100644
index 0000000000000000000000000000000000000000..d0e3b3eb30f46cb8327e7b976713990721305c9b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
@@ -0,0 +1,45 @@
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+
+#include "arm_mve.h"
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file

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

* [PING][PATCH] arm: Split up MVE _Generic associations to prevent type clashes [PR107515]
  2022-12-01 18:19 [PATCH] arm: Split up MVE _Generic associations to prevent type clashes [PR107515] Stam Markianos-Wright
@ 2023-01-10 14:07 ` Stam Markianos-Wright
  2023-01-13 16:47 ` [PATCH] " Richard Earnshaw
  1 sibling, 0 replies; 3+ messages in thread
From: Stam Markianos-Wright @ 2023-01-10 14:07 UTC (permalink / raw)
  To: gcc-patches; +Cc: Kyrylo Tkachov, Richard Earnshaw


[-- Attachment #1.1: Type: text/plain, Size: 6743 bytes --]


Hi all,

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`.

Ok for trunk?

Thanks,
Stam Markianos-Wright

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.


=========== Inline Ctrl+C, Ctrl+V or patch ===========

diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
index
09167ec118ed3310c5077145e119196f29d83cac..70003653db65736fcfd019e83d9f18153be650dc
100644
--- a/gcc/config/arm/arm_mve.h
+++ b/gcc/config/arm/arm_mve.h
@@ -35659,9 +35659,9 @@ extern void *__ARM_undef;
  #define __ARM_mve_coerce1(param, type) \
      _Generic(param, type: param, const type: param, default: *(type
*)__ARM_undef)
  #define __ARM_mve_coerce2(param, type) \
-    _Generic(param, type: param, float16_t: param, float32_t: param,
default: *(type *)__ARM_undef)
+    _Generic(param, type: param, __fp16: param, default: _Generic
(param, _Float16: param, float16_t: param, float32_t: param, default:
*(type *)__ARM_undef))
  #define __ARM_mve_coerce3(param, type) \
-    _Generic(param, type: param, int8_t: param, int16_t: param,
int32_t: param, int64_t: param, uint8_t: param, uint16_t: param,
uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef)
+    _Generic(param, type: param, default: _Generic (param, int8_t:
param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param,
uint16_t: param, uint32_t: param, uint64_t: param, default: *(type
*)__ARM_undef))

  #if (__ARM_FEATURE_MVE & 2) /* MVE Floating point.  */

diff --git
a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
new file mode 100644
index
0000000000000000000000000000000000000000..427dcacb5ff59b53d5eab1f1582ef6460da3f2f3
--- /dev/null
+++
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
@@ -0,0 +1,65 @@
+/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
+/* { dg-add-options arm_v8_1m_mve_fp } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+#include "arm_mve.h"
+
+float f1;
+double f2;
+float16_t f3;
+float32_t f4;
+__fp16 f5;
+_Float16 f6;
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+float16x8_t floatvec;
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+
+    /* Test a few different supported ways of passing a float value.  */
+    floatvec = vmulq(floatvec, 0.5);
+    floatvec = vmulq(floatvec, 0.5f);
+    floatvec = vmulq(floatvec, (__fp16) 0.5);
+    floatvec = vmulq(floatvec, f1);
+    floatvec = vmulq(floatvec, f2);
+    floatvec = vmulq(floatvec, f3);
+    floatvec = vmulq(floatvec, f4);
+    floatvec = vmulq(floatvec, f5);
+    floatvec = vmulq(floatvec, f6);
+    floatvec = vmulq(floatvec, 0.15f16);
+    floatvec = vmulq(floatvec, (_Float16) 0.15);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file
diff --git
a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
new file mode 100644
index
0000000000000000000000000000000000000000..d0e3b3eb30f46cb8327e7b976713990721305c9b
--- /dev/null
+++
b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
@@ -0,0 +1,45 @@
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+
+#include "arm_mve.h"
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rb16663.patch --]
[-- Type: text/x-patch; name="rb16663.patch", Size: 4954 bytes --]

diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
index 09167ec118ed3310c5077145e119196f29d83cac..70003653db65736fcfd019e83d9f18153be650dc 100644
--- a/gcc/config/arm/arm_mve.h
+++ b/gcc/config/arm/arm_mve.h
@@ -35659,9 +35659,9 @@ extern void *__ARM_undef;
 #define __ARM_mve_coerce1(param, type) \
     _Generic(param, type: param, const type: param, default: *(type *)__ARM_undef)
 #define __ARM_mve_coerce2(param, type) \
-    _Generic(param, type: param, float16_t: param, float32_t: param, default: *(type *)__ARM_undef)
+    _Generic(param, type: param, __fp16: param, default: _Generic (param, _Float16: param, float16_t: param, float32_t: param, default: *(type *)__ARM_undef))
 #define __ARM_mve_coerce3(param, type) \
-    _Generic(param, type: param, int8_t: param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param, uint16_t: param, uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef)
+    _Generic(param, type: param, default: _Generic (param, int8_t: param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param, uint16_t: param, uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef))
 
 #if (__ARM_FEATURE_MVE & 2) /* MVE Floating point.  */
 
diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
new file mode 100644
index 0000000000000000000000000000000000000000..427dcacb5ff59b53d5eab1f1582ef6460da3f2f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
@@ -0,0 +1,65 @@
+/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
+/* { dg-add-options arm_v8_1m_mve_fp } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+#include "arm_mve.h"
+
+float f1;
+double f2;
+float16_t f3;
+float32_t f4;
+__fp16 f5;
+_Float16 f6;
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+float16x8_t floatvec;
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+
+    /* Test a few different supported ways of passing a float value.  */
+    floatvec = vmulq(floatvec, 0.5);
+    floatvec = vmulq(floatvec, 0.5f);
+    floatvec = vmulq(floatvec, (__fp16) 0.5);
+    floatvec = vmulq(floatvec, f1);
+    floatvec = vmulq(floatvec, f2);
+    floatvec = vmulq(floatvec, f3);
+    floatvec = vmulq(floatvec, f4);
+    floatvec = vmulq(floatvec, f5);
+    floatvec = vmulq(floatvec, f6);
+    floatvec = vmulq(floatvec, 0.15f16);
+    floatvec = vmulq(floatvec, (_Float16) 0.15);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
new file mode 100644
index 0000000000000000000000000000000000000000..d0e3b3eb30f46cb8327e7b976713990721305c9b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
@@ -0,0 +1,45 @@
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
+
+#include "arm_mve.h"
+
+int i1;
+short i2;
+long i3;
+long long i4;
+int8_t i5;
+int16_t i6;
+int32_t i7;
+int64_t i8;
+
+const int ci1;
+const short ci2;
+const long ci3;
+const long long ci4;
+const int8_t ci5;
+const int16_t ci6;
+const int32_t ci7;
+const int64_t ci8;
+
+int16x8_t intvec;
+
+void test(void)
+{
+    /* Test a few different supported ways of passing an int value.  The
+    intrinsic vmulq was chosen arbitrarily, but it is representative of
+    all intrinsics that take a non-const scalar value.  */
+    intvec = vmulq(intvec, 2);
+    intvec = vmulq(intvec, (int32_t) 2);
+    intvec = vmulq(intvec, (short) 2);
+    intvec = vmulq(intvec, i1);
+    intvec = vmulq(intvec, i2);
+    intvec = vmulq(intvec, i3);
+    intvec = vmulq(intvec, i4);
+    intvec = vmulq(intvec, i5);
+    intvec = vmulq(intvec, i6);
+    intvec = vmulq(intvec, i7);
+    intvec = vmulq(intvec, i8);
+}
+
+/* { dg-final { scan-assembler-not "__ARM_undef" } } */
\ No newline at end of file

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

* Re: [PATCH] arm: Split up MVE _Generic associations to prevent type clashes [PR107515]
  2022-12-01 18:19 [PATCH] arm: Split up MVE _Generic associations to prevent type clashes [PR107515] Stam Markianos-Wright
  2023-01-10 14:07 ` [PING][PATCH] " Stam Markianos-Wright
@ 2023-01-13 16:47 ` Richard Earnshaw
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Earnshaw @ 2023-01-13 16:47 UTC (permalink / raw)
  To: Stam Markianos-Wright, gcc-patches; +Cc: Kyrylo Tkachov



On 01/12/2022 18:19, Stam Markianos-Wright via Gcc-patches wrote:
> Hi all,
> 
> 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`.
> 
> Ok for trunk?
> 
> Thanks,
> Stam Markianos-Wright
> 
> 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.

Please fix the missing new lines at the end of the tests.

Otherwise OK.

R.

> 
> 
> =========== Inline Ctrl+C, Ctrl+V or patch ===========
> 
> diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
> index 
> 09167ec118ed3310c5077145e119196f29d83cac..70003653db65736fcfd019e83d9f18153be650dc 100644
> --- a/gcc/config/arm/arm_mve.h
> +++ b/gcc/config/arm/arm_mve.h
> @@ -35659,9 +35659,9 @@ extern void *__ARM_undef;
>   #define __ARM_mve_coerce1(param, type) \
>       _Generic(param, type: param, const type: param, default: *(type 
> *)__ARM_undef)
>   #define __ARM_mve_coerce2(param, type) \
> -    _Generic(param, type: param, float16_t: param, float32_t: param, 
> default: *(type *)__ARM_undef)
> +    _Generic(param, type: param, __fp16: param, default: _Generic 
> (param, _Float16: param, float16_t: param, float32_t: param, default: 
> *(type *)__ARM_undef))
>   #define __ARM_mve_coerce3(param, type) \
> -    _Generic(param, type: param, int8_t: param, int16_t: param, 
> int32_t: param, int64_t: param, uint8_t: param, uint16_t: param, 
> uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef)
> +    _Generic(param, type: param, default: _Generic (param, int8_t: 
> param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param, 
> uint16_t: param, uint32_t: param, uint64_t: param, default: *(type 
> *)__ARM_undef))
> 
>   #if (__ARM_FEATURE_MVE & 2) /* MVE Floating point.  */
> 
> diff --git 
> a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..427dcacb5ff59b53d5eab1f1582ef6460da3f2f3
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c
> @@ -0,0 +1,65 @@
> +/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
> +/* { dg-add-options arm_v8_1m_mve_fp } */
> +/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
> +#include "arm_mve.h"
> +
> +float f1;
> +double f2;
> +float16_t f3;
> +float32_t f4;
> +__fp16 f5;
> +_Float16 f6;
> +
> +int i1;
> +short i2;
> +long i3;
> +long long i4;
> +int8_t i5;
> +int16_t i6;
> +int32_t i7;
> +int64_t i8;
> +
> +const int ci1;
> +const short ci2;
> +const long ci3;
> +const long long ci4;
> +const int8_t ci5;
> +const int16_t ci6;
> +const int32_t ci7;
> +const int64_t ci8;
> +
> +float16x8_t floatvec;
> +int16x8_t intvec;
> +
> +void test(void)
> +{
> +    /* Test a few different supported ways of passing an int value.  The
> +    intrinsic vmulq was chosen arbitrarily, but it is representative of
> +    all intrinsics that take a non-const scalar value.  */
> +    intvec = vmulq(intvec, 2);
> +    intvec = vmulq(intvec, (int32_t) 2);
> +    intvec = vmulq(intvec, (short) 2);
> +    intvec = vmulq(intvec, i1);
> +    intvec = vmulq(intvec, i2);
> +    intvec = vmulq(intvec, i3);
> +    intvec = vmulq(intvec, i4);
> +    intvec = vmulq(intvec, i5);
> +    intvec = vmulq(intvec, i6);
> +    intvec = vmulq(intvec, i7);
> +    intvec = vmulq(intvec, i8);
> +
> +    /* Test a few different supported ways of passing a float value.  */
> +    floatvec = vmulq(floatvec, 0.5);
> +    floatvec = vmulq(floatvec, 0.5f);
> +    floatvec = vmulq(floatvec, (__fp16) 0.5);
> +    floatvec = vmulq(floatvec, f1);
> +    floatvec = vmulq(floatvec, f2);
> +    floatvec = vmulq(floatvec, f3);
> +    floatvec = vmulq(floatvec, f4);
> +    floatvec = vmulq(floatvec, f5);
> +    floatvec = vmulq(floatvec, f6);
> +    floatvec = vmulq(floatvec, 0.15f16);
> +    floatvec = vmulq(floatvec, (_Float16) 0.15);
> +}
> +
> +/* { dg-final { scan-assembler-not "__ARM_undef" } } */
> \ No newline at end of file
> diff --git 
> a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..d0e3b3eb30f46cb8327e7b976713990721305c9b
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c
> @@ -0,0 +1,45 @@
> +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> +/* { dg-add-options arm_v8_1m_mve } */
> +/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */
> +
> +#include "arm_mve.h"
> +
> +int i1;
> +short i2;
> +long i3;
> +long long i4;
> +int8_t i5;
> +int16_t i6;
> +int32_t i7;
> +int64_t i8;
> +
> +const int ci1;
> +const short ci2;
> +const long ci3;
> +const long long ci4;
> +const int8_t ci5;
> +const int16_t ci6;
> +const int32_t ci7;
> +const int64_t ci8;
> +
> +int16x8_t intvec;
> +
> +void test(void)
> +{
> +    /* Test a few different supported ways of passing an int value.  The
> +    intrinsic vmulq was chosen arbitrarily, but it is representative of
> +    all intrinsics that take a non-const scalar value.  */
> +    intvec = vmulq(intvec, 2);
> +    intvec = vmulq(intvec, (int32_t) 2);
> +    intvec = vmulq(intvec, (short) 2);
> +    intvec = vmulq(intvec, i1);
> +    intvec = vmulq(intvec, i2);
> +    intvec = vmulq(intvec, i3);
> +    intvec = vmulq(intvec, i4);
> +    intvec = vmulq(intvec, i5);
> +    intvec = vmulq(intvec, i6);
> +    intvec = vmulq(intvec, i7);
> +    intvec = vmulq(intvec, i8);
> +}
> +
> +/* { dg-final { scan-assembler-not "__ARM_undef" } } */
> \ No newline at end of file

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

end of thread, other threads:[~2023-01-13 16:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 18:19 [PATCH] arm: Split up MVE _Generic associations to prevent type clashes [PR107515] Stam Markianos-Wright
2023-01-10 14:07 ` [PING][PATCH] " Stam Markianos-Wright
2023-01-13 16:47 ` [PATCH] " Richard Earnshaw

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).