[PATCH] arm: Require MVE memory operand for destination of vst1q intrinsic

Ramana Radhakrishnan ramana.gcc@googlemail.com
Thu Aug 20 15:06:29 GMT 2020


On Thu, Aug 20, 2020 at 3:31 PM Joe Ramsay <Joe.Ramsay@arm.com> wrote:
>
> Hi Ramana,
>
> Thanks for the review.
>
> On 18/08/2020, 18:37, "Ramana Radhakrishnan" <ramana.gcc@googlemail.com> wrote:
>
>     On Thu, Aug 13, 2020 at 2:18 PM Joe Ramsay <joe.ramsay@arm.com> wrote:
>     >
>     > From: Joe Ramsay <Joe.Ramsay@arm.com>
>     >
>     > Hi,
>     >
>     > Previously, the machine description patterns for vst1q accepted a generic memory
>     > operand for the destination, which could lead to an unrecognised builtin when
>     > expanding vst1q* intrinsics. This change fixes the patterns to only accept MVE
>     > memory operands.
>
>     This is OK though I suspect this needs a PR and a backport request for GCC 10.
>
> There's now a PR for this, 96683. I've attached an updated patch file, the only change is
> that I've included the PR number in the changelog. Please let me know if this is OK for
> trunk.

Yep absolutely fine - FTR such fixes to changelogs with PR numbers and
administrativia count as obvious and can just be applied.

Ramana

