On 05/08/2020 10:28, Joe Ramsay wrote:
> From: Joe Ramsay <joe.ram...@arm.com>
>
> Hi,
>
> There was previously no way to specify that a register operand cannot
> have any writeback modifiers, and as a result the argument to vldr.16
> and vstr.16 could be erroneously output with post-increment. This
> change adds a constraint which forbids all writeback, and
> selects it in the relevant case for vldr.16 and vstr.16
>
> Bootstrapped on arm-none-eabi. Patch backports cleanly onto gcc-10
> branch with no regressions. OK for gcc-10 branch?
>
OK.
R.
> Thanks,
> Joe
>
> gcc/ChangeLog:
>
> 2020-08-04 Joe Ramsay <joe.ram...@arm.com>
>
> Backported from master
> 2020-05-20 Joe Ramsay <joe.ram...@arm.com>
>
> * config/arm/arm-protos.h (arm_coproc_mem_operand_no_writeback):
> Declare prototype.
> (arm_mve_mode_and_operands_type_check): Declare prototype.
> * config/arm/arm.c (arm_coproc_mem_operand): Refactor to use
> _arm_coproc_mem_operand.
> (arm_coproc_mem_operand_wb): New function to cover full, limited
> and no writeback.
> (arm_coproc_mem_operand_no_writeback): New constraint for memory
> operand with no writeback.
> (arm_print_operand): Extend 'E' specifier for memory operand
> that does not support writeback.
> (arm_mve_mode_and_operands_type_check): New constraint check for
> MVE memory operands.
> * config/arm/constraints.md: Add Uj constraint for VFP vldr.16
> and vstr.16.
> * config/arm/vfp.md (*mov_load_vfp_hf16): New pattern for
> vldr.16.
> (*mov_store_vfp_hf16): New pattern for vstr.16.
> (*mov<mode>_vfp_<mode>16): Remove MVE moves.
>
> gcc/testsuite/ChangeLog:
>
> 2020-08-04 Joe Ramsay <joe.ram...@arm.com>
>
> Backported from master
> 2020-05-20 Joe Ramsay <joe.ram...@arm.com>
>
> * gcc.target/arm/mve/intrinsics/mve-vldstr16-no-writeback.c: New test.
> ---
> gcc/config/arm/arm-protos.h | 3 +
> gcc/config/arm/arm.c | 74
> ++++++++++++++++++----
> gcc/config/arm/constraints.md | 7 ++
> gcc/config/arm/vfp.md | 26 +++++---
> .../arm/mve/intrinsics/mve-vldstr16-no-writeback.c | 17 +++++
> 5 files changed, 105 insertions(+), 22 deletions(-)
> create mode 100644
> gcc/testsuite/gcc.target/arm/mve/intrinsics/mve-vldstr16-no-writeback.c
>
> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
> index 33d162c..e811da4 100644
> --- a/gcc/config/arm/arm-protos.h
> +++ b/gcc/config/arm/arm-protos.h
> @@ -115,8 +115,11 @@ extern enum reg_class coproc_secondary_reload_class
> (machine_mode, rtx,
> extern bool arm_tls_referenced_p (rtx);
>
> extern int arm_coproc_mem_operand (rtx, bool);
> +extern int arm_coproc_mem_operand_no_writeback (rtx);
> +extern int arm_coproc_mem_operand_wb (rtx, int);
> extern int neon_vector_mem_operand (rtx, int, bool);
> extern int mve_vector_mem_operand (machine_mode, rtx, bool);
> +bool arm_mve_mode_and_operands_type_check (machine_mode, rtx, rtx);
> extern int neon_struct_mem_operand (rtx);
>
> extern rtx *neon_vcmla_lane_prepare_operands (rtx *);
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index a8825ee..d8da167 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -13192,13 +13192,14 @@ neon_element_bits (machine_mode mode)
> /* Predicates for `match_operand' and `match_operator'. */
>
> /* Return TRUE if OP is a valid coprocessor memory address pattern.
> - WB is true if full writeback address modes are allowed and is false
> + WB level is 2 if full writeback address modes are allowed, 1
> if limited writeback address modes (POST_INC and PRE_DEC) are
> - allowed. */
> + allowed and 0 if no writeback at all is supported. */
>
> int
> -arm_coproc_mem_operand (rtx op, bool wb)
> +arm_coproc_mem_operand_wb (rtx op, int wb_level)
> {
> + gcc_assert (wb_level == 0 || wb_level == 1 || wb_level == 2);
> rtx ind;
>
> /* Reject eliminable registers. */
> @@ -13231,16 +13232,18 @@ arm_coproc_mem_operand (rtx op, bool wb)
>
> /* Autoincremment addressing modes. POST_INC and PRE_DEC are
> acceptable in any case (subject to verification by
> - arm_address_register_rtx_p). We need WB to be true to accept
> + arm_address_register_rtx_p). We need full writeback to accept
> + PRE_INC and POST_DEC, and at least restricted writeback for
> PRE_INC and POST_DEC. */
> - if (GET_CODE (ind) == POST_INC
> - || GET_CODE (ind) == PRE_DEC
> - || (wb
> - && (GET_CODE (ind) == PRE_INC
> - || GET_CODE (ind) == POST_DEC)))
> + if (wb_level > 0
> + && (GET_CODE (ind) == POST_INC
> + || GET_CODE (ind) == PRE_DEC
> + || (wb_level > 1
> + && (GET_CODE (ind) == PRE_INC
> + || GET_CODE (ind) == POST_DEC))))
> return arm_address_register_rtx_p (XEXP (ind, 0), 0);
>
> - if (wb
> + if (wb_level > 1
> && (GET_CODE (ind) == POST_MODIFY || GET_CODE (ind) == PRE_MODIFY)
> && arm_address_register_rtx_p (XEXP (ind, 0), 0)
> && GET_CODE (XEXP (ind, 1)) == PLUS
> @@ -13262,6 +13265,25 @@ arm_coproc_mem_operand (rtx op, bool wb)
> return FALSE;
> }
>
> +/* Return TRUE if OP is a valid coprocessor memory address pattern.
> + WB is true if full writeback address modes are allowed and is false
> + if limited writeback address modes (POST_INC and PRE_DEC) are
> + allowed. */
> +
> +int arm_coproc_mem_operand (rtx op, bool wb)
> +{
> + return arm_coproc_mem_operand_wb (op, wb ? 2 : 1);
> +}
> +
> +/* Return TRUE if OP is a valid coprocessor memory address pattern in a
> + context in which no writeback address modes are allowed. */
> +
> +int
> +arm_coproc_mem_operand_no_writeback (rtx op)
> +{
> + return arm_coproc_mem_operand_wb (op, 0);
> +}
> +
> /* This function returns TRUE on matching mode and op.
> 1. For given modes, check for [Rn], return TRUE for Rn <= LO_REGS.
> 2. For other modes, check for [Rn], return TRUE for Rn < R15 (expect R13).
> */
> @@ -23532,7 +23554,7 @@ arm_print_condition (FILE *stream)
> /* Globally reserved letters: acln
> Puncutation letters currently used: @_|?().!#
> Lower case letters currently used: bcdefhimpqtvwxyz
> - Upper case letters currently used: ABCDFGHJKLMNOPQRSTU
> + Upper case letters currently used: ABCDEFGHIJKLMNOPQRSTU
> Letters previously used, but now deprecated/obsolete: sVWXYZ.
>
> Note that the global reservation for 'c' is only for CONSTANT_ADDRESS_P.
> @@ -24098,11 +24120,12 @@ arm_print_operand (FILE *stream, rtx x, int code)
> }
> return;
>
> - /* To print the memory operand with "Ux" constraint. Based on the
> rtx_code
> - the memory operands output looks like following.
> + /* To print the memory operand with "Ux" or "Uj" constraint. Based on
> the
> + rtx_code the memory operands output looks like following.
> 1. [Rn], #+/-<imm>
> 2. [Rn, #+/-<imm>]!
> - 3. [Rn]. */
> + 3. [Rn, #+/-<imm>]
> + 4. [Rn]. */
> case 'E':
> {
> rtx addr;
> @@ -24137,6 +24160,16 @@ arm_print_operand (FILE *stream, rtx x, int code)
> asm_fprintf (stream, ", #%wd]!",INTVAL (postinc_reg));
> }
> }
> + else if (code == PLUS)
> + {
> + rtx base = XEXP (addr, 0);
> + rtx index = XEXP (addr, 1);
> +
> + gcc_assert (REG_P (base) && CONST_INT_P (index));
> +
> + HOST_WIDE_INT offset = INTVAL (index);
> + asm_fprintf (stream, "[%r, #%wd]", REGNO (base), offset);
> + }
> else
> {
> gcc_assert (REG_P (addr));
> @@ -33482,4 +33515,17 @@ arm_mode_base_reg_class (machine_mode mode)
>
> struct gcc_target targetm = TARGET_INITIALIZER;
>
> +bool
> +arm_mve_mode_and_operands_type_check (machine_mode mode, rtx op0, rtx op1)
> +{
> + if (!(TARGET_HAVE_MVE || TARGET_HAVE_MVE_FLOAT))
> + return true;
> + else if (mode == E_BFmode)
> + return false;
> + else if ((s_register_operand (op0, mode) && MEM_P (op1))
> + || (s_register_operand (op1, mode) && MEM_P (op0)))
> + return false;
> + return true;
> +}
> +
> #include "gt-arm.h"
> diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
> index 011badc..ff229aa 100644
> --- a/gcc/config/arm/constraints.md
> +++ b/gcc/config/arm/constraints.md
> @@ -452,6 +452,13 @@
> (and (match_code "mem")
> (match_test "TARGET_32BIT && arm_coproc_mem_operand (op, FALSE)")))
>
> +(define_memory_constraint "Uj"
> + "@internal
> + In ARM/Thumb-2 state an VFP load/store address which does not support
> + writeback at all (eg vldr.16)."
> + (and (match_code "mem")
> + (match_test "TARGET_32BIT && arm_coproc_mem_operand_no_writeback
> (op)")))
> +
> (define_memory_constraint "Uy"
> "@internal
> In ARM/Thumb-2 state a valid iWMMX load/store address."
> diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
> index 3470679..6a2bc5a 100644
> --- a/gcc/config/arm/vfp.md
> +++ b/gcc/config/arm/vfp.md
> @@ -387,6 +387,20 @@
> (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> )
>
> +(define_insn "*mov_load_vfp_hf16"
> + [(set (match_operand:HF 0 "s_register_operand" "=t")
> + (match_operand:HF 1 "memory_operand" "Uj"))]
> + "TARGET_HAVE_MVE_FLOAT"
> + "vldr.16\\t%0, %E1"
> +)
> +
> +(define_insn "*mov_store_vfp_hf16"
> + [(set (match_operand:HF 0 "memory_operand" "=Uj")
> + (match_operand:HF 1 "s_register_operand" "t"))]
> + "TARGET_HAVE_MVE_FLOAT"
> + "vstr.16\\t%1, %E0"
> +)
> +
> ;; HFmode and BFmode moves
>
> (define_insn "*mov<mode>_vfp_<mode>16"
> @@ -396,6 +410,8 @@
> " m,r,t,r,r,t,Dv,Um,t, F"))]
> "TARGET_32BIT
> && TARGET_VFP_FP16INST
> + && arm_mve_mode_and_operands_type_check (<MODE>mode, operands[0],
> + operands[1])
> && (s_register_operand (operands[0], <MODE>mode)
> || s_register_operand (operands[1], <MODE>mode))"
> {
> @@ -414,15 +430,9 @@
> case 6: /* S register from immediate. */
> return \"vmov.f16\\t%0, %1\t%@ __<fporbf>\";
> case 7: /* S register from memory. */
> - if (TARGET_HAVE_MVE)
> - return \"vldr.16\\t%0, %A1\";
> - else
> - return \"vld1.16\\t{%z0}, %A1\";
> + return \"vld1.16\\t{%z0}, %A1\";
> case 8: /* Memory from S register. */
> - if (TARGET_HAVE_MVE)
> - return \"vstr.16\\t%1, %A0\";
> - else
> - return \"vst1.16\\t{%z1}, %A0\";
> + return \"vst1.16\\t{%z1}, %A0\";
> case 9: /* ARM register from constant. */
> {
> long bits;
> diff --git
> a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve-vldstr16-no-writeback.c
> b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve-vldstr16-no-writeback.c
> new file mode 100644
> index 0000000..0a69ace
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve-vldstr16-no-writeback.c
> @@ -0,0 +1,17 @@
> +/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
> +/* { dg-add-options arm_v8_1m_mve_fp } */
> +/* { dg-additional-options "-O2" } */
> +
> +void
> +fn1 (__fp16 *pSrc)
> +{
> + __fp16 high;
> + __fp16 *pDst = 0;
> + unsigned i;
> + for (i = 0;; i++)
> + if (pSrc[i])
> + pDst[i] = high;
> +}
> +
> +/* { dg-final { scan-assembler {vldr\.16\ts[0-9]+, \[r[0-9]+\]\n} } } */
> +/* { dg-final { scan-assembler {vstr\.16\ts[0-9]+, \[r[0-9]+\]\n} } } */
>
