On 29/08/2025 14:57, Christophe Lyon wrote:
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 ?

Is there a PR for it that would justify that? We normally only back-port fixes for crashes or wrong-code.

R.


Thanks,

Christophe

Thanks

Christophe

R.

Reply via email to