* [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627]
@ 2021-12-23 2:09 Kewen.Lin
2022-01-13 2:00 ` PING^1 " Kewen.Lin
2022-01-26 22:19 ` Segher Boessenkool
0 siblings, 2 replies; 5+ messages in thread
From: Kewen.Lin @ 2021-12-23 2:09 UTC (permalink / raw)
To: GCC Patches
Cc: Segher Boessenkool, David Edelsohn, Bill Schmidt, Peter Bergner
Hi,
As PR103627 shows, there is an unexpected case where !TARGET_VSX
and TARGET_MMA co-exist. As ISA3.1 claims, SIMD is a requirement
for MMA. By looking into the ICE, I noticed that the current
MMA implementation depends on vector pairs load/store, but since
we don't have a separated option to control Power10 vector, this
patch is to check for Power9 vector instead.
Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
powerpc64-linux-gnu P8.
Is it ok for trunk?
BR,
Kewen
-----
gcc/ChangeLog:
PR target/103627
* config/rs6000/rs6000.c (rs6000_option_override_internal): Disable
MMA if !TARGET_P9_VECTOR.
gcc/testsuite/ChangeLog:
PR target/103627
* gcc.target/powerpc/pr103627-1.c: New test.
* gcc.target/powerpc/pr103627-2.c: New test.
---
gcc/config/rs6000/rs6000.c | 11 +++++++++++
gcc/testsuite/gcc.target/powerpc/pr103627-1.c | 16 ++++++++++++++++
gcc/testsuite/gcc.target/powerpc/pr103627-2.c | 16 ++++++++++++++++
3 files changed, 43 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103627-1.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103627-2.c
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index c020947abc8..ec3b46682a7 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4505,6 +4505,17 @@ rs6000_option_override_internal (bool global_init_p)
rs6000_isa_flags &= ~OPTION_MASK_MMA;
}
+ /* MMA requires SIMD support as ISA 3.1 claims and our implementation
+ such as "*movoo" uses vector pair access which are only supported
+ from ISA 3.1. But since we don't have one separated option to
+ control Power10 vector, check for Power9 vector instead. */
+ if (TARGET_MMA && !TARGET_P9_VECTOR)
+ {
+ if ((rs6000_isa_flags_explicit & OPTION_MASK_MMA) != 0)
+ error ("%qs requires %qs", "-mmma", "-mpower9-vector");
+ rs6000_isa_flags &= ~OPTION_MASK_MMA;
+ }
+
if (!TARGET_PCREL && TARGET_PCREL_OPT)
rs6000_isa_flags &= ~OPTION_MASK_PCREL_OPT;
diff --git a/gcc/testsuite/gcc.target/powerpc/pr103627-1.c b/gcc/testsuite/gcc.target/powerpc/pr103627-1.c
new file mode 100644
index 00000000000..6c6c16188fb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr103627-1.c
@@ -0,0 +1,16 @@
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-mdejagnu-cpu=power10 -mno-power9-vector" } */
+
+/* Verify compiler emits error message instead of ICE. */
+
+extern float *dest;
+extern __vector_quad src;
+
+int
+foo ()
+{
+ __builtin_mma_disassemble_acc (dest, &src);
+ /* { dg-error "'__builtin_mma_disassemble_acc' requires the '-mmma' option" "" { target *-*-* } .-1 } */
+ return 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/powerpc/pr103627-2.c b/gcc/testsuite/gcc.target/powerpc/pr103627-2.c
new file mode 100644
index 00000000000..6604872c0e8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr103627-2.c
@@ -0,0 +1,16 @@
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-mdejagnu-cpu=power10 -mmma -mno-power9-vector" } */
+
+/* Verify the emitted error message. */
+
+extern float *dest;
+extern __vector_quad src;
+
+int
+foo ()
+{
+ __builtin_mma_disassemble_acc (dest, &src);
+ /* { dg-error "'-mmma' requires '-mpower9-vector'" "mma" { target *-*-* } 0 } */
+ return 0;
+}
+
--
2.27.0
^ permalink raw reply [flat|nested] 5+ messages in thread* PING^1 [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627]
2021-12-23 2:09 [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627] Kewen.Lin
@ 2022-01-13 2:00 ` Kewen.Lin
2022-01-26 2:15 ` PING^2 " Kewen.Lin
2022-01-26 22:19 ` Segher Boessenkool
1 sibling, 1 reply; 5+ messages in thread
From: Kewen.Lin @ 2022-01-13 2:00 UTC (permalink / raw)
To: GCC Patches
Cc: Peter Bergner, Bill Schmidt, David Edelsohn, Segher Boessenkool
Gentle ping:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587310.html
on 2021/12/23 上午10:09, Kewen.Lin via Gcc-patches wrote:
> Hi,
>
> As PR103627 shows, there is an unexpected case where !TARGET_VSX
> and TARGET_MMA co-exist. As ISA3.1 claims, SIMD is a requirement
> for MMA. By looking into the ICE, I noticed that the current
> MMA implementation depends on vector pairs load/store, but since
> we don't have a separated option to control Power10 vector, this
> patch is to check for Power9 vector instead.
>
> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
> powerpc64-linux-gnu P8.
>
> Is it ok for trunk?
>
> BR,
> Kewen
> -----
> gcc/ChangeLog:
>
> PR target/103627
> * config/rs6000/rs6000.c (rs6000_option_override_internal): Disable
> MMA if !TARGET_P9_VECTOR.
>
> gcc/testsuite/ChangeLog:
>
> PR target/103627
> * gcc.target/powerpc/pr103627-1.c: New test.
> * gcc.target/powerpc/pr103627-2.c: New test.
> ---
> gcc/config/rs6000/rs6000.c | 11 +++++++++++
> gcc/testsuite/gcc.target/powerpc/pr103627-1.c | 16 ++++++++++++++++
> gcc/testsuite/gcc.target/powerpc/pr103627-2.c | 16 ++++++++++++++++
> 3 files changed, 43 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103627-1.c
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103627-2.c
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index c020947abc8..ec3b46682a7 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4505,6 +4505,17 @@ rs6000_option_override_internal (bool global_init_p)
> rs6000_isa_flags &= ~OPTION_MASK_MMA;
> }
>
> + /* MMA requires SIMD support as ISA 3.1 claims and our implementation
> + such as "*movoo" uses vector pair access which are only supported
> + from ISA 3.1. But since we don't have one separated option to
> + control Power10 vector, check for Power9 vector instead. */
> + if (TARGET_MMA && !TARGET_P9_VECTOR)
> + {
> + if ((rs6000_isa_flags_explicit & OPTION_MASK_MMA) != 0)
> + error ("%qs requires %qs", "-mmma", "-mpower9-vector");
> + rs6000_isa_flags &= ~OPTION_MASK_MMA;
> + }
> +
> if (!TARGET_PCREL && TARGET_PCREL_OPT)
> rs6000_isa_flags &= ~OPTION_MASK_PCREL_OPT;
>
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103627-1.c b/gcc/testsuite/gcc.target/powerpc/pr103627-1.c
> new file mode 100644
> index 00000000000..6c6c16188fb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr103627-1.c
> @@ -0,0 +1,16 @@
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-mdejagnu-cpu=power10 -mno-power9-vector" } */
> +
> +/* Verify compiler emits error message instead of ICE. */
> +
> +extern float *dest;
> +extern __vector_quad src;
> +
> +int
> +foo ()
> +{
> + __builtin_mma_disassemble_acc (dest, &src);
> + /* { dg-error "'__builtin_mma_disassemble_acc' requires the '-mmma' option" "" { target *-*-* } .-1 } */
> + return 0;
> +}
> +
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103627-2.c b/gcc/testsuite/gcc.target/powerpc/pr103627-2.c
> new file mode 100644
> index 00000000000..6604872c0e8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr103627-2.c
> @@ -0,0 +1,16 @@
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-mdejagnu-cpu=power10 -mmma -mno-power9-vector" } */
> +
> +/* Verify the emitted error message. */
> +
> +extern float *dest;
> +extern __vector_quad src;
> +
> +int
> +foo ()
> +{
> + __builtin_mma_disassemble_acc (dest, &src);
> + /* { dg-error "'-mmma' requires '-mpower9-vector'" "mma" { target *-*-* } 0 } */
> + return 0;
> +}
> +
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* PING^2 [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627]
2022-01-13 2:00 ` PING^1 " Kewen.Lin
@ 2022-01-26 2:15 ` Kewen.Lin
0 siblings, 0 replies; 5+ messages in thread
From: Kewen.Lin @ 2022-01-26 2:15 UTC (permalink / raw)
To: GCC Patches
Cc: Peter Bergner, Bill Schmidt, Segher Boessenkool, David Edelsohn
Gentle ping:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587310.html
BR,
Kewen
> on 2021/12/23 上午10:09, Kewen.Lin via Gcc-patches wrote:
>> Hi,
>>
>> As PR103627 shows, there is an unexpected case where !TARGET_VSX
>> and TARGET_MMA co-exist. As ISA3.1 claims, SIMD is a requirement
>> for MMA. By looking into the ICE, I noticed that the current
>> MMA implementation depends on vector pairs load/store, but since
>> we don't have a separated option to control Power10 vector, this
>> patch is to check for Power9 vector instead.
>>
>> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
>> powerpc64-linux-gnu P8.
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> -----
>> gcc/ChangeLog:
>>
>> PR target/103627
>> * config/rs6000/rs6000.c (rs6000_option_override_internal): Disable
>> MMA if !TARGET_P9_VECTOR.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR target/103627
>> * gcc.target/powerpc/pr103627-1.c: New test.
>> * gcc.target/powerpc/pr103627-2.c: New test.
>> ---
>> gcc/config/rs6000/rs6000.c | 11 +++++++++++
>> gcc/testsuite/gcc.target/powerpc/pr103627-1.c | 16 ++++++++++++++++
>> gcc/testsuite/gcc.target/powerpc/pr103627-2.c | 16 ++++++++++++++++
>> 3 files changed, 43 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103627-1.c
>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr103627-2.c
>>
>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> index c020947abc8..ec3b46682a7 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -4505,6 +4505,17 @@ rs6000_option_override_internal (bool global_init_p)
>> rs6000_isa_flags &= ~OPTION_MASK_MMA;
>> }
>>
>> + /* MMA requires SIMD support as ISA 3.1 claims and our implementation
>> + such as "*movoo" uses vector pair access which are only supported
>> + from ISA 3.1. But since we don't have one separated option to
>> + control Power10 vector, check for Power9 vector instead. */
>> + if (TARGET_MMA && !TARGET_P9_VECTOR)
>> + {
>> + if ((rs6000_isa_flags_explicit & OPTION_MASK_MMA) != 0)
>> + error ("%qs requires %qs", "-mmma", "-mpower9-vector");
>> + rs6000_isa_flags &= ~OPTION_MASK_MMA;
>> + }
>> +
>> if (!TARGET_PCREL && TARGET_PCREL_OPT)
>> rs6000_isa_flags &= ~OPTION_MASK_PCREL_OPT;
>>
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103627-1.c b/gcc/testsuite/gcc.target/powerpc/pr103627-1.c
>> new file mode 100644
>> index 00000000000..6c6c16188fb
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr103627-1.c
>> @@ -0,0 +1,16 @@
>> +/* { dg-require-effective-target power10_ok } */
>> +/* { dg-options "-mdejagnu-cpu=power10 -mno-power9-vector" } */
>> +
>> +/* Verify compiler emits error message instead of ICE. */
>> +
>> +extern float *dest;
>> +extern __vector_quad src;
>> +
>> +int
>> +foo ()
>> +{
>> + __builtin_mma_disassemble_acc (dest, &src);
>> + /* { dg-error "'__builtin_mma_disassemble_acc' requires the '-mmma' option" "" { target *-*-* } .-1 } */
>> + return 0;
>> +}
>> +
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr103627-2.c b/gcc/testsuite/gcc.target/powerpc/pr103627-2.c
>> new file mode 100644
>> index 00000000000..6604872c0e8
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr103627-2.c
>> @@ -0,0 +1,16 @@
>> +/* { dg-require-effective-target power10_ok } */
>> +/* { dg-options "-mdejagnu-cpu=power10 -mmma -mno-power9-vector" } */
>> +
>> +/* Verify the emitted error message. */
>> +
>> +extern float *dest;
>> +extern __vector_quad src;
>> +
>> +int
>> +foo ()
>> +{
>> + __builtin_mma_disassemble_acc (dest, &src);
>> + /* { dg-error "'-mmma' requires '-mpower9-vector'" "mma" { target *-*-* } 0 } */
>> + return 0;
>> +}
>> +
>> --
>> 2.27.0
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627]
2021-12-23 2:09 [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627] Kewen.Lin
2022-01-13 2:00 ` PING^1 " Kewen.Lin
@ 2022-01-26 22:19 ` Segher Boessenkool
2022-01-27 11:35 ` Kewen.Lin
1 sibling, 1 reply; 5+ messages in thread
From: Segher Boessenkool @ 2022-01-26 22:19 UTC (permalink / raw)
To: Kewen.Lin; +Cc: GCC Patches, David Edelsohn, Bill Schmidt, Peter Bergner
Hi!
On Thu, Dec 23, 2021 at 10:09:27AM +0800, Kewen.Lin wrote:
> As PR103627 shows, there is an unexpected case where !TARGET_VSX
> and TARGET_MMA co-exist. As ISA3.1 claims, SIMD is a requirement
> for MMA. By looking into the ICE, I noticed that the current
> MMA implementation depends on vector pairs load/store, but since
> we don't have a separated option to control Power10 vector, this
> patch is to check for Power9 vector instead.
>
> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
> powerpc64-linux-gnu P8.
>
> Is it ok for trunk?
No, sorry.
> + /* MMA requires SIMD support as ISA 3.1 claims and our implementation
> + such as "*movoo" uses vector pair access which are only supported
> + from ISA 3.1. But since we don't have one separated option to
> + control Power10 vector, check for Power9 vector instead. */
> + if (TARGET_MMA && !TARGET_P9_VECTOR)
> + {
> + if ((rs6000_isa_flags_explicit & OPTION_MASK_MMA) != 0)
> + error ("%qs requires %qs", "-mmma", "-mpower9-vector");
> + rs6000_isa_flags &= ~OPTION_MASK_MMA;
> + }
-mpower9-vector is a workaround that should go away. TARGET_MMA should
require ISA 3.1 (or POWER10) directly, and require VSX.
> +/* { dg-options "-mdejagnu-cpu=power10 -mno-power9-vector" } */
It should be impossible to select this at all. Either you have vectors
or you don't, but it should be impossible to selectively disable part of
vector support. That way madness lies.
We can allow no VSRs, only VRs, or all VSRs. There is precedent for
that (see -msoft-float for example, which means "do not use FPRs") --
when compiling certain code we want to disallow whole register banks.
But disallowing or allowing some instructions separately is not a good
idea: it doesn't give any useful extra functionality, it is hard to use,
and it is a lot of extra work for us (it is impossible to test, already,
too many combinations).
Segher
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627]
2022-01-26 22:19 ` Segher Boessenkool
@ 2022-01-27 11:35 ` Kewen.Lin
0 siblings, 0 replies; 5+ messages in thread
From: Kewen.Lin @ 2022-01-27 11:35 UTC (permalink / raw)
To: Segher Boessenkool
Cc: GCC Patches, David Edelsohn, Bill Schmidt, Peter Bergner
Hi Segher,
on 2022/1/27 上午6:19, Segher Boessenkool wrote:
> Hi!
>
> On Thu, Dec 23, 2021 at 10:09:27AM +0800, Kewen.Lin wrote:
>> As PR103627 shows, there is an unexpected case where !TARGET_VSX
>> and TARGET_MMA co-exist. As ISA3.1 claims, SIMD is a requirement
>> for MMA. By looking into the ICE, I noticed that the current
>> MMA implementation depends on vector pairs load/store, but since
>> we don't have a separated option to control Power10 vector, this
>> patch is to check for Power9 vector instead.
>>
>> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
>> powerpc64-linux-gnu P8.
>>
>> Is it ok for trunk?
>
> No, sorry.
>
>> + /* MMA requires SIMD support as ISA 3.1 claims and our implementation
>> + such as "*movoo" uses vector pair access which are only supported
>> + from ISA 3.1. But since we don't have one separated option to
>> + control Power10 vector, check for Power9 vector instead. */
>> + if (TARGET_MMA && !TARGET_P9_VECTOR)
>> + {
>> + if ((rs6000_isa_flags_explicit & OPTION_MASK_MMA) != 0)
>> + error ("%qs requires %qs", "-mmma", "-mpower9-vector");
>> + rs6000_isa_flags &= ~OPTION_MASK_MMA;
>> + }
>
> -mpower9-vector is a workaround that should go away. TARGET_MMA should
> require ISA 3.1 (or POWER10) directly, and require VSX.
OK, I see. Thanks for the detailed explanation below. I guess that's why
we don't have one option like "-mpower10-vector" any more?
I posted patch v2 guarded with VSX at the below link instead
https://gcc.gnu.org/pipermail/gcc-patches/2022-January/589325.html
Hope it looks better to you. :)
BR,
Kewen
>
>> +/* { dg-options "-mdejagnu-cpu=power10 -mno-power9-vector" } */
>
> It should be impossible to select this at all. Either you have vectors
> or you don't, but it should be impossible to selectively disable part of
> vector support. That way madness lies.
>
> We can allow no VSRs, only VRs, or all VSRs. There is precedent for
> that (see -msoft-float for example, which means "do not use FPRs") --
> when compiling certain code we want to disallow whole register banks.
> But disallowing or allowing some instructions separately is not a good
> idea: it doesn't give any useful extra functionality, it is hard to use,
> and it is a lot of extra work for us (it is impossible to test, already,
> too many combinations).
>
>
> Segher
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-27 11:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-23 2:09 [PATCH] rs6000: Disable MMA if no P9 VECTOR support [PR103627] Kewen.Lin
2022-01-13 2:00 ` PING^1 " Kewen.Lin
2022-01-26 2:15 ` PING^2 " Kewen.Lin
2022-01-26 22:19 ` Segher Boessenkool
2022-01-27 11:35 ` Kewen.Lin
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).