> -----Original Message-----
> From: Srinath Parvathaneni <srinath.parvathan...@arm.com>
> Sent: 17 June 2020 17:17
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov <kyrylo.tkac...@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.parvathan...@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;
> +}
