[V2] rs6000: Don't allow AltiVec address in movoo & movxo pattern [PR110411]
Checks
Commit Message
Hi All,
The following patch has been bootstrapped and regtested on powerpc64le-linux.
There are no instructions that do traditional AltiVec addresses (i.e.
with the low four bits of the address masked off) for OOmode and XOmode
objects. The solution is to modify the constraints used in the movoo and
movxo pattern to disallow these types of addresses, which assists LRA in
resolving this issue. Furthermore, the mode size 16 check has been
removed in vsx_quad_dform_memory_operand to allow OOmode and
quad_address_p already handles less than size 16.
2023-07-19 Jeevitha Palanisamy <jeevitha@linux.ibm.com>
gcc/
PR target/110411
* config/rs6000/mma.md (define_insn_and_split movoo): Disallow
AltiVec address in movoo and movxo pattern.
(define_insn_and_split movxo): Likewise.
*config/rs6000/predicates.md (vsx_quad_dform_memory_operand):Remove
redundant mode size check.
gcc/testsuite/
PR target/110411
* gcc.target/powerpc/pr110411-1.c: New testcase.
* gcc.target/powerpc/pr110411-2.c: New testcase.
Comments
Ping!
please review.
Thanks & Regards
Jeevitha
On 19/07/23 10:16 pm, jeevitha wrote:
> Hi All,
>
> The following patch has been bootstrapped and regtested on powerpc64le-linux.
>
> There are no instructions that do traditional AltiVec addresses (i.e.
> with the low four bits of the address masked off) for OOmode and XOmode
> objects. The solution is to modify the constraints used in the movoo and
> movxo pattern to disallow these types of addresses, which assists LRA in
> resolving this issue. Furthermore, the mode size 16 check has been
> removed in vsx_quad_dform_memory_operand to allow OOmode and
> quad_address_p already handles less than size 16.
>
> 2023-07-19 Jeevitha Palanisamy <jeevitha@linux.ibm.com>
>
> gcc/
> PR target/110411
> * config/rs6000/mma.md (define_insn_and_split movoo): Disallow
> AltiVec address in movoo and movxo pattern.
> (define_insn_and_split movxo): Likewise.
> *config/rs6000/predicates.md (vsx_quad_dform_memory_operand):Remove
> redundant mode size check.
>
> gcc/testsuite/
> PR target/110411
> * gcc.target/powerpc/pr110411-1.c: New testcase.
> * gcc.target/powerpc/pr110411-2.c: New testcase.
>
> diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
> index d36dc13872b..575751d477e 100644
> --- a/gcc/config/rs6000/mma.md
> +++ b/gcc/config/rs6000/mma.md
> @@ -293,8 +293,8 @@
> })
>
> (define_insn_and_split "*movoo"
> - [(set (match_operand:OO 0 "nonimmediate_operand" "=wa,m,wa")
> - (match_operand:OO 1 "input_operand" "m,wa,wa"))]
> + [(set (match_operand:OO 0 "nonimmediate_operand" "=wa,ZwO,wa")
> + (match_operand:OO 1 "input_operand" "ZwO,wa,wa"))]
> "TARGET_MMA
> && (gpc_reg_operand (operands[0], OOmode)
> || gpc_reg_operand (operands[1], OOmode))"
> @@ -340,8 +340,8 @@
> })
>
> (define_insn_and_split "*movxo"
> - [(set (match_operand:XO 0 "nonimmediate_operand" "=d,m,d")
> - (match_operand:XO 1 "input_operand" "m,d,d"))]
> + [(set (match_operand:XO 0 "nonimmediate_operand" "=d,ZwO,d")
> + (match_operand:XO 1 "input_operand" "ZwO,d,d"))]
> "TARGET_MMA
> && (gpc_reg_operand (operands[0], XOmode)
> || gpc_reg_operand (operands[1], XOmode))"
> diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
> index 3552d908e9d..925f69cd3fc 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -924,7 +924,7 @@
> (define_predicate "vsx_quad_dform_memory_operand"
> (match_code "mem")
> {
> - if (!TARGET_P9_VECTOR || GET_MODE_SIZE (mode) != 16)
> + if (!TARGET_P9_VECTOR)
> return false;
>
> return quad_address_p (XEXP (op, 0), mode, false);
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110411-1.c b/gcc/testsuite/gcc.target/powerpc/pr110411-1.c
> new file mode 100644
> index 00000000000..f42e9388d65
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110411-1.c
> @@ -0,0 +1,22 @@
> +/* PR target/110411 */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mblock-ops-vector-pair" } */
> +
> +/* Verify we do not ICE on the following. */
> +
> +#include <string.h>
> +
> +struct s {
> + long a;
> + long b;
> + long c;
> + long d: 1;
> +};
> +unsigned long ptr;
> +
> +void
> +bug (struct s *dst)
> +{
> + struct s *src = (struct s *)(ptr & ~0xFUL);
> + memcpy (dst, src, sizeof(struct s));
> +}
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110411-2.c b/gcc/testsuite/gcc.target/powerpc/pr110411-2.c
> new file mode 100644
> index 00000000000..c2046fb9855
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110411-2.c
> @@ -0,0 +1,12 @@
> +/* PR target/110411 */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> +
> +/* Verify we do not ICE on the following. */
> +
> +void
> +bug (__vector_quad *dst)
> +{
> + dst = (__vector_quad *)((unsigned long)dst & ~0xFUL);
> + __builtin_mma_xxsetaccz (dst);
> +}
>
>
>
On 7/19/23 11:46 AM, jeevitha via Gcc-patches wrote:
> gcc/
> PR target/110411
> * config/rs6000/mma.md (define_insn_and_split movoo): Disallow
> AltiVec address in movoo and movxo pattern.
No need to mention movxo here, since the next change covers movxo.
And maybe better as "Disallow AltiVec address operands."?
> (define_insn_and_split movxo): Likewise.
Fine.
> *config/rs6000/predicates.md (vsx_quad_dform_memory_operand):Remove
^ ^
Need a space in the two spots above.
I cannot approve it, but it looks good to me with the above bits fixed.
Peter
Hi Jeevitha,
on 2023/7/20 00:46, jeevitha wrote:
> Hi All,
>
> The following patch has been bootstrapped and regtested on powerpc64le-linux.
>
> There are no instructions that do traditional AltiVec addresses (i.e.
> with the low four bits of the address masked off) for OOmode and XOmode
> objects. The solution is to modify the constraints used in the movoo and
> movxo pattern to disallow these types of addresses, which assists LRA in
> resolving this issue. Furthermore, the mode size 16 check has been
> removed in vsx_quad_dform_memory_operand to allow OOmode and
Excepting for the nits Peter caught, one minor nit: "... to allow
OOmode and XOmode, and ..."
This patch looks quite reasonable that to make mov[ox]o to use the same
memory constraints as what the underlying vsx paired load/store insns use.
It's okay for trunk with those nits tweaked, also okay for all affected
release branches after burn-in time, thanks!
BR,
Kewen
> quad_address_p already handles less than size 16.
>
> 2023-07-19 Jeevitha Palanisamy <jeevitha@linux.ibm.com>
>
> gcc/
> PR target/110411
> * config/rs6000/mma.md (define_insn_and_split movoo): Disallow
> AltiVec address in movoo and movxo pattern.
> (define_insn_and_split movxo): Likewise.
> *config/rs6000/predicates.md (vsx_quad_dform_memory_operand):Remove
> redundant mode size check.
>
> gcc/testsuite/
> PR target/110411
> * gcc.target/powerpc/pr110411-1.c: New testcase.
> * gcc.target/powerpc/pr110411-2.c: New testcase.
>
> diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
> index d36dc13872b..575751d477e 100644
> --- a/gcc/config/rs6000/mma.md
> +++ b/gcc/config/rs6000/mma.md
> @@ -293,8 +293,8 @@
> })
>
> (define_insn_and_split "*movoo"
> - [(set (match_operand:OO 0 "nonimmediate_operand" "=wa,m,wa")
> - (match_operand:OO 1 "input_operand" "m,wa,wa"))]
> + [(set (match_operand:OO 0 "nonimmediate_operand" "=wa,ZwO,wa")
> + (match_operand:OO 1 "input_operand" "ZwO,wa,wa"))]
> "TARGET_MMA
> && (gpc_reg_operand (operands[0], OOmode)
> || gpc_reg_operand (operands[1], OOmode))"
> @@ -340,8 +340,8 @@
> })
>
> (define_insn_and_split "*movxo"
> - [(set (match_operand:XO 0 "nonimmediate_operand" "=d,m,d")
> - (match_operand:XO 1 "input_operand" "m,d,d"))]
> + [(set (match_operand:XO 0 "nonimmediate_operand" "=d,ZwO,d")
> + (match_operand:XO 1 "input_operand" "ZwO,d,d"))]
> "TARGET_MMA
> && (gpc_reg_operand (operands[0], XOmode)
> || gpc_reg_operand (operands[1], XOmode))"
> diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
> index 3552d908e9d..925f69cd3fc 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -924,7 +924,7 @@
> (define_predicate "vsx_quad_dform_memory_operand"
> (match_code "mem")
> {
> - if (!TARGET_P9_VECTOR || GET_MODE_SIZE (mode) != 16)
> + if (!TARGET_P9_VECTOR)
> return false;
>
> return quad_address_p (XEXP (op, 0), mode, false);
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110411-1.c b/gcc/testsuite/gcc.target/powerpc/pr110411-1.c
> new file mode 100644
> index 00000000000..f42e9388d65
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110411-1.c
> @@ -0,0 +1,22 @@
> +/* PR target/110411 */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mblock-ops-vector-pair" } */
> +
> +/* Verify we do not ICE on the following. */
> +
> +#include <string.h>
> +
> +struct s {
> + long a;
> + long b;
> + long c;
> + long d: 1;
> +};
> +unsigned long ptr;
> +
> +void
> +bug (struct s *dst)
> +{
> + struct s *src = (struct s *)(ptr & ~0xFUL);
> + memcpy (dst, src, sizeof(struct s));
> +}
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110411-2.c b/gcc/testsuite/gcc.target/powerpc/pr110411-2.c
> new file mode 100644
> index 00000000000..c2046fb9855
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110411-2.c
> @@ -0,0 +1,12 @@
> +/* PR target/110411 */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> +
> +/* Verify we do not ICE on the following. */
> +
> +void
> +bug (__vector_quad *dst)
> +{
> + dst = (__vector_quad *)((unsigned long)dst & ~0xFUL);
> + __builtin_mma_xxsetaccz (dst);
> +}
>
>
@@ -293,8 +293,8 @@
})
(define_insn_and_split "*movoo"
- [(set (match_operand:OO 0 "nonimmediate_operand" "=wa,m,wa")
- (match_operand:OO 1 "input_operand" "m,wa,wa"))]
+ [(set (match_operand:OO 0 "nonimmediate_operand" "=wa,ZwO,wa")
+ (match_operand:OO 1 "input_operand" "ZwO,wa,wa"))]
"TARGET_MMA
&& (gpc_reg_operand (operands[0], OOmode)
|| gpc_reg_operand (operands[1], OOmode))"
@@ -340,8 +340,8 @@
})
(define_insn_and_split "*movxo"
- [(set (match_operand:XO 0 "nonimmediate_operand" "=d,m,d")
- (match_operand:XO 1 "input_operand" "m,d,d"))]
+ [(set (match_operand:XO 0 "nonimmediate_operand" "=d,ZwO,d")
+ (match_operand:XO 1 "input_operand" "ZwO,d,d"))]
"TARGET_MMA
&& (gpc_reg_operand (operands[0], XOmode)
|| gpc_reg_operand (operands[1], XOmode))"
@@ -924,7 +924,7 @@
(define_predicate "vsx_quad_dform_memory_operand"
(match_code "mem")
{
- if (!TARGET_P9_VECTOR || GET_MODE_SIZE (mode) != 16)
+ if (!TARGET_P9_VECTOR)
return false;
return quad_address_p (XEXP (op, 0), mode, false);
new file mode 100644
@@ -0,0 +1,22 @@
+/* PR target/110411 */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10 -mblock-ops-vector-pair" } */
+
+/* Verify we do not ICE on the following. */
+
+#include <string.h>
+
+struct s {
+ long a;
+ long b;
+ long c;
+ long d: 1;
+};
+unsigned long ptr;
+
+void
+bug (struct s *dst)
+{
+ struct s *src = (struct s *)(ptr & ~0xFUL);
+ memcpy (dst, src, sizeof(struct s));
+}
new file mode 100644
@@ -0,0 +1,12 @@
+/* PR target/110411 */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
+
+/* Verify we do not ICE on the following. */
+
+void
+bug (__vector_quad *dst)
+{
+ dst = (__vector_quad *)((unsigned long)dst & ~0xFUL);
+ __builtin_mma_xxsetaccz (dst);
+}