On Fri, 29 Aug 2025 at 15:50, Christophe Lyon
<[email protected]> wrote:
>
> 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.
>

Sorry I forgot to ask: is it OK to backport the series to gcc-15 ?

Thanks,

Christophe

> Thanks
>
> Christophe
>
> > R.

Reply via email to