* [PATCH] aarch64: PR target/102252 Invalid addressing mode for SVE load predicate
@ 2021-09-13 8:20 Kyrylo Tkachov
2021-09-13 11:08 ` Richard Sandiford
0 siblings, 1 reply; 3+ messages in thread
From: Kyrylo Tkachov @ 2021-09-13 8:20 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]
Hi all,
In the testcase we generate invalid assembly for an SVE load predicate instruction.
The RTL for the insn is:
(insn 9 8 10 (set (reg:VNx16BI 68 p0)
(mem:VNx16BI (plus:DI (mult:DI (reg:DI 1 x1 [93])
(const_int 8 [0x8]))
(reg/f:DI 0 x0 [92])) [2 work_3(D)->array[offset_4(D)]+0 S8 A16]))
That addressing mode is not valid for the instruction [1] as it only accepts the addressing mode:
[<Xn|SP>{, #<imm>, MUL VL}]
This patch rejects the register index form for SVE predicate modes.
Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?
Thanks,
Kyrill
[1] https://developer.arm.com/documentation/ddi0602/2021-06/SVE-Instructions/LDR--predicate---Load-predicate-register-
gcc/ChangeLog:
PR target/102252
* config/aarch64/aarch64.c (aarch64_classify_address): Don't allow
register index for SVE predicate modes.
gcc/testsuite/ChangeLog:
PR target/102252
* g++.target/aarch64/sve/pr102252.C: New test.
[-- Attachment #2: pred-addr.patch --]
[-- Type: application/octet-stream, Size: 2463 bytes --]
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e37922db0007e3b4b559cda65f135247f4fb1b9f..e6253edeb55cdcc3dbc7303e03bad26dd519c4b1 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9770,7 +9770,7 @@ aarch64_classify_address (struct aarch64_address_info *info,
|| mode == TImode
|| mode == TFmode
|| (BYTES_BIG_ENDIAN && advsimd_struct_p));
-
+ bool sve_pred_p = (vec_flags & VEC_SVE_PRED) != 0;
/* If we are dealing with ADDR_QUERY_LDP_STP_N that means the incoming mode
corresponds to the actual size of the memory being loaded/stored and the
mode of the corresponding addressing mode is half of that. */
@@ -9779,12 +9779,14 @@ aarch64_classify_address (struct aarch64_address_info *info,
mode = DFmode;
bool allow_reg_index_p = (!load_store_pair_p
+ && !sve_pred_p
&& (known_lt (GET_MODE_SIZE (mode), 16)
|| vec_flags == VEC_ADVSIMD
|| vec_flags & VEC_SVE_DATA));
- /* For SVE, only accept [Rn], [Rn, Rm, LSL #shift] and
- [Rn, #offset, MUL VL]. */
+ /* For SVE, only accept [Rn], [Rn, #offset, MUL VL] and [Rn, Rm, LSL #shift].
+ The latter is not valid for SVE predicates, and that's rejected through
+ allow_reg_index_p above. */
if ((vec_flags & (VEC_SVE_DATA | VEC_SVE_PRED)) != 0
&& (code != REG && code != PLUS))
return false;
diff --git a/gcc/testsuite/g++.target/aarch64/sve/pr102252.C b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
new file mode 100644
index 0000000000000000000000000000000000000000..f90f1218555f0dfdb0253fe83c656ba03b1aac43
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
@@ -0,0 +1,37 @@
+/* PR target/102252. */
+/* { dg-do assemble { target aarch64_asm_sve_ok } } */
+/* { dg-options "-march=armv8.2-a+sve -msve-vector-bits=512" } */
+
+/* We used to generate invalid assembly for SVE predicate loads. */
+
+#include <arm_sve.h>
+
+class SimdBool
+{
+private:
+ typedef svbool_t simdInternalType_ __attribute__((arm_sve_vector_bits(512)));
+
+public:
+ SimdBool() {}
+
+ simdInternalType_ simdInternal_;
+
+};
+
+static svfloat32_t selectByMask(svfloat32_t a, SimdBool m) {
+ return svsel_f32(m.simdInternal_, a, svdup_f32(0.0));
+}
+
+struct s {
+ SimdBool array[1];
+};
+
+
+
+void foo(struct s* const work, int offset)
+{
+ svfloat32_t tz_S0;
+
+ tz_S0 = selectByMask(tz_S0, work->array[offset]);
+}
+
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] aarch64: PR target/102252 Invalid addressing mode for SVE load predicate
2021-09-13 8:20 [PATCH] aarch64: PR target/102252 Invalid addressing mode for SVE load predicate Kyrylo Tkachov
@ 2021-09-13 11:08 ` Richard Sandiford
2021-09-13 14:42 ` Kyrylo Tkachov
0 siblings, 1 reply; 3+ messages in thread
From: Richard Sandiford @ 2021-09-13 11:08 UTC (permalink / raw)
To: Kyrylo Tkachov; +Cc: gcc-patches
Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:
> Hi all,
>
> In the testcase we generate invalid assembly for an SVE load predicate instruction.
> The RTL for the insn is:
> (insn 9 8 10 (set (reg:VNx16BI 68 p0)
> (mem:VNx16BI (plus:DI (mult:DI (reg:DI 1 x1 [93])
> (const_int 8 [0x8]))
> (reg/f:DI 0 x0 [92])) [2 work_3(D)->array[offset_4(D)]+0 S8 A16]))
>
> That addressing mode is not valid for the instruction [1] as it only accepts the addressing mode:
> [<Xn|SP>{, #<imm>, MUL VL}]
>
> This patch rejects the register index form for SVE predicate modes.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
>
> Ok for trunk?
> Thanks,
> Kyrill
>
> [1] https://developer.arm.com/documentation/ddi0602/2021-06/SVE-Instructions/LDR--predicate---Load-predicate-register-
>
> gcc/ChangeLog:
>
> PR target/102252
> * config/aarch64/aarch64.c (aarch64_classify_address): Don't allow
> register index for SVE predicate modes.
>
> gcc/testsuite/ChangeLog:
>
> PR target/102252
> * g++.target/aarch64/sve/pr102252.C: New test.
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index e37922db0007e3b4b559cda65f135247f4fb1b9f..e6253edeb55cdcc3dbc7303e03bad26dd519c4b1 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -9770,7 +9770,7 @@ aarch64_classify_address (struct aarch64_address_info *info,
> || mode == TImode
> || mode == TFmode
> || (BYTES_BIG_ENDIAN && advsimd_struct_p));
> -
> + bool sve_pred_p = (vec_flags & VEC_SVE_PRED) != 0;
> /* If we are dealing with ADDR_QUERY_LDP_STP_N that means the incoming mode
> corresponds to the actual size of the memory being loaded/stored and the
> mode of the corresponding addressing mode is half of that. */
> @@ -9779,12 +9779,14 @@ aarch64_classify_address (struct aarch64_address_info *info,
> mode = DFmode;
>
> bool allow_reg_index_p = (!load_store_pair_p
> + && !sve_pred_p
> && (known_lt (GET_MODE_SIZE (mode), 16)
> || vec_flags == VEC_ADVSIMD
> || vec_flags & VEC_SVE_DATA));
I think the known_lt (GET_MODE_SIZE (mode), 16) is really there for
non-vector cases, with the ||s enumerating the valid vector cases.
So how about:
bool allow_reg_index_p = (!load_store_pair_p
&& ((vec_flags == 0
&& known_lt (GET_MODE_SIZE (mode), 16))
|| vec_flags == VEC_ADVSIMD
|| vec_flags & VEC_SVE_DATA));
instead? OK with that change from my POV.
Thanks,
Richard
>
> - /* For SVE, only accept [Rn], [Rn, Rm, LSL #shift] and
> - [Rn, #offset, MUL VL]. */
> + /* For SVE, only accept [Rn], [Rn, #offset, MUL VL] and [Rn, Rm, LSL #shift].
> + The latter is not valid for SVE predicates, and that's rejected through
> + allow_reg_index_p above. */
> if ((vec_flags & (VEC_SVE_DATA | VEC_SVE_PRED)) != 0
> && (code != REG && code != PLUS))
> return false;
> diff --git a/gcc/testsuite/g++.target/aarch64/sve/pr102252.C b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
> new file mode 100644
> index 0000000000000000000000000000000000000000..f90f1218555f0dfdb0253fe83c656ba03b1aac43
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
> @@ -0,0 +1,37 @@
> +/* PR target/102252. */
> +/* { dg-do assemble { target aarch64_asm_sve_ok } } */
> +/* { dg-options "-march=armv8.2-a+sve -msve-vector-bits=512" } */
> +
> +/* We used to generate invalid assembly for SVE predicate loads. */
> +
> +#include <arm_sve.h>
> +
> +class SimdBool
> +{
> +private:
> + typedef svbool_t simdInternalType_ __attribute__((arm_sve_vector_bits(512)));
> +
> +public:
> + SimdBool() {}
> +
> + simdInternalType_ simdInternal_;
> +
> +};
> +
> +static svfloat32_t selectByMask(svfloat32_t a, SimdBool m) {
> + return svsel_f32(m.simdInternal_, a, svdup_f32(0.0));
> +}
> +
> +struct s {
> + SimdBool array[1];
> +};
> +
> +
> +
> +void foo(struct s* const work, int offset)
> +{
> + svfloat32_t tz_S0;
> +
> + tz_S0 = selectByMask(tz_S0, work->array[offset]);
> +}
> +
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] aarch64: PR target/102252 Invalid addressing mode for SVE load predicate
2021-09-13 11:08 ` Richard Sandiford
@ 2021-09-13 14:42 ` Kyrylo Tkachov
0 siblings, 0 replies; 3+ messages in thread
From: Kyrylo Tkachov @ 2021-09-13 14:42 UTC (permalink / raw)
To: Richard Sandiford; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4970 bytes --]
Hi Richard,
> -----Original Message-----
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: 13 September 2021 12:09
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] aarch64: PR target/102252 Invalid addressing mode for
> SVE load predicate
>
> Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:
> > Hi all,
> >
> > In the testcase we generate invalid assembly for an SVE load predicate
> instruction.
> > The RTL for the insn is:
> > (insn 9 8 10 (set (reg:VNx16BI 68 p0)
> > (mem:VNx16BI (plus:DI (mult:DI (reg:DI 1 x1 [93])
> > (const_int 8 [0x8]))
> > (reg/f:DI 0 x0 [92])) [2 work_3(D)->array[offset_4(D)]+0 S8 A16]))
> >
> > That addressing mode is not valid for the instruction [1] as it only accepts
> the addressing mode:
> > [<Xn|SP>{, #<imm>, MUL VL}]
> >
> > This patch rejects the register index form for SVE predicate modes.
> >
> > Bootstrapped and tested on aarch64-none-linux-gnu.
> >
> > Ok for trunk?
> > Thanks,
> > Kyrill
> >
> > [1] https://developer.arm.com/documentation/ddi0602/2021-06/SVE-
> Instructions/LDR--predicate---Load-predicate-register-
> >
> > gcc/ChangeLog:
> >
> > PR target/102252
> > * config/aarch64/aarch64.c (aarch64_classify_address): Don't allow
> > register index for SVE predicate modes.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR target/102252
> > * g++.target/aarch64/sve/pr102252.C: New test.
> >
> > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > index
> e37922db0007e3b4b559cda65f135247f4fb1b9f..e6253edeb55cdcc3dbc7303
> e03bad26dd519c4b1 100644
> > --- a/gcc/config/aarch64/aarch64.c
> > +++ b/gcc/config/aarch64/aarch64.c
> > @@ -9770,7 +9770,7 @@ aarch64_classify_address (struct
> aarch64_address_info *info,
> > || mode == TImode
> > || mode == TFmode
> > || (BYTES_BIG_ENDIAN && advsimd_struct_p));
> > -
> > + bool sve_pred_p = (vec_flags & VEC_SVE_PRED) != 0;
> > /* If we are dealing with ADDR_QUERY_LDP_STP_N that means the
> incoming mode
> > corresponds to the actual size of the memory being loaded/stored and
> the
> > mode of the corresponding addressing mode is half of that. */
> > @@ -9779,12 +9779,14 @@ aarch64_classify_address (struct
> aarch64_address_info *info,
> > mode = DFmode;
> >
> > bool allow_reg_index_p = (!load_store_pair_p
> > + && !sve_pred_p
> > && (known_lt (GET_MODE_SIZE (mode), 16)
> > || vec_flags == VEC_ADVSIMD
> > || vec_flags & VEC_SVE_DATA));
>
> I think the known_lt (GET_MODE_SIZE (mode), 16) is really there for
> non-vector cases, with the ||s enumerating the valid vector cases.
> So how about:
>
> bool allow_reg_index_p = (!load_store_pair_p
> && ((vec_flags == 0
> && known_lt (GET_MODE_SIZE (mode), 16))
> || vec_flags == VEC_ADVSIMD
> || vec_flags & VEC_SVE_DATA));
>
> instead? OK with that change from my POV.
Yeah, that works.
Thanks, here's what I've committed. I'll wait a bit before backporting to the branches.
Kyrill
>
> Thanks,
> Richard
>
> >
> > - /* For SVE, only accept [Rn], [Rn, Rm, LSL #shift] and
> > - [Rn, #offset, MUL VL]. */
> > + /* For SVE, only accept [Rn], [Rn, #offset, MUL VL] and [Rn, Rm, LSL
> #shift].
> > + The latter is not valid for SVE predicates, and that's rejected through
> > + allow_reg_index_p above. */
> > if ((vec_flags & (VEC_SVE_DATA | VEC_SVE_PRED)) != 0
> > && (code != REG && code != PLUS))
> > return false;
> > diff --git a/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
> b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..f90f1218555f0dfdb0253fe
> 83c656ba03b1aac43
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
> > @@ -0,0 +1,37 @@
> > +/* PR target/102252. */
> > +/* { dg-do assemble { target aarch64_asm_sve_ok } } */
> > +/* { dg-options "-march=armv8.2-a+sve -msve-vector-bits=512" } */
> > +
> > +/* We used to generate invalid assembly for SVE predicate loads. */
> > +
> > +#include <arm_sve.h>
> > +
> > +class SimdBool
> > +{
> > +private:
> > + typedef svbool_t simdInternalType_
> __attribute__((arm_sve_vector_bits(512)));
> > +
> > +public:
> > + SimdBool() {}
> > +
> > + simdInternalType_ simdInternal_;
> > +
> > +};
> > +
> > +static svfloat32_t selectByMask(svfloat32_t a, SimdBool m) {
> > + return svsel_f32(m.simdInternal_, a, svdup_f32(0.0));
> > +}
> > +
> > +struct s {
> > + SimdBool array[1];
> > +};
> > +
> > +
> > +
> > +void foo(struct s* const work, int offset)
> > +{
> > + svfloat32_t tz_S0;
> > +
> > + tz_S0 = selectByMask(tz_S0, work->array[offset]);
> > +}
> > +
[-- Attachment #2: pred-addr.patch --]
[-- Type: application/octet-stream, Size: 2460 bytes --]
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index eaf2587c4817e51b47fd96d5a97965ad06deff4e..40a9d056850b91d8589a8ebf45fafff76c39964d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9770,7 +9770,6 @@ aarch64_classify_address (struct aarch64_address_info *info,
|| mode == TImode
|| mode == TFmode
|| (BYTES_BIG_ENDIAN && advsimd_struct_p));
-
/* If we are dealing with ADDR_QUERY_LDP_STP_N that means the incoming mode
corresponds to the actual size of the memory being loaded/stored and the
mode of the corresponding addressing mode is half of that. */
@@ -9779,12 +9778,14 @@ aarch64_classify_address (struct aarch64_address_info *info,
mode = DFmode;
bool allow_reg_index_p = (!load_store_pair_p
- && (known_lt (GET_MODE_SIZE (mode), 16)
+ && ((vec_flags == 0
+ && known_lt (GET_MODE_SIZE (mode), 16))
|| vec_flags == VEC_ADVSIMD
|| vec_flags & VEC_SVE_DATA));
- /* For SVE, only accept [Rn], [Rn, Rm, LSL #shift] and
- [Rn, #offset, MUL VL]. */
+ /* For SVE, only accept [Rn], [Rn, #offset, MUL VL] and [Rn, Rm, LSL #shift].
+ The latter is not valid for SVE predicates, and that's rejected through
+ allow_reg_index_p above. */
if ((vec_flags & (VEC_SVE_DATA | VEC_SVE_PRED)) != 0
&& (code != REG && code != PLUS))
return false;
diff --git a/gcc/testsuite/g++.target/aarch64/sve/pr102252.C b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
new file mode 100644
index 0000000000000000000000000000000000000000..f90f1218555f0dfdb0253fe83c656ba03b1aac43
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/sve/pr102252.C
@@ -0,0 +1,37 @@
+/* PR target/102252. */
+/* { dg-do assemble { target aarch64_asm_sve_ok } } */
+/* { dg-options "-march=armv8.2-a+sve -msve-vector-bits=512" } */
+
+/* We used to generate invalid assembly for SVE predicate loads. */
+
+#include <arm_sve.h>
+
+class SimdBool
+{
+private:
+ typedef svbool_t simdInternalType_ __attribute__((arm_sve_vector_bits(512)));
+
+public:
+ SimdBool() {}
+
+ simdInternalType_ simdInternal_;
+
+};
+
+static svfloat32_t selectByMask(svfloat32_t a, SimdBool m) {
+ return svsel_f32(m.simdInternal_, a, svdup_f32(0.0));
+}
+
+struct s {
+ SimdBool array[1];
+};
+
+
+
+void foo(struct s* const work, int offset)
+{
+ svfloat32_t tz_S0;
+
+ tz_S0 = selectByMask(tz_S0, work->array[offset]);
+}
+
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-13 14:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 8:20 [PATCH] aarch64: PR target/102252 Invalid addressing mode for SVE load predicate Kyrylo Tkachov
2021-09-13 11:08 ` Richard Sandiford
2021-09-13 14:42 ` Kyrylo Tkachov
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).