On Fri, 29 Aug 2025 at 15:35, Richard Earnshaw <[email protected]> wrote: > > On 27/08/2025 15:45, Christophe Lyon wrote: > > The thumb2_asrl, thumb2_lsll and thumb2_lsrl patterns were incorrecly > > using (match_dup 0) for the first argument of the shift operator. > > > > This patch replaces that with (match_operand:DI 1 > > arm_general_register_operandarm_general_register_operand "0") and > > fixes the related expanders in arm.md to use that additional argument > > and get rid of the copy of operands[1] to operands[0]. > > > > Finally, since these patterns are MVE-only, rename them into mve_XXX > > and move them to mve.md. > > > > gcc/ChangeLog: > > > > * config/arm/thumb2.md (thumb2_asrl, thumb2_lsll, thumb2_lsrl): > > Move to ... > > * config/arm/mve.md (mve_asrl, mve_lsll, mve_lsrl): ... here. Use > > match_operand instead of match_dup. > > * config/arm/arm.md (ashldi3, ashrdi3, lshrdi3): Remove useless > > copy. Update for new prototype. > > --- > > gcc/config/arm/arm.md | 15 +++------------ > > gcc/config/arm/mve.md | 25 +++++++++++++++++++++++++ > > gcc/config/arm/thumb2.md | 24 ------------------------ > > 3 files changed, 28 insertions(+), 36 deletions(-) > > > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > > index 537a3e26a45..e0d2a45669e 100644 > > --- a/gcc/config/arm/arm.md > > +++ b/gcc/config/arm/arm.md > > @@ -4588,10 +4588,7 @@ (define_expand "ashldi3" > > if (arm_reg_or_long_shift_imm (operands[2], GET_MODE (operands[2])) > > && (REG_P (operands[2]) || INTVAL(operands[2]) != 32)) > > { > > - if (!reg_overlap_mentioned_p(operands[0], operands[1])) > > - emit_insn (gen_movdi (operands[0], operands[1])); > > - > > - emit_insn (gen_thumb2_lsll (operands[0], operands[2])); > > + emit_insn (gen_mve_lsll (operands[0], operands[1], operands[2])); > > DONE; > > } > > } > > @@ -4627,10 +4624,7 @@ (define_expand "ashrdi3" > > if (TARGET_HAVE_MVE && !BYTES_BIG_ENDIAN > > && arm_reg_or_long_shift_imm (operands[2], GET_MODE (operands[2]))) > > { > > - if (!reg_overlap_mentioned_p(operands[0], operands[1])) > > - emit_insn (gen_movdi (operands[0], operands[1])); > > - > > - emit_insn (gen_thumb2_asrl (operands[0], operands[2])); > > + emit_insn (gen_mve_asrl (operands[0], operands[1], operands[2])); > > DONE; > > } > > > > @@ -4662,10 +4656,7 @@ (define_expand "lshrdi3" > > if (TARGET_HAVE_MVE && !BYTES_BIG_ENDIAN > > && long_shift_imm (operands[2], GET_MODE (operands[2]))) > > { > > - if (!reg_overlap_mentioned_p(operands[0], operands[1])) > > - emit_insn (gen_movdi (operands[0], operands[1])); > > - > > - emit_insn (gen_thumb2_lsrl (operands[0], operands[2])); > > + emit_insn (gen_mve_lsrl (operands[0], operands[1], operands[2])); > > DONE; > > } > > > > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md > > index 70b1c77fdce..4636ff356f0 100644 > > --- a/gcc/config/arm/mve.md > > +++ b/gcc/config/arm/mve.md > > @@ -4703,3 +4703,28 @@ (define_insn "dlstp<dlstp_elemsize>_insn" > > "TARGET_HAVE_MVE" > > "dlstp.<dlstp_elemsize>\t%|lr, %0" > > [(set_attr "type" "mve_misc")]) > > + > > +;; Scalar shifts > > +(define_insn "mve_asrl" > > + [(set (match_operand:DI 0 "arm_general_register_operand" "=r") > > + (ashiftrt:DI (match_operand:DI 1 "arm_general_register_operand" "0") > > + (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")))] > > + "TARGET_HAVE_MVE" > > + "asrl%?\\t%Q0, %R1, %2" > > + [(set_attr "predicable" "yes")]) > > + > > +(define_insn "mve_lsll" > > + [(set (match_operand:DI 0 "arm_general_register_operand" "=r") > > + (ashift:DI (match_operand:DI 1 "arm_general_register_operand" "0") > > + (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")))] > > + "TARGET_HAVE_MVE" > > + "lsll%?\\t%Q0, %R1, %2" > > + [(set_attr "predicable" "yes")]) > > + > > +(define_insn "mve_lsrl" > > + [(set (match_operand:DI 0 "arm_general_register_operand" "=r") > > + (lshiftrt:DI (match_operand:DI 1 "arm_general_register_operand" "0") > > + (match_operand:SI 2 "long_shift_imm" "Pg")))] > > + "TARGET_HAVE_MVE" > > + "lsrl%?\\t%Q0, %R1, %2" > > + [(set_attr "predicable" "yes")]) > > diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md > > index 2c2026b1e74..40c0e052946 100644 > > --- a/gcc/config/arm/thumb2.md > > +++ b/gcc/config/arm/thumb2.md > > @@ -1733,30 +1733,6 @@ (define_insn "*clear_multiple" > > [(set_attr "predicable" "yes")] > > ) > > > > -(define_insn "thumb2_asrl" > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r") > > - (ashiftrt:DI (match_dup 0) > > - (match_operand:SI 1 "arm_reg_or_long_shift_imm" "rPg")))] > > - "TARGET_HAVE_MVE" > > - "asrl%?\\t%Q0, %R0, %1" > > - [(set_attr "predicable" "yes")]) > > - > > -(define_insn "thumb2_lsll" > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r") > > - (ashift:DI (match_dup 0) > > - (match_operand:SI 1 "arm_reg_or_long_shift_imm" "rPg")))] > > - "TARGET_HAVE_MVE" > > - "lsll%?\\t%Q0, %R0, %1" > > - [(set_attr "predicable" "yes")]) > > - > > -(define_insn "thumb2_lsrl" > > - [(set (match_operand:DI 0 "arm_general_register_operand" "+r") > > - (lshiftrt:DI (match_dup 0) > > - (match_operand:SI 1 "long_shift_imm" "Pg")))] > > - "TARGET_HAVE_MVE" > > - "lsrl%?\\t%Q0, %R0, %1" > > - [(set_attr "predicable" "yes")]) > > - > > ;; Originally expanded by 'doloop_end'. > > (define_insn "*doloop_end_internal" > > [(set (pc) > > This is OK, but I'm not really sure it belongs in this patch set. If we > needed to back out (or backport) the other bits, this would need to be > treated separately. >
Right... I just happened to notice the problem while working on patches 3 and 4. I can certainly move this one before the series. Thanks Christophe > R.
