Wilco Dijkstra <wilco.dijks...@arm.com> writes: > A MOPS memmove may corrupt registers since there is no copy of the input > operands to temporary registers. Fix this by calling > aarch64_expand_cpymem_mops. > > Passes regress/bootstrap, OK for commit? > > gcc/ChangeLog/ > PR target/111121 > * config/aarch64/aarch64.md (aarch64_movmemdi): Add new expander. > (movmemdi): Call aarch64_expand_cpymem_mops for correct expansion. > * config/aarch64/aarch64.cc (aarch64_expand_cpymem_mops): Add > support > for memmove. > * config/aarch64/aarch64-protos.h (aarch64_expand_cpymem_mops): > Add new > function. > > gcc/testsuite/ChangeLog/ > PR target/111121 > * gcc.target/aarch64/mops_4.c: Add memmove testcases.
OK, thanks. Also OK for whichever branches need it. Sorry for the slow review, too much email backlog :( Richard > > --- > > diff --git a/gcc/config/aarch64/aarch64-protos.h > b/gcc/config/aarch64/aarch64-protos.h > index > 70303d6fd953e0c397b9138ede8858c2db2e53db..e8d91cba30e32e03c4794ccc24254691d135f2dd > 100644 > --- a/gcc/config/aarch64/aarch64-protos.h > +++ b/gcc/config/aarch64/aarch64-protos.h > @@ -765,6 +765,7 @@ bool aarch64_emit_approx_div (rtx, rtx, rtx); > bool aarch64_emit_approx_sqrt (rtx, rtx, bool); > tree aarch64_vector_load_decl (tree); > void aarch64_expand_call (rtx, rtx, rtx, bool); > +bool aarch64_expand_cpymem_mops (rtx *, bool); > bool aarch64_expand_cpymem (rtx *); > bool aarch64_expand_setmem (rtx *); > bool aarch64_float_const_zero_rtx_p (rtx); > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index > 219c4ee6d4cd7522f6ad634c794485841e5d08fa..dd6874d13a75f20d10a244578afc355b25c73da2 > 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -25228,10 +25228,11 @@ aarch64_copy_one_block_and_progress_pointers (rtx > *src, rtx *dst, > *dst = aarch64_progress_pointer (*dst); > } > > -/* Expand a cpymem using the MOPS extension. OPERANDS are taken > - from the cpymem pattern. Return true iff we succeeded. */ > -static bool > -aarch64_expand_cpymem_mops (rtx *operands) > +/* Expand a cpymem/movmem using the MOPS extension. OPERANDS are taken > + from the cpymem/movmem pattern. IS_MEMMOVE is true if this is a memmove > + rather than memcpy. Return true iff we succeeded. */ > +bool > +aarch64_expand_cpymem_mops (rtx *operands, bool is_memmove = false) > { > if (!TARGET_MOPS) > return false; > @@ -25243,8 +25244,10 @@ aarch64_expand_cpymem_mops (rtx *operands) > rtx dst_mem = replace_equiv_address (operands[0], dst_addr); > rtx src_mem = replace_equiv_address (operands[1], src_addr); > rtx sz_reg = copy_to_mode_reg (DImode, operands[2]); > - emit_insn (gen_aarch64_cpymemdi (dst_mem, src_mem, sz_reg)); > - > + if (is_memmove) > + emit_insn (gen_aarch64_movmemdi (dst_mem, src_mem, sz_reg)); > + else > + emit_insn (gen_aarch64_cpymemdi (dst_mem, src_mem, sz_reg)); > return true; > } > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index > 60133b541e9289610ce58116b0258a61f29bdc00..6d0f072a9dd6d094e8764a513222a9129d8296fa > 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -1635,7 +1635,22 @@ (define_expand "cpymemdi" > } > ) > > -(define_insn "aarch64_movmemdi" > +(define_expand "aarch64_movmemdi" > + [(parallel > + [(set (match_operand 2) (const_int 0)) > + (clobber (match_dup 3)) > + (clobber (match_dup 4)) > + (clobber (reg:CC CC_REGNUM)) > + (set (match_operand 0) > + (unspec:BLK [(match_operand 1) (match_dup 2)] UNSPEC_MOVMEM))])] > + "TARGET_MOPS" > + { > + operands[3] = XEXP (operands[0], 0); > + operands[4] = XEXP (operands[1], 0); > + } > +) > + > +(define_insn "*aarch64_movmemdi" > [(parallel [ > (set (match_operand:DI 2 "register_operand" "+&r") (const_int 0)) > (clobber (match_operand:DI 0 "register_operand" "+&r")) > @@ -1668,17 +1683,9 @@ (define_expand "movmemdi" > && INTVAL (sz_reg) < aarch64_mops_memmove_size_threshold) > FAIL; > > - rtx addr_dst = XEXP (operands[0], 0); > - rtx addr_src = XEXP (operands[1], 0); > - > - if (!REG_P (sz_reg)) > - sz_reg = force_reg (DImode, sz_reg); > - if (!REG_P (addr_dst)) > - addr_dst = force_reg (DImode, addr_dst); > - if (!REG_P (addr_src)) > - addr_src = force_reg (DImode, addr_src); > - emit_insn (gen_aarch64_movmemdi (addr_dst, addr_src, sz_reg)); > - DONE; > + if (aarch64_expand_cpymem_mops (operands, true)) > + DONE; > + FAIL; > } > ) > > diff --git a/gcc/testsuite/gcc.target/aarch64/mops_4.c > b/gcc/testsuite/gcc.target/aarch64/mops_4.c > index > 1b87759cb5e8bbcbb58cf63404d1d579d44b2818..dd796115cb4093251964d881e93bf4b98ade0c32 > 100644 > --- a/gcc/testsuite/gcc.target/aarch64/mops_4.c > +++ b/gcc/testsuite/gcc.target/aarch64/mops_4.c > @@ -50,6 +50,54 @@ copy3 (int *x, int *y, long z, long *res) > *res = z; > } > > +/* > +** move1: > +** mov (x[0-9]+), x0 > +** cpyp \[\1\]!, \[x1\]!, x2! > +** cpym \[\1\]!, \[x1\]!, x2! > +** cpye \[\1\]!, \[x1\]!, x2! > +** str x0, \[x3\] > +** ret > +*/ > +void > +move1 (int *x, int *y, long z, int **res) > +{ > + __builtin_memmove (x, y, z); > + *res = x; > +} > + > +/* > +** move2: > +** mov (x[0-9]+), x1 > +** cpyp \[x0\]!, \[\1\]!, x2! > +** cpym \[x0\]!, \[\1\]!, x2! > +** cpye \[x0\]!, \[\1\]!, x2! > +** str x1, \[x3\] > +** ret > +*/ > +void > +move2 (int *x, int *y, long z, int **res) > +{ > + __builtin_memmove (x, y, z); > + *res = y; > +} > + > +/* > +** move3: > +** mov (x[0-9]+), x2 > +** cpyp \[x0\]!, \[x1\]!, \1! > +** cpym \[x0\]!, \[x1\]!, \1! > +** cpye \[x0\]!, \[x1\]!, \1! > +** str x2, \[x3\] > +** ret > +*/ > +void > +move3 (int *x, int *y, long z, long *res) > +{ > + __builtin_memmove (x, y, z); > + *res = z; > +} > + > /* > ** set1: > ** mov (x[0-9]+), x0