>
> Thanks,
> Joe
>
>     regards
>     Ramana
>
>     >
>     > Thanks,
>     > Joe
>     >
>     > gcc/ChangeLog:
>     >
>     > 2020-08-13  Joe Ramsay <joe.ramsay@.arm.com>
>     >
>     >         * config/arm/mve.md (mve_vst1q_f<mode>): Require MVE memory operand for
>     >         destination.
>     >         (mve_vst1q_<supf><mode>): Likewise.
>     >
>     > gcc/testsuite/ChangeLog:
>     >
>     > 2020-08-13  Joe Ramsay <joe.ramsay@.arm.com>
>     >
>     >         * gcc.target/arm/mve/intrinsics/vst1q_f16.c: Add test that only MVE
>     >         memory operand is accepted.
>     >         * gcc.target/arm/mve/intrinsics/vst1q_s16.c: Likewise.
>     >         * gcc.target/arm/mve/intrinsics/vst1q_s8.c: Likewise.
>     >         * gcc.target/arm/mve/intrinsics/vst1q_u16.c: Likewise.
>     >         * gcc.target/arm/mve/intrinsics/vst1q_u8.c: Likewise.
>     > ---
>     >  gcc/config/arm/mve.md                                   |  4 ++--
>     >  gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_f16.c | 10 +++++++---
>     >  gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s16.c | 10 +++++++---
>     >  gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s8.c  | 10 +++++++---
>     >  gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u16.c | 10 +++++++---
>     >  gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u8.c  | 10 +++++++---
>     >  6 files changed, 37 insertions(+), 17 deletions(-)
>     >
>     > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md
>     > index 9758862..465b39a 100644
>     > --- a/gcc/config/arm/mve.md
>     > +++ b/gcc/config/arm/mve.md
>     > @@ -9330,7 +9330,7 @@
>     >    [(set_attr "length" "4")])
>     >
>     >  (define_expand "mve_vst1q_f<mode>"
>     > -  [(match_operand:<MVE_CNVT> 0 "memory_operand")
>     > +  [(match_operand:<MVE_CNVT> 0 "mve_memory_operand")
>     >     (unspec:<MVE_CNVT> [(match_operand:MVE_0 1 "s_register_operand")] VST1Q_F)
>     >    ]
>     >    "TARGET_HAVE_MVE || TARGET_HAVE_MVE_FLOAT"
>     > @@ -9340,7 +9340,7 @@
>     >  })
>     >
>     >  (define_expand "mve_vst1q_<supf><mode>"
>     > -  [(match_operand:MVE_2 0 "memory_operand")
>     > +  [(match_operand:MVE_2 0 "mve_memory_operand")
>     >     (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand")] VST1Q)
>     >    ]
>     >    "TARGET_HAVE_MVE"
>     > diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_f16.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_f16.c
>     > index 363b4ca..312b746 100644
>     > --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_f16.c
>     > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_f16.c
>     > @@ -10,12 +10,16 @@ foo (float16_t * addr, float16x8_t value)
>     >    vst1q_f16 (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrh.16"  }  } */
>     > -
>     >  void
>     >  foo1 (float16_t * addr, float16x8_t value)
>     >  {
>     >    vst1q (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrh.16"  }  } */
>     > +/* { dg-final { scan-assembler-times "vstrh.16" 2 }  } */
>     > +
>     > +void
>     > +foo2 (float16_t a, float16x8_t x)
>     > +{
>     > +  vst1q (&a, x);
>     > +}
>     > diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s16.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s16.c
>     > index 37c4713..cd14e2c 100644
>     > --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s16.c
>     > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s16.c
>     > @@ -10,12 +10,16 @@ foo (int16_t * addr, int16x8_t value)
>     >    vst1q_s16 (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrh.16"  }  } */
>     > -
>     >  void
>     >  foo1 (int16_t * addr, int16x8_t value)
>     >  {
>     >    vst1q (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrh.16"  }  } */
>     > +/* { dg-final { scan-assembler-times "vstrh.16" 2 }  } */
>     > +
>     > +void
>     > +foo2 (int16_t a, int16x8_t x)
>     > +{
>     > +  vst1q (&a, x);
>     > +}
>     > diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s8.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s8.c
>     > index fe5edea..0004c80 100644
>     > --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s8.c
>     > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_s8.c
>     > @@ -10,12 +10,16 @@ foo (int8_t * addr, int8x16_t value)
>     >    vst1q_s8 (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrb.8"  }  } */
>     > -
>     >  void
>     >  foo1 (int8_t * addr, int8x16_t value)
>     >  {
>     >    vst1q (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrb.8"  }  } */
>     > +/* { dg-final { scan-assembler-times "vstrb.8" 2 }  } */
>     > +
>     > +void
>     > +foo2 (int8_t a, int8x16_t x)
>     > +{
>     > +  vst1q (&a, x);
>     > +}
>     > diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u16.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u16.c
>     > index a4c8c1a..248e7ce 100644
>     > --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u16.c
>     > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u16.c
>     > @@ -10,12 +10,16 @@ foo (uint16_t * addr, uint16x8_t value)
>     >    vst1q_u16 (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrh.16"  }  } */
>     > -
>     >  void
>     >  foo1 (uint16_t * addr, uint16x8_t value)
>     >  {
>     >    vst1q (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrh.16"  }  } */
>     > +/* { dg-final { scan-assembler-times "vstrh.16" 2 }  } */
>     > +
>     > +void
>     > +foo2 (uint16_t a, uint16x8_t x)
>     > +{
>     > +  vst1q (&a, x);
>     > +}
>     > diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u8.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u8.c
>     > index bf20b6d..f8b48a6 100644
>     > --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u8.c
>     > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/vst1q_u8.c
>     > @@ -10,12 +10,16 @@ foo (uint8_t * addr, uint8x16_t value)
>     >    vst1q_u8 (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrb.8"  }  } */
>     > -
>     >  void
>     >  foo1 (uint8_t * addr, uint8x16_t value)
>     >  {
>     >    vst1q (addr, value);
>     >  }
>     >
>     > -/* { dg-final { scan-assembler "vstrb.8"  }  } */
>     > +/* { dg-final { scan-assembler-times "vstrb.8" 2 }  } */
>     > +
>     > +void
>     > +foo2 (uint8_t a, uint8x16_t x)
>     > +{
>     > +  vst1q (&a, x);
>     > +}
>     > --
>     > 2.7.4
>     >
>


More information about the Gcc-patches mailing list