diff mbox series

[avr] Fix PR116781: Fix ICE due to (clobber (match_dup)) in insn pattern

Message ID 7d98b9fa-30a8-4fb6-ace5-38e41bc51a68@gjlay.de
State New
Headers show
Series [avr] Fix PR116781: Fix ICE due to (clobber (match_dup)) in insn pattern | expand

Commit Message

Georg-Johann Lay Nov. 16, 2024, 2:01 p.m. UTC
Some passes like CSE kick-up when they see a (clobber (match_dup)) in an
insn pattern, like it is the case for avr.md's *tablejump.

This patch uses a new "scratch_operand" instead.  As the clobbered
entity is known, the casesi expander now uses REG_Z or scratch:HI.
The latter means that the entity is not clobbered.

The avr-casesi pass and optimization has to be adjusted to the new
anatomy of the casesi / tablejump insns.

Test results are looking good:

With -mno-lra there is no change for atmega8, atmega128, atmega2560.

With -mlra, the ICEs as of PR116781 go away.

Ok for trunk?

Johann

--

AVR: target/116781 - Fix ICE due to (clobber (match_dup)) in tablejump.

This patch avoids (clobber (match_dup)) in insn patterns for tablejump.
The machine description now uses a scratch_operand instead which is
possible since the clobbered entity is known in advance:

3-byte PC        : REG_Z
2-byte PC + JMP  : REG_Z
2-byte PC + RJMP : None, hence scratch:HI is used.

The avr-casesi pass and optimization has to be adjusted to the new patterns.

	PR target/116781
gcc/
	* config/avr/avr.md (*tablejump_split, *tablejump): Add
	operand 2 as a "scratch_operand" instead of a match_dup.
	(casesi): Adjust expander operands accordingly.  Use a scratch:HI
	when the jump address is not clobbered.  This is the case for a
	2-byte PC + has no JMP instruction.  In all the other cases, the
	affected operand is REG_Z (reg:HI 30).
	(casesi_<mode>_sequence): Adjust matcher to new anatomy.
	* config/avr/avr-passes.cc (avr_is_casesi_sequence)
	(avr_is_casesi_sequence, avr_optimize_casesi)
	(avr_casei_sequence_check_operands): Adjust to new anatomy.

Comments

Denis Chertykov Nov. 16, 2024, 6:32 p.m. UTC | #1
сб, 16 нояб. 2024 г. в 18:01, Georg-Johann Lay <avr@gjlay.de>:

>
> Some passes like CSE kick-up when they see a (clobber (match_dup)) in an
> insn pattern, like it is the case for avr.md's *tablejump.
>
> This patch uses a new "scratch_operand" instead.  As the clobbered
> entity is known, the casesi expander now uses REG_Z or scratch:HI.
> The latter means that the entity is not clobbered.
>
> The avr-casesi pass and optimization has to be adjusted to the new
> anatomy of the casesi / tablejump insns.
>
> Test results are looking good:
>
> With -mno-lra there is no change for atmega8, atmega128, atmega2560.
>
> With -mlra, the ICEs as of PR116781 go away.
>
> Ok for trunk?

Please apply.

Denis.
diff mbox series

Patch

    AVR: target/116781 - Fix ICE due to (clobber (match_dup)) in tablejump.
    
    This patch avoids (clobber (match_dup)) in insn patterns for tablejump.
    The machine description now uses a scratch_operand instead which is
    possible since the clobbered entity is known in advance:
    
    3-byte PC        : REG_Z
    2-byte PC + JMP  : REG_Z
    2-byte PC + RJMP : None, hence scratch:HI is used.
    
    The avr-casesi pass and optimization has to be adjusted to the new patterns.
    
            PR target/116781
    gcc/
            * config/avr/avr.md (*tablejump_split, *tablejump): Add
            operand 2 as a "scratch_operand" instead of a match_dup.
            (casesi): Adjust expander operands accordingly.  Use a scratch:HI
            when the jump address is not clobbered.  This is the case for a
            2-byte PC + has no JMP instruction.  In all the other cases, the
            affected operand is REG_Z (reg:HI 30).
            (casesi_<mode>_sequence): Adjust matcher to new anatomy.
            * config/avr/avr-passes.cc (avr_is_casesi_sequence)
            (avr_is_casesi_sequence, avr_optimize_casesi)
            (avr_casei_sequence_check_operands): Adjust to new anatomy.

