RE: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
> -Original Message-
> From: Andre Vieira (lists)
> Sent: 03 February 2021 13:58
> To: Kyrylo Tkachov ; gcc-patches@gcc.gnu.org
> Cc: ja...@redhat.com
> Subject: Re: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
>
> Same patch applies cleanly on gcc-8, bootstrapped
> arm-none-linux-gnueabihf and ran regressions also clean.
>
> Can I also commit it to gcc-8?
Ok.
Thanks,
Kyrill
>
> Thanks,
> Andre
>
> On 02/02/2021 17:36, Kyrylo Tkachov wrote:
> >
> >> -Original Message-
> >> From: Andre Vieira (lists)
> >> Sent: 02 February 2021 17:27
> >> To: gcc-patches@gcc.gnu.org
> >> Cc: Kyrylo Tkachov ; ja...@redhat.com
> >> Subject: Re: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
> >>
> >> Hi,
> >>
> >> This is a gcc-9 backport of the PR97528 fix that has been applied to
> >> trunk and gcc-10.
> >> Bootstraped on arm-linux-gnueabihf and regression tested.
> >>
> >> OK for gcc-9 branch?
> > Ok.
> > Thanks,
> > Kyrill
> >
> >> 2021-02-02 Andre Vieira
> >>
> >> Backport from mainline
> >> 2020-11-20 Jakub Jelinek
> >>
> >> PR target/97528
> >> * config/arm/arm.c (neon_vector_mem_operand): For POST_MODIFY,
> >> require
> >> first POST_MODIFY operand is a REG and is equal to the first operand
> >> of PLUS.
> >>
> >> * gcc.target/arm/pr97528.c: New test.
> >>
> >> On 20/11/2020 11:25, Kyrylo Tkachov via Gcc-patches wrote:
> >>>> -Original Message-
> >>>> From: Jakub Jelinek
> >>>> Sent: 19 November 2020 18:57
> >>>> To: Richard Earnshaw ; Ramana
> >>>> Radhakrishnan ; Kyrylo Tkachov
> >>>>
> >>>> Cc: gcc-patches@gcc.gnu.org
> >>>> Subject: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
> >>>>
> >>>> Hi!
> >>>>
> >>>> The documentation for POST_MODIFY says:
> >>>> Currently, the compiler can only handle second operands of the
> >>>> form (plus (reg) (reg)) and (plus (reg) (const_int)), where
> >>>> the first operand of the PLUS has to be the same register as
> >>>> the first operand of the *_MODIFY.
> >>>> The following testcase ICEs, because combine just attempts to simplify
> >>>> things and ends up with
> >>>> (post_modify (reg1) (plus (mult (reg2) (const_int 4)) (reg1))
> >>>> but the target predicates accept it, because they only verify
> >>>> that POST_MODIFY's second operand is PLUS and the second operand
> >>>> of the PLUS is a REG.
> >>>>
> >>>> The following patch fixes this by performing further verification that
> >>>> the POST_MODIFY is in the form it should be.
> >>>>
> >>>> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk
> >>>> and release branches after a while?
> >>> Ok.
> >>> Thanks,
> >>> Kyrill
> >>>
> >>>> 2020-11-19 Jakub Jelinek
> >>>>
> >>>> PR target/97528
> >>>> * config/arm/arm.c (neon_vector_mem_operand): For
> >>>> POST_MODIFY, require
> >>>> first POST_MODIFY operand is a REG and is equal to the first operand
> >>>> of PLUS.
> >>>>
> >>>> * gcc.target/arm/pr97528.c: New test.
> >>>>
> >>>> --- gcc/config/arm/arm.c.jj 2020-11-13 19:00:46.729620560
> +0100
> >>>> +++ gcc/config/arm/arm.c 2020-11-18 17:05:44.656867343
> +0100
> >>>> @@ -13429,7 +13429,9 @@ neon_vector_mem_operand (rtx op, int
> typ
> >>>> /* Allow post-increment by register for VLDn */
> >>>> if (type == 2 && GET_CODE (ind) == POST_MODIFY
> >>>> && GET_CODE (XEXP (ind, 1)) == PLUS
> >>>> - && REG_P (XEXP (XEXP (ind, 1), 1)))
> >>>> + && REG_P (XEXP (XEXP (ind, 1), 1))
> >>>> + && REG_P (XEXP (ind, 0))
> >>>> + && rtx_equal_p (XEXP (ind, 0), XEXP (XEXP (ind, 1), 0)))
> >>>> return true;
> >>>>
> >>>> /* Match:
> >>>> --- gcc/testsuite/gcc.target/arm/pr97528.c.jj2020-11-18
> >>>> 17:09:58.195053288 +0100
> >>>> +++ gcc/testsuite/gcc.target/arm/pr97528.c 2020-11-18
> >>>> 17:09:47.839168237 +0100
> >>>> @@ -0,0 +1,28 @@
> >>>> +/* PR target/97528 */
> >>>> +/* { dg-do compile } */
> >>>> +/* { dg-require-effective-target arm_neon_ok } */
> >>>> +/* { dg-options "-O1" } */
> >>>> +/* { dg-add-options arm_neon } */
> >>>> +
> >>>> +#include
> >>>> +
> >>>> +typedef __simd64_int16_t T;
> >>>> +typedef __simd64_uint16_t U;
> >>>> +unsigned short c;
> >>>> +int d;
> >>>> +U e;
> >>>> +
> >>>> +void
> >>>> +foo (void)
> >>>> +{
> >>>> + unsigned short *dst = &c;
> >>>> + int g = d, b = 4;
> >>>> + U dc = e;
> >>>> + for (int h = 0; h < b; h++)
> >>>> +{
> >>>> + unsigned short *i = dst;
> >>>> + U j = dc;
> >>>> + vst1_s16 ((int16_t *) i, (T) j);
> >>>> + dst += g;
> >>>> +}
> >>>> +}
> >>>>
> >>>>
> >>>> Jakub
Re: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
Same patch applies cleanly on gcc-8, bootstrapped
arm-none-linux-gnueabihf and ran regressions also clean.
Can I also commit it to gcc-8?
Thanks,
Andre
On 02/02/2021 17:36, Kyrylo Tkachov wrote:
-Original Message-
From: Andre Vieira (lists)
Sent: 02 February 2021 17:27
To: gcc-patches@gcc.gnu.org
Cc: Kyrylo Tkachov ; ja...@redhat.com
Subject: Re: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
Hi,
This is a gcc-9 backport of the PR97528 fix that has been applied to
trunk and gcc-10.
Bootstraped on arm-linux-gnueabihf and regression tested.
OK for gcc-9 branch?
Ok.
Thanks,
Kyrill
2021-02-02 Andre Vieira
Backport from mainline
2020-11-20 Jakub Jelinek
PR target/97528
* config/arm/arm.c (neon_vector_mem_operand): For POST_MODIFY,
require
first POST_MODIFY operand is a REG and is equal to the first operand
of PLUS.
* gcc.target/arm/pr97528.c: New test.
On 20/11/2020 11:25, Kyrylo Tkachov via Gcc-patches wrote:
-Original Message-
From: Jakub Jelinek
Sent: 19 November 2020 18:57
To: Richard Earnshaw ; Ramana
Radhakrishnan ; Kyrylo Tkachov
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
Hi!
The documentation for POST_MODIFY says:
Currently, the compiler can only handle second operands of the
form (plus (reg) (reg)) and (plus (reg) (const_int)), where
the first operand of the PLUS has to be the same register as
the first operand of the *_MODIFY.
The following testcase ICEs, because combine just attempts to simplify
things and ends up with
(post_modify (reg1) (plus (mult (reg2) (const_int 4)) (reg1))
but the target predicates accept it, because they only verify
that POST_MODIFY's second operand is PLUS and the second operand
of the PLUS is a REG.
The following patch fixes this by performing further verification that
the POST_MODIFY is in the form it should be.
Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk
and release branches after a while?
Ok.
Thanks,
Kyrill
2020-11-19 Jakub Jelinek
PR target/97528
* config/arm/arm.c (neon_vector_mem_operand): For
POST_MODIFY, require
first POST_MODIFY operand is a REG and is equal to the first operand
of PLUS.
* gcc.target/arm/pr97528.c: New test.
--- gcc/config/arm/arm.c.jj 2020-11-13 19:00:46.729620560 +0100
+++ gcc/config/arm/arm.c2020-11-18 17:05:44.656867343 +0100
@@ -13429,7 +13429,9 @@ neon_vector_mem_operand (rtx op, int typ
/* Allow post-increment by register for VLDn */
if (type == 2 && GET_CODE (ind) == POST_MODIFY
&& GET_CODE (XEXP (ind, 1)) == PLUS
- && REG_P (XEXP (XEXP (ind, 1), 1)))
+ && REG_P (XEXP (XEXP (ind, 1), 1))
+ && REG_P (XEXP (ind, 0))
+ && rtx_equal_p (XEXP (ind, 0), XEXP (XEXP (ind, 1), 0)))
return true;
/* Match:
--- gcc/testsuite/gcc.target/arm/pr97528.c.jj 2020-11-18
17:09:58.195053288 +0100
+++ gcc/testsuite/gcc.target/arm/pr97528.c 2020-11-18
17:09:47.839168237 +0100
@@ -0,0 +1,28 @@
+/* PR target/97528 */
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-O1" } */
+/* { dg-add-options arm_neon } */
+
+#include
+
+typedef __simd64_int16_t T;
+typedef __simd64_uint16_t U;
+unsigned short c;
+int d;
+U e;
+
+void
+foo (void)
+{
+ unsigned short *dst = &c;
+ int g = d, b = 4;
+ U dc = e;
+ for (int h = 0; h < b; h++)
+{
+ unsigned short *i = dst;
+ U j = dc;
+ vst1_s16 ((int16_t *) i, (T) j);
+ dst += g;
+}
+}
Jakub
RE: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
> -Original Message-
> From: Andre Vieira (lists)
> Sent: 02 February 2021 17:27
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov ; ja...@redhat.com
> Subject: Re: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
>
> Hi,
>
> This is a gcc-9 backport of the PR97528 fix that has been applied to
> trunk and gcc-10.
> Bootstraped on arm-linux-gnueabihf and regression tested.
>
> OK for gcc-9 branch?
Ok.
Thanks,
Kyrill
>
> 2021-02-02 Andre Vieira
>
> Backport from mainline
> 2020-11-20 Jakub Jelinek
>
> PR target/97528
> * config/arm/arm.c (neon_vector_mem_operand): For POST_MODIFY,
> require
> first POST_MODIFY operand is a REG and is equal to the first operand
> of PLUS.
>
> * gcc.target/arm/pr97528.c: New test.
>
> On 20/11/2020 11:25, Kyrylo Tkachov via Gcc-patches wrote:
> >
> >> -Original Message-
> >> From: Jakub Jelinek
> >> Sent: 19 November 2020 18:57
> >> To: Richard Earnshaw ; Ramana
> >> Radhakrishnan ; Kyrylo Tkachov
> >>
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
> >>
> >> Hi!
> >>
> >> The documentation for POST_MODIFY says:
> >> Currently, the compiler can only handle second operands of the
> >> form (plus (reg) (reg)) and (plus (reg) (const_int)), where
> >> the first operand of the PLUS has to be the same register as
> >> the first operand of the *_MODIFY.
> >> The following testcase ICEs, because combine just attempts to simplify
> >> things and ends up with
> >> (post_modify (reg1) (plus (mult (reg2) (const_int 4)) (reg1))
> >> but the target predicates accept it, because they only verify
> >> that POST_MODIFY's second operand is PLUS and the second operand
> >> of the PLUS is a REG.
> >>
> >> The following patch fixes this by performing further verification that
> >> the POST_MODIFY is in the form it should be.
> >>
> >> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk
> >> and release branches after a while?
> > Ok.
> > Thanks,
> > Kyrill
> >
> >> 2020-11-19 Jakub Jelinek
> >>
> >>PR target/97528
> >>* config/arm/arm.c (neon_vector_mem_operand): For
> >> POST_MODIFY, require
> >>first POST_MODIFY operand is a REG and is equal to the first operand
> >>of PLUS.
> >>
> >>* gcc.target/arm/pr97528.c: New test.
> >>
> >> --- gcc/config/arm/arm.c.jj2020-11-13 19:00:46.729620560 +0100
> >> +++ gcc/config/arm/arm.c 2020-11-18 17:05:44.656867343 +0100
> >> @@ -13429,7 +13429,9 @@ neon_vector_mem_operand (rtx op, int typ
> >> /* Allow post-increment by register for VLDn */
> >> if (type == 2 && GET_CODE (ind) == POST_MODIFY
> >> && GET_CODE (XEXP (ind, 1)) == PLUS
> >> - && REG_P (XEXP (XEXP (ind, 1), 1)))
> >> + && REG_P (XEXP (XEXP (ind, 1), 1))
> >> + && REG_P (XEXP (ind, 0))
> >> + && rtx_equal_p (XEXP (ind, 0), XEXP (XEXP (ind, 1), 0)))
> >>return true;
> >>
> >> /* Match:
> >> --- gcc/testsuite/gcc.target/arm/pr97528.c.jj 2020-11-18
> >> 17:09:58.195053288 +0100
> >> +++ gcc/testsuite/gcc.target/arm/pr97528.c 2020-11-18
> >> 17:09:47.839168237 +0100
> >> @@ -0,0 +1,28 @@
> >> +/* PR target/97528 */
> >> +/* { dg-do compile } */
> >> +/* { dg-require-effective-target arm_neon_ok } */
> >> +/* { dg-options "-O1" } */
> >> +/* { dg-add-options arm_neon } */
> >> +
> >> +#include
> >> +
> >> +typedef __simd64_int16_t T;
> >> +typedef __simd64_uint16_t U;
> >> +unsigned short c;
> >> +int d;
> >> +U e;
> >> +
> >> +void
> >> +foo (void)
> >> +{
> >> + unsigned short *dst = &c;
> >> + int g = d, b = 4;
> >> + U dc = e;
> >> + for (int h = 0; h < b; h++)
> >> +{
> >> + unsigned short *i = dst;
> >> + U j = dc;
> >> + vst1_s16 ((int16_t *) i, (T) j);
> >> + dst += g;
> >> +}
> >> +}
> >>
> >>
> >>Jakub
Re: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
Hi,
This is a gcc-9 backport of the PR97528 fix that has been applied to
trunk and gcc-10.
Bootstraped on arm-linux-gnueabihf and regression tested.
OK for gcc-9 branch?
2021-02-02 Andre Vieira
Backport from mainline
2020-11-20 Jakub Jelinek
PR target/97528
* config/arm/arm.c (neon_vector_mem_operand): For POST_MODIFY, require
first POST_MODIFY operand is a REG and is equal to the first operand
of PLUS.
* gcc.target/arm/pr97528.c: New test.
On 20/11/2020 11:25, Kyrylo Tkachov via Gcc-patches wrote:
-Original Message-
From: Jakub Jelinek
Sent: 19 November 2020 18:57
To: Richard Earnshaw ; Ramana
Radhakrishnan ; Kyrylo Tkachov
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
Hi!
The documentation for POST_MODIFY says:
Currently, the compiler can only handle second operands of the
form (plus (reg) (reg)) and (plus (reg) (const_int)), where
the first operand of the PLUS has to be the same register as
the first operand of the *_MODIFY.
The following testcase ICEs, because combine just attempts to simplify
things and ends up with
(post_modify (reg1) (plus (mult (reg2) (const_int 4)) (reg1))
but the target predicates accept it, because they only verify
that POST_MODIFY's second operand is PLUS and the second operand
of the PLUS is a REG.
The following patch fixes this by performing further verification that
the POST_MODIFY is in the form it should be.
Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk
and release branches after a while?
Ok.
Thanks,
Kyrill
2020-11-19 Jakub Jelinek
PR target/97528
* config/arm/arm.c (neon_vector_mem_operand): For
POST_MODIFY, require
first POST_MODIFY operand is a REG and is equal to the first operand
of PLUS.
* gcc.target/arm/pr97528.c: New test.
--- gcc/config/arm/arm.c.jj 2020-11-13 19:00:46.729620560 +0100
+++ gcc/config/arm/arm.c2020-11-18 17:05:44.656867343 +0100
@@ -13429,7 +13429,9 @@ neon_vector_mem_operand (rtx op, int typ
/* Allow post-increment by register for VLDn */
if (type == 2 && GET_CODE (ind) == POST_MODIFY
&& GET_CODE (XEXP (ind, 1)) == PLUS
- && REG_P (XEXP (XEXP (ind, 1), 1)))
+ && REG_P (XEXP (XEXP (ind, 1), 1))
+ && REG_P (XEXP (ind, 0))
+ && rtx_equal_p (XEXP (ind, 0), XEXP (XEXP (ind, 1), 0)))
return true;
/* Match:
--- gcc/testsuite/gcc.target/arm/pr97528.c.jj 2020-11-18
17:09:58.195053288 +0100
+++ gcc/testsuite/gcc.target/arm/pr97528.c 2020-11-18
17:09:47.839168237 +0100
@@ -0,0 +1,28 @@
+/* PR target/97528 */
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-O1" } */
+/* { dg-add-options arm_neon } */
+
+#include
+
+typedef __simd64_int16_t T;
+typedef __simd64_uint16_t U;
+unsigned short c;
+int d;
+U e;
+
+void
+foo (void)
+{
+ unsigned short *dst = &c;
+ int g = d, b = 4;
+ U dc = e;
+ for (int h = 0; h < b; h++)
+{
+ unsigned short *i = dst;
+ U j = dc;
+ vst1_s16 ((int16_t *) i, (T) j);
+ dst += g;
+}
+}
Jakub
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
04edd637d43198ad801bb5ada8f1807faae7001e..4679da75dd823778d5a3e37c497ee10793e9c7d7
100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -12730,7 +12730,9 @@ neon_vector_mem_operand (rtx op, int type, bool strict)
/* Allow post-increment by register for VLDn */
if (type == 2 && GET_CODE (ind) == POST_MODIFY
&& GET_CODE (XEXP (ind, 1)) == PLUS
- && REG_P (XEXP (XEXP (ind, 1), 1)))
+ && REG_P (XEXP (XEXP (ind, 1), 1))
+ && REG_P (XEXP (ind, 0))
+ && rtx_equal_p (XEXP (ind, 0), XEXP (XEXP (ind, 1), 0)))
return true;
/* Match:
diff --git a/gcc/testsuite/gcc.target/arm/pr97528.c
b/gcc/testsuite/gcc.target/arm/pr97528.c
new file mode 100644
index
..6cc59f2158c5f8c8dd78e5083ca7ebc4e5f63a44
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr97528.c
@@ -0,0 +1,28 @@
+/* PR target/97528 */
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-O1" } */
+/* { dg-add-options arm_neon } */
+
+#include
+
+typedef __simd64_int16_t T;
+typedef __simd64_uint16_t U;
+unsigned short c;
+int d;
+U e;
+
+void
+foo (void)
+{
+ unsigned short *dst = &c;
+ int g = d, b = 4;
+ U dc = e;
+ for (int h = 0; h < b; h++)
+{
+ unsigned short *i = dst;
+ U j = dc;
+ vst1_s16 ((int16_t *) i, (T) j);
+ dst += g;
+}
+}
RE: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
> -Original Message-
> From: Jakub Jelinek
> Sent: 19 November 2020 18:57
> To: Richard Earnshaw ; Ramana
> Radhakrishnan ; Kyrylo Tkachov
>
> Cc: gcc-patches@gcc.gnu.org
> Subject: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
>
> Hi!
>
> The documentation for POST_MODIFY says:
>Currently, the compiler can only handle second operands of the
>form (plus (reg) (reg)) and (plus (reg) (const_int)), where
>the first operand of the PLUS has to be the same register as
>the first operand of the *_MODIFY.
> The following testcase ICEs, because combine just attempts to simplify
> things and ends up with
> (post_modify (reg1) (plus (mult (reg2) (const_int 4)) (reg1))
> but the target predicates accept it, because they only verify
> that POST_MODIFY's second operand is PLUS and the second operand
> of the PLUS is a REG.
>
> The following patch fixes this by performing further verification that
> the POST_MODIFY is in the form it should be.
>
> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk
> and release branches after a while?
Ok.
Thanks,
Kyrill
>
> 2020-11-19 Jakub Jelinek
>
> PR target/97528
> * config/arm/arm.c (neon_vector_mem_operand): For
> POST_MODIFY, require
> first POST_MODIFY operand is a REG and is equal to the first operand
> of PLUS.
>
> * gcc.target/arm/pr97528.c: New test.
>
> --- gcc/config/arm/arm.c.jj 2020-11-13 19:00:46.729620560 +0100
> +++ gcc/config/arm/arm.c 2020-11-18 17:05:44.656867343 +0100
> @@ -13429,7 +13429,9 @@ neon_vector_mem_operand (rtx op, int typ
>/* Allow post-increment by register for VLDn */
>if (type == 2 && GET_CODE (ind) == POST_MODIFY
>&& GET_CODE (XEXP (ind, 1)) == PLUS
> - && REG_P (XEXP (XEXP (ind, 1), 1)))
> + && REG_P (XEXP (XEXP (ind, 1), 1))
> + && REG_P (XEXP (ind, 0))
> + && rtx_equal_p (XEXP (ind, 0), XEXP (XEXP (ind, 1), 0)))
> return true;
>
>/* Match:
> --- gcc/testsuite/gcc.target/arm/pr97528.c.jj 2020-11-18
> 17:09:58.195053288 +0100
> +++ gcc/testsuite/gcc.target/arm/pr97528.c2020-11-18
> 17:09:47.839168237 +0100
> @@ -0,0 +1,28 @@
> +/* PR target/97528 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_neon_ok } */
> +/* { dg-options "-O1" } */
> +/* { dg-add-options arm_neon } */
> +
> +#include
> +
> +typedef __simd64_int16_t T;
> +typedef __simd64_uint16_t U;
> +unsigned short c;
> +int d;
> +U e;
> +
> +void
> +foo (void)
> +{
> + unsigned short *dst = &c;
> + int g = d, b = 4;
> + U dc = e;
> + for (int h = 0; h < b; h++)
> +{
> + unsigned short *i = dst;
> + U j = dc;
> + vst1_s16 ((int16_t *) i, (T) j);
> + dst += g;
> +}
> +}
>
>
> Jakub
