[PATCH][GCC-10 Backport] arm: Fix MVE scalar shift intrinsics code-gen.
Christophe Lyon
christophe.lyon@linaro.org
Thu Jun 18 15:44:18 GMT 2020
On Thu, 18 Jun 2020 at 17:34, Srinath Parvathaneni
<Srinath.Parvathaneni@arm.com> wrote:
>
> Hi,
>
> > -----Original Message-----
> > From: Christophe Lyon <christophe.lyon@linaro.org>
> > Sent: 18 June 2020 16:06
> > To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> > Cc: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>; gcc-
> > patches@gcc.gnu.org
> > Subject: Re: [PATCH][GCC-10 Backport] arm: Fix MVE scalar shift intrinsics
> > code-gen.
> >
> > Hi,
> >
> > On Thu, 18 Jun 2020 at 11:43, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> > wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> > > > Sent: 17 June 2020 17:17
> > > > To: gcc-patches@gcc.gnu.org
> > > > Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> > > > Subject: [PATCH][GCC-10 Backport] arm: Fix MVE scalar shift
> > > > intrinsics code- gen.
> > > >
> > > > Hello,
> > > >
> > > > This patch modifies the MVE scalar shift RTL patterns. The current
> > > > patterns have wrong constraints and predicates due to which the
> > > > values returned from MVE scalar shift instructions are overwritten
> > > > in the code-gen.
> > > >
> > > > example:
> > > > $ cat x.c
> > > > int32_t foo(int64_t acc, int shift) {
> > > > return sqrshrl_sat48 (acc, shift); }
> > > >
> > > > Code-gen before applying this patch:
> > > > $ arm-none-eabi-gcc -march=armv8.1-m.main+mve -mfloat-abi=hard -O2
> > > > -S $ cat x.s
> > > > foo:
> > > > push {r4, r5}
> > > > sqrshrl r0, r1, #48, r2 ----> (a)
> > > > mov r0, r4 ----> (b)
> > > > pop {r4, r5}
> > > > bx lr
> > > >
> > > > Code-gen after applying this patch:
> > > > foo:
> > > > sqrshrl r0, r1, #48, r2
> > > > bx lr
> > > >
> > > > In the current compiler the return value (r0) from sqrshrl (a) is
> > > > getting overwritten by the mov statement (b).
> > > > This patch fixes above issue.
> > > >
> > > > Regression tested on arm-none-eabi and found no regressions.
> > > >
> > > > Ok for gcc-10 branch?
> > > >
> > >
> > > Ok.
> > > Thanks,
> > > Kyrill
> > >
> > > > Thanks,
> > > > Srinath.
> > > >
> > > > 2020-06-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
> > > >
> > > > gcc/
> > > > * config/arm/mve.md (mve_uqrshll_sat<supf>_di): Correct the
> > > > predicate
> > > > and constraint of all the operands.
> > > > (mve_sqrshrl_sat<supf>_di): Likewise.
> > > > (mve_uqrshl_si): Likewise.
> > > > (mve_sqrshr_si): Likewise.
> > > > (mve_uqshll_di): Likewise.
> > > > (mve_urshrl_di): Likewise.
> > > > (mve_uqshl_si): Likewise.
> > > > (mve_urshr_si): Likewise.
> > > > (mve_sqshl_si): Likewise.
> > > > (mve_srshr_si): Likewise.
> > > > (mve_srshrl_di): Likewise.
> > > > (mve_sqshll_di): Likewise.
> > > > * config/arm/predicates.md (arm_low_register_operand): Define.
> > > >
> > > > gcc/testsuite/
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts1.c: New test.
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts2.c: Likewise.
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts3.c: Likewise.
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts4.c: Likewise.
> > > >
> >
> > These new tests fails to link on arm-linux-gnueabi* targets with no m-profile
> > multilibs:
> > FAIL: gcc.target/arm/mve/intrinsics/mve_scalar_shifts1.c (test for excess
> > errors) Excess errors:
> > /aci-gcc-fsf/builds/gcc-fsf-gccsrc-thumb/tools/arm-none-linux-
> > gnueabi/bin/ld:
> > error: ./mve_scalar_shifts1.o: conflicting architecture profiles M/A
> > /aci-gcc-fsf/builds/gcc-fsf-gccsrc-thumb/tools/arm-none-linux-
> > gnueabi/bin/ld:
> > failed to merge target specific data of file ./mve_scalar_shifts1.o
> >
> > probably because arm_v8_1m_mve_ok only checks that compilation is OK
> > and does not try to link
> >
> > (same applies to the trunk commit)
>
> Following patch is posted to fix above failures.
> https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548507.html
>
I thought that one was meant to make sure the right HW or simulator is
available to run the test.
There are other cases where the tests are compile-only when
HW/simulator is not available.
> Regards,
> SRI.
> >
> > Christophe
> >
> >
> > > > (cherry picked from commit
> > 6af598703f919b56f628c496843cdfe6f0cb8276)
> > > >
> > > >
> > > > ############### Attachment also inlined for ease of reply
> > > > ###############
> > > >
> > > >
> > > > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md index
> > > >
> > 3a57901bd5bcd770832d59dc77cd92b6d9b5ecb4..9758862ac2bb40805dc5b6
> > > > 6c9b05466fffcf91df 100644
> > > > --- a/gcc/config/arm/mve.md
> > > > +++ b/gcc/config/arm/mve.md
> > > > @@ -11344,9 +11344,9 @@
> > > > ;; [uqrshll_di]
> > > > ;;
> > > > (define_insn "mve_uqrshll_sat<supf>_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > UQRSHLLQ))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqrshll%?\\t%Q1, %R1, #<supf>, %2"
> > > > @@ -11356,9 +11356,9 @@
> > > > ;; [sqrshrl_di]
> > > > ;;
> > > > (define_insn "mve_sqrshrl_sat<supf>_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > SQRSHRLQ))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqrshrl%?\\t%Q1, %R1, #<supf>, %2"
> > > > @@ -11368,9 +11368,9 @@
> > > > ;; [uqrshl_si]
> > > > ;;
> > > > (define_insn "mve_uqrshl_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > UQRSHL))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqrshl%?\\t%1, %2"
> > > > @@ -11380,9 +11380,9 @@
> > > > ;; [sqrshr_si]
> > > > ;;
> > > > (define_insn "mve_sqrshr_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > SQRSHR))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqrshr%?\\t%1, %2"
> > > > @@ -11392,9 +11392,9 @@
> > > > ;; [uqshll_di]
> > > > ;;
> > > > (define_insn "mve_uqshll_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (us_ashift:DI (match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (us_ashift:DI (match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqshll%?\\t%Q1, %R1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > @@ -11403,9 +11403,9 @@
> > > > ;; [urshrl_di]
> > > > ;;
> > > > (define_insn "mve_urshrl_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > URSHRL))]
> > > > "TARGET_HAVE_MVE"
> > > > "urshrl%?\\t%Q1, %R1, %2"
> > > > @@ -11415,9 +11415,9 @@
> > > > ;; [uqshl_si]
> > > > ;;
> > > > (define_insn "mve_uqshl_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (us_ashift:SI (match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (us_ashift:SI (match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqshl%?\\t%1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > @@ -11426,9 +11426,9 @@
> > > > ;; [urshr_si]
> > > > ;;
> > > > (define_insn "mve_urshr_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > URSHR))]
> > > > "TARGET_HAVE_MVE"
> > > > "urshr%?\\t%1, %2"
> > > > @@ -11438,9 +11438,9 @@
> > > > ;; [sqshl_si]
> > > > ;;
> > > > (define_insn "mve_sqshl_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (ss_ashift:SI (match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (ss_ashift:SI (match_operand:DI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqshl%?\\t%1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > @@ -11449,9 +11449,9 @@
> > > > ;; [srshr_si]
> > > > ;;
> > > > (define_insn "mve_srshr_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > SRSHR))]
> > > > "TARGET_HAVE_MVE"
> > > > "srshr%?\\t%1, %2"
> > > > @@ -11461,9 +11461,9 @@
> > > > ;; [srshrl_di]
> > > > ;;
> > > > (define_insn "mve_srshrl_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > SRSHRL))]
> > > > "TARGET_HAVE_MVE"
> > > > "srshrl%?\\t%Q1, %R1, %2"
> > > > @@ -11473,9 +11473,9 @@
> > > > ;; [sqshll_di]
> > > > ;;
> > > > (define_insn "mve_sqshll_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (ss_ashift:DI (match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (ss_ashift:DI (match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqshll%?\\t%Q1, %R1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > diff --git a/gcc/config/arm/predicates.md
> > > > b/gcc/config/arm/predicates.md index
> > > >
> > 9e9bca4d87fdc31e045b2b5bb03b996f082079bd..371b43cd86115565f8abf2e
> > > > 91383f7012b87f390 100644
> > > > --- a/gcc/config/arm/predicates.md
> > > > +++ b/gcc/config/arm/predicates.md
> > > > @@ -155,6 +155,18 @@
> > > > || REGNO (op) >= FIRST_PSEUDO_REGISTER));
> > > > })
> > > >
> > > > +;; Low core register, or any pseudo.
> > > > +(define_predicate "arm_low_register_operand"
> > > > + (match_code "reg,subreg")
> > > > +{
> > > > + if (GET_CODE (op) == SUBREG)
> > > > + op = SUBREG_REG (op);
> > > > +
> > > > + return (REG_P (op)
> > > > + && (REGNO (op) <= LAST_LO_REGNUM
> > > > + || REGNO (op) >= FIRST_PSEUDO_REGISTER));
> > > > +})
> > > > +
> > > > (define_predicate "arm_general_adddi_operand"
> > > > (ior (match_operand 0 "arm_general_register_operand")
> > > > (and (match_code "const_int") diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts1.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts1.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..e1c136e7f302c1824f0b00b
> > > > 5e7bc468ff5fcfe27
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts1
> > > > +++ .c
> > > > @@ -0,0 +1,40 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include "stdio.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +void
> > > > +foo (int64_t acc, int shift)
> > > > +{
> > > > + acc = sqrshrl_sat48 (acc, shift);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc = sqrshrl (acc, shift);
> > > > + if (acc != 2)
> > > > + abort();
> > > > +}
> > > > +
> > > > +void
> > > > +foo1 (uint64_t acc, int shift)
> > > > +{
> > > > + acc = uqrshll_sat48 (acc, shift);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc = uqrshll (acc, shift);
> > > > + if (acc != 128)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int64_t acc = 128;
> > > > + uint64_t acc1 = 2;
> > > > + int shift = 3;
> > > > + foo (acc, shift);
> > > > + foo1 (acc1, shift);
> > > > + return 0;
> > > > +}
> > > > diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts2.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts2.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..0b5a8edb15849913d4f2849
> > > > ab86decb286692386
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts2
> > > > +++ .c
> > > > @@ -0,0 +1,35 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include "stdio.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +#define IMM 3
> > > > +
> > > > +void
> > > > +foo (int64_t acc, uint64_t acc1)
> > > > +{
> > > > + acc = sqshll (acc, IMM);
> > > > + if (acc != 128)
> > > > + abort();
> > > > + acc = srshrl (acc, IMM);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc1 = uqshll (acc1, IMM);
> > > > + if (acc1 != 128)
> > > > + abort();
> > > > + acc1 = urshrl (acc1, IMM);
> > > > + if (acc1 != 16)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int64_t acc = 16;
> > > > + uint64_t acc1 = 16;
> > > > + foo (acc, acc1);
> > > > + return 0;
> > > > +}
> > > > diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts3.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts3.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..7e3da54f5e62467e2cdaabd
> > > > 85df3db127f608802
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts3
> > > > +++ .c
> > > > @@ -0,0 +1,28 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include "stdio.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +void
> > > > +foo (int32_t acc, uint32_t acc1, int shift) {
> > > > + acc = sqrshr (acc, shift);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc1 = uqrshl (acc1, shift);
> > > > + if (acc1 != 128)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int32_t acc = 128;
> > > > + uint32_t acc1 = 16;
> > > > + int shift = 3;
> > > > + foo (acc, acc1, shift);
> > > > + return 0;
> > > > +}
> > > > diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts4.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts4.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..8bee12f7fdfaef51daf7e2f5
> > > > d3c1e284115d2649
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts4
> > > > +++ .c
> > > > @@ -0,0 +1,34 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +#define IMM 3
> > > > +
> > > > +void
> > > > +foo (int32_t acc, uint32_t acc1)
> > > > +{
> > > > + acc = sqshl (acc, IMM);
> > > > + if (acc != 128)
> > > > + abort();
> > > > + acc = srshr (acc, IMM);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc1 = uqshl (acc1, IMM);
> > > > + if (acc1 != 128)
> > > > + abort();
> > > > + acc1 = urshr (acc1, IMM);
> > > > + if (acc1 != 16)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int32_t acc = 16;
> > > > + uint32_t acc1 = 16;
> > > > + foo (acc, acc1);
> > > > + return 0;
> > > > +}
> > >
> >
> > On Thu, 18 Jun 2020 at 11:43, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> > wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> > > > Sent: 17 June 2020 17:17
> > > > To: gcc-patches@gcc.gnu.org
> > > > Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> > > > Subject: [PATCH][GCC-10 Backport] arm: Fix MVE scalar shift
> > > > intrinsics code- gen.
> > > >
> > > > Hello,
> > > >
> > > > This patch modifies the MVE scalar shift RTL patterns. The current
> > > > patterns have wrong constraints and predicates due to which the
> > > > values returned from MVE scalar shift instructions are overwritten
> > > > in the code-gen.
> > > >
> > > > example:
> > > > $ cat x.c
> > > > int32_t foo(int64_t acc, int shift) {
> > > > return sqrshrl_sat48 (acc, shift); }
> > > >
> > > > Code-gen before applying this patch:
> > > > $ arm-none-eabi-gcc -march=armv8.1-m.main+mve -mfloat-abi=hard -O2
> > > > -S $ cat x.s
> > > > foo:
> > > > push {r4, r5}
> > > > sqrshrl r0, r1, #48, r2 ----> (a)
> > > > mov r0, r4 ----> (b)
> > > > pop {r4, r5}
> > > > bx lr
> > > >
> > > > Code-gen after applying this patch:
> > > > foo:
> > > > sqrshrl r0, r1, #48, r2
> > > > bx lr
> > > >
> > > > In the current compiler the return value (r0) from sqrshrl (a) is
> > > > getting overwritten by the mov statement (b).
> > > > This patch fixes above issue.
> > > >
> > > > Regression tested on arm-none-eabi and found no regressions.
> > > >
> > > > Ok for gcc-10 branch?
> > > >
> > >
> > > Ok.
> > > Thanks,
> > > Kyrill
> > >
> > > > Thanks,
> > > > Srinath.
> > > >
> > > > 2020-06-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com>
> > > >
> > > > gcc/
> > > > * config/arm/mve.md (mve_uqrshll_sat<supf>_di): Correct the
> > > > predicate
> > > > and constraint of all the operands.
> > > > (mve_sqrshrl_sat<supf>_di): Likewise.
> > > > (mve_uqrshl_si): Likewise.
> > > > (mve_sqrshr_si): Likewise.
> > > > (mve_uqshll_di): Likewise.
> > > > (mve_urshrl_di): Likewise.
> > > > (mve_uqshl_si): Likewise.
> > > > (mve_urshr_si): Likewise.
> > > > (mve_sqshl_si): Likewise.
> > > > (mve_srshr_si): Likewise.
> > > > (mve_srshrl_di): Likewise.
> > > > (mve_sqshll_di): Likewise.
> > > > * config/arm/predicates.md (arm_low_register_operand): Define.
> > > >
> > > > gcc/testsuite/
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts1.c: New test.
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts2.c: Likewise.
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts3.c: Likewise.
> > > > * gcc.target/arm/mve/intrinsics/mve_scalar_shifts4.c: Likewise.
> > > >
> > > > (cherry picked from commit
> > 6af598703f919b56f628c496843cdfe6f0cb8276)
> > > >
> > > >
> > > > ############### Attachment also inlined for ease of reply
> > > > ###############
> > > >
> > > >
> > > > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md index
> > > >
> > 3a57901bd5bcd770832d59dc77cd92b6d9b5ecb4..9758862ac2bb40805dc5b6
> > > > 6c9b05466fffcf91df 100644
> > > > --- a/gcc/config/arm/mve.md
> > > > +++ b/gcc/config/arm/mve.md
> > > > @@ -11344,9 +11344,9 @@
> > > > ;; [uqrshll_di]
> > > > ;;
> > > > (define_insn "mve_uqrshll_sat<supf>_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > UQRSHLLQ))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqrshll%?\\t%Q1, %R1, #<supf>, %2"
> > > > @@ -11356,9 +11356,9 @@
> > > > ;; [sqrshrl_di]
> > > > ;;
> > > > (define_insn "mve_sqrshrl_sat<supf>_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > SQRSHRLQ))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqrshrl%?\\t%Q1, %R1, #<supf>, %2"
> > > > @@ -11368,9 +11368,9 @@
> > > > ;; [uqrshl_si]
> > > > ;;
> > > > (define_insn "mve_uqrshl_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > UQRSHL))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqrshl%?\\t%1, %2"
> > > > @@ -11380,9 +11380,9 @@
> > > > ;; [sqrshr_si]
> > > > ;;
> > > > (define_insn "mve_sqrshr_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "s_register_operand" "r")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "register_operand" "r")]
> > > > SQRSHR))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqrshr%?\\t%1, %2"
> > > > @@ -11392,9 +11392,9 @@
> > > > ;; [uqshll_di]
> > > > ;;
> > > > (define_insn "mve_uqshll_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (us_ashift:DI (match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (us_ashift:DI (match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqshll%?\\t%Q1, %R1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > @@ -11403,9 +11403,9 @@
> > > > ;; [urshrl_di]
> > > > ;;
> > > > (define_insn "mve_urshrl_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > URSHRL))]
> > > > "TARGET_HAVE_MVE"
> > > > "urshrl%?\\t%Q1, %R1, %2"
> > > > @@ -11415,9 +11415,9 @@
> > > > ;; [uqshl_si]
> > > > ;;
> > > > (define_insn "mve_uqshl_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (us_ashift:SI (match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (us_ashift:SI (match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "uqshl%?\\t%1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > @@ -11426,9 +11426,9 @@
> > > > ;; [urshr_si]
> > > > ;;
> > > > (define_insn "mve_urshr_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:SI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > URSHR))]
> > > > "TARGET_HAVE_MVE"
> > > > "urshr%?\\t%1, %2"
> > > > @@ -11438,9 +11438,9 @@
> > > > ;; [sqshl_si]
> > > > ;;
> > > > (define_insn "mve_sqshl_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (ss_ashift:SI (match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (ss_ashift:SI (match_operand:DI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqshl%?\\t%1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > @@ -11449,9 +11449,9 @@
> > > > ;; [srshr_si]
> > > > ;;
> > > > (define_insn "mve_srshr_si"
> > > > - [(set (match_operand:SI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:SI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:SI 0 "arm_general_register_operand" "=r")
> > > > + (unspec:SI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > SRSHR))]
> > > > "TARGET_HAVE_MVE"
> > > > "srshr%?\\t%1, %2"
> > > > @@ -11461,9 +11461,9 @@
> > > > ;; [srshrl_di]
> > > > ;;
> > > > (define_insn "mve_srshrl_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (unspec:DI [(match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (unspec:DI [(match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")]
> > > > SRSHRL))]
> > > > "TARGET_HAVE_MVE"
> > > > "srshrl%?\\t%Q1, %R1, %2"
> > > > @@ -11473,9 +11473,9 @@
> > > > ;; [sqshll_di]
> > > > ;;
> > > > (define_insn "mve_sqshll_di"
> > > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
> > > > - (ss_ashift:DI (match_operand:DI 1 "arm_general_register_operand"
> > > > "r")
> > > > - (match_operand:SI 2 "arm_reg_or_long_shift_imm"
> > > > "rPg")))]
> > > > + [(set (match_operand:DI 0 "arm_low_register_operand" "=l")
> > > > + (ss_ashift:DI (match_operand:DI 1 "arm_low_register_operand" "0")
> > > > + (match_operand:SI 2 "immediate_operand" "Pg")))]
> > > > "TARGET_HAVE_MVE"
> > > > "sqshll%?\\t%Q1, %R1, %2"
> > > > [(set_attr "predicable" "yes")])
> > > > diff --git a/gcc/config/arm/predicates.md
> > > > b/gcc/config/arm/predicates.md index
> > > >
> > 9e9bca4d87fdc31e045b2b5bb03b996f082079bd..371b43cd86115565f8abf2e
> > > > 91383f7012b87f390 100644
> > > > --- a/gcc/config/arm/predicates.md
> > > > +++ b/gcc/config/arm/predicates.md
> > > > @@ -155,6 +155,18 @@
> > > > || REGNO (op) >= FIRST_PSEUDO_REGISTER));
> > > > })
> > > >
> > > > +;; Low core register, or any pseudo.
> > > > +(define_predicate "arm_low_register_operand"
> > > > + (match_code "reg,subreg")
> > > > +{
> > > > + if (GET_CODE (op) == SUBREG)
> > > > + op = SUBREG_REG (op);
> > > > +
> > > > + return (REG_P (op)
> > > > + && (REGNO (op) <= LAST_LO_REGNUM
> > > > + || REGNO (op) >= FIRST_PSEUDO_REGISTER));
> > > > +})
> > > > +
> > > > (define_predicate "arm_general_adddi_operand"
> > > > (ior (match_operand 0 "arm_general_register_operand")
> > > > (and (match_code "const_int") diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts1.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts1.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..e1c136e7f302c1824f0b00b
> > > > 5e7bc468ff5fcfe27
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts1
> > > > +++ .c
> > > > @@ -0,0 +1,40 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include "stdio.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +void
> > > > +foo (int64_t acc, int shift)
> > > > +{
> > > > + acc = sqrshrl_sat48 (acc, shift);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc = sqrshrl (acc, shift);
> > > > + if (acc != 2)
> > > > + abort();
> > > > +}
> > > > +
> > > > +void
> > > > +foo1 (uint64_t acc, int shift)
> > > > +{
> > > > + acc = uqrshll_sat48 (acc, shift);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc = uqrshll (acc, shift);
> > > > + if (acc != 128)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int64_t acc = 128;
> > > > + uint64_t acc1 = 2;
> > > > + int shift = 3;
> > > > + foo (acc, shift);
> > > > + foo1 (acc1, shift);
> > > > + return 0;
> > > > +}
> > > > diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts2.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts2.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..0b5a8edb15849913d4f2849
> > > > ab86decb286692386
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts2
> > > > +++ .c
> > > > @@ -0,0 +1,35 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include "stdio.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +#define IMM 3
> > > > +
> > > > +void
> > > > +foo (int64_t acc, uint64_t acc1)
> > > > +{
> > > > + acc = sqshll (acc, IMM);
> > > > + if (acc != 128)
> > > > + abort();
> > > > + acc = srshrl (acc, IMM);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc1 = uqshll (acc1, IMM);
> > > > + if (acc1 != 128)
> > > > + abort();
> > > > + acc1 = urshrl (acc1, IMM);
> > > > + if (acc1 != 16)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int64_t acc = 16;
> > > > + uint64_t acc1 = 16;
> > > > + foo (acc, acc1);
> > > > + return 0;
> > > > +}
> > > > diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts3.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts3.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..7e3da54f5e62467e2cdaabd
> > > > 85df3db127f608802
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts3
> > > > +++ .c
> > > > @@ -0,0 +1,28 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include "stdio.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +void
> > > > +foo (int32_t acc, uint32_t acc1, int shift) {
> > > > + acc = sqrshr (acc, shift);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc1 = uqrshl (acc1, shift);
> > > > + if (acc1 != 128)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int32_t acc = 128;
> > > > + uint32_t acc1 = 16;
> > > > + int shift = 3;
> > > > + foo (acc, acc1, shift);
> > > > + return 0;
> > > > +}
> > > > diff --git
> > > > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts4.c
> > > > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts4.c
> > > > new file mode 100644
> > > > index
> > > >
> > 0000000000000000000000000000000000000000..8bee12f7fdfaef51daf7e2f5
> > > > d3c1e284115d2649
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_scalar_shifts4
> > > > +++ .c
> > > > @@ -0,0 +1,34 @@
> > > > +/* { dg-do run } */
> > > > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> > > > +/* { dg-options "-O2" } */
> > > > +/* { dg-add-options arm_v8_1m_mve } */
> > > > +
> > > > +#include "arm_mve.h"
> > > > +#include <stdlib.h>
> > > > +
> > > > +#define IMM 3
> > > > +
> > > > +void
> > > > +foo (int32_t acc, uint32_t acc1)
> > > > +{
> > > > + acc = sqshl (acc, IMM);
> > > > + if (acc != 128)
> > > > + abort();
> > > > + acc = srshr (acc, IMM);
> > > > + if (acc != 16)
> > > > + abort();
> > > > + acc1 = uqshl (acc1, IMM);
> > > > + if (acc1 != 128)
> > > > + abort();
> > > > + acc1 = urshr (acc1, IMM);
> > > > + if (acc1 != 16)
> > > > + abort();
> > > > +}
> > > > +
> > > > +int main()
> > > > +{
> > > > + int32_t acc = 16;
> > > > + uint32_t acc1 = 16;
> > > > + foo (acc, acc1);
> > > > + return 0;
> > > > +}
> > >
More information about the Gcc-patches
mailing list