diff --git a/gcc/config/avr/avr-passes.cc b/gcc/config/avr/avr-passes.cc
index d075aaecb54..6ebefc7ef8a 100644
--- a/gcc/config/avr/avr-passes.cc
+++ b/gcc/config/avr/avr-passes.cc
@@ -3942,8 +3942,8 @@  avr_is_casesi_sequence (basic_block bb, rtx_insn *insn, rtx_insn *insns[5])
 
   // Assert on the anatomy of xinsn's operands we are going to work with.
 
-  gcc_assert (recog_data.n_operands == 11);
-  gcc_assert (recog_data.n_dups == 4);
+  gcc_assert (recog_data.n_operands == 12);
+  gcc_assert (recog_data.n_dups == 3);
 
   if (dump_file)
     {
@@ -3962,7 +3962,7 @@  avr_is_casesi_sequence (basic_block bb, rtx_insn *insn, rtx_insn *insns[5])
    operands of INSNS as extracted by insn_extract from pattern
    casesi_<mode>_sequence:
 
-      $0: SImode reg switch value as result of $9.
+      $0: SImode reg switch value as result of $10.
       $1: Negative of smallest index in switch.
       $2: Number of entries in switch.
       $3: Label to table.
@@ -3971,9 +3971,10 @@  avr_is_casesi_sequence (basic_block bb, rtx_insn *insn, rtx_insn *insns[5])
       $6: 3-byte PC: subreg:HI ($5) + label_ref ($3)
 	  2-byte PC: subreg:HI ($5)
       $7: HI reg index into table (Z or pseudo)
-      $8: R24 or const0_rtx (to be clobbered)
-      $9: Extension to SImode of an 8-bit or 16-bit integer register $10.
-      $10: QImode or HImode register input of $9.
+      $8: Z or scratch:HI (to be clobbered)
+      $9: R24 or const0_rtx (to be clobbered)
+      $10: Extension to SImode of an 8-bit or 16-bit integer register $11.
+      $11: QImode or HImode register input of $10.
 
    Try to optimize this sequence, i.e. use the original HImode / QImode
    switch value instead of SImode.  */
@@ -3982,11 +3983,11 @@  static void
 avr_optimize_casesi (rtx_insn *insns[5], rtx *xop)
 {
   // Original mode of the switch value; this is QImode or HImode.
-  machine_mode mode = GET_MODE (xop[10]);
+  machine_mode mode = GET_MODE (xop[11]);
 
   // How the original switch value was extended to SImode; this is
   // SIGN_EXTEND or ZERO_EXTEND.
-  rtx_code code = GET_CODE (xop[9]);
+  rtx_code code = GET_CODE (xop[10]);
 
   // Lower index, upper index (plus one) and range of case calues.
   HOST_WIDE_INT low_idx = -INTVAL (xop[1]);
@@ -4030,7 +4031,7 @@  avr_optimize_casesi (rtx_insn *insns[5], rtx *xop)
 
   start_sequence();
 
-  rtx reg = copy_to_mode_reg (mode, xop[10]);
+  rtx reg = copy_to_mode_reg (mode, xop[11]);
 
   rtx (*gen_add)(rtx,rtx,rtx) = QImode == mode ? gen_addqi3 : gen_addhi3;
   rtx (*gen_cbranch)(rtx,rtx,rtx,rtx)
@@ -4140,7 +4141,7 @@  avr_casei_sequence_check_operands (rtx *xop)
 
   if (AVR_HAVE_EIJMP_EICALL
       // The last clobber op of the tablejump.
-      && xop[8] == all_regs_rtx[REG_24])
+      && xop[9] == all_regs_rtx[REG_24])
     {
       // $6 is: (subreg:SI ($5) 0)
       sub_5 = xop[6];
@@ -4153,7 +4154,7 @@  avr_casei_sequence_check_operands (rtx *xop)
       && LABEL_REF == GET_CODE (XEXP (xop[6], 1))
       && rtx_equal_p (xop[3], XEXP (XEXP (xop[6], 1), 0))
       // The last clobber op of the tablejump.
-      && xop[8] == const0_rtx)
+      && xop[9] == const0_rtx)
     {
       sub_5 = XEXP (xop[6], 0);
     }
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index 550b01b36fb..a183cbc2c08 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -7713,7 +7713,7 @@  (define_insn_and_split "*tablejump_split"
         (unspec:HI [(match_operand:HI 0 "register_operand" "!z,*r,z")]
                    UNSPEC_INDEX_JMP))
    (use (label_ref (match_operand 1 "" "")))
-   (clobber (match_dup 0))
+   (clobber (match_operand:HI 2 "scratch_operand" "=X,X,0"))
    (clobber (const_int 0))]
   "!AVR_HAVE_EIJMP_EICALL"
   "#"
@@ -7722,7 +7722,7 @@  (define_insn_and_split "*tablejump_split"
                    (unspec:HI [(match_dup 0)]
                               UNSPEC_INDEX_JMP))
               (use (label_ref (match_dup 1)))
-              (clobber (match_dup 0))
+              (clobber (match_dup 2))
               (clobber (const_int 0))
               (clobber (reg:CC REG_CC))])]
   ""
@@ -7733,7 +7733,7 @@  (define_insn "*tablejump"
         (unspec:HI [(match_operand:HI 0 "register_operand" "!z,*r,z")]
                    UNSPEC_INDEX_JMP))
    (use (label_ref (match_operand 1 "" "")))
-   (clobber (match_dup 0))
+   (clobber (match_operand:HI 2 "scratch_operand" "=X,X,0"))
    (clobber (const_int 0))
    (clobber (reg:CC REG_CC))]
   "!AVR_HAVE_EIJMP_EICALL && reload_completed"
@@ -7819,8 +7819,8 @@  (define_expand "casesi"
    (parallel [(set (pc)
                    (unspec:HI [(match_dup 7)] UNSPEC_INDEX_JMP))
               (use (label_ref (match_dup 3)))
-              (clobber (match_dup 7))
-              (clobber (match_dup 8))])]
+              (clobber (match_dup 8))
+              (clobber (match_dup 9))])]
   ""
   {
     operands[1] = simplify_unary_operation (NEG, SImode, operands[1], SImode);
@@ -7830,14 +7830,24 @@  (define_expand "casesi"
     if (AVR_HAVE_EIJMP_EICALL)
       {
         operands[7] = gen_rtx_REG (HImode, REG_Z);
-        operands[8] = all_regs_rtx[24];
+        operands[8] = gen_rtx_REG (HImode, REG_Z);
+        operands[9] = all_regs_rtx[24];
       }
     else
       {
         operands[6] = gen_rtx_PLUS (HImode, operands[6],
-                                    gen_rtx_LABEL_REF (VOIDmode, operands[3]));
-        operands[7] = gen_reg_rtx (HImode);
-        operands[8] = const0_rtx;
+                                            gen_rtx_LABEL_REF (VOIDmode, operands[3]));
+        if (AVR_HAVE_JMP_CALL)
+          {
+            operands[7] = gen_rtx_REG (HImode, REG_Z);
+            operands[8] = gen_rtx_REG (HImode, REG_Z);
+          }
+        else
+          {
+            operands[7] = gen_reg_rtx (HImode);
+            operands[8] = gen_rtx_SCRATCH (HImode);
+          }
+        operands[9] = const0_rtx;
       }
   })
 
@@ -7850,11 +7860,11 @@  (define_expand "casesi"
 ;; "casesi_hi_sequence"
 (define_insn "casesi_<mode>_sequence"
   [(set (match_operand:SI 0 "register_operand")
-        (match_operator:SI 9 "extend_operator"
-                           [(match_operand:QIHI 10 "register_operand")]))
+        (match_operator:SI 10 "extend_operator"
+                           [(match_operand:QIHI 11 "register_operand")]))
 
    ;; What follows is a matcher for code from casesi.
-   ;; We keep the same operand numbering (except for $9 and $10
+   ;; We keep the same operand numbering (except for $10 and $11
    ;; which don't appear in casesi).
    (parallel [(set (match_operand:SI 5 "register_operand")
                    (plus:SI (match_dup 0)
@@ -7874,8 +7884,8 @@  (define_insn "casesi_<mode>_sequence"
    (parallel [(set (pc)
                    (unspec:HI [(match_dup 7)] UNSPEC_INDEX_JMP))
               (use (label_ref (match_operand 3)))
-              (clobber (match_dup 7))
-              (clobber (match_operand:QI 8))])]
+              (clobber (match_operand:HI 8))
+              (clobber (match_operand:QI 9))])]
   "optimize
    && avr_casei_sequence_check_operands (operands)"
   { gcc_unreachable(); }