On 7/20/23 07:17, senthilkumar.selva...@microchip.com wrote:
Hi,

   The avr backend has this define_insn_and_split

(define_insn_and_split "*tablejump_split"
   [(set (pc)
         (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 (const_int 0))]
   "!AVR_HAVE_EIJMP_EICALL"
   "#"
   "&& reload_completed"
   [(parallel [(set (pc)
                    (unspec:HI [(match_dup 0)]
                               UNSPEC_INDEX_JMP))
               (use (label_ref (match_dup 1)))
               (clobber (match_dup 0))
               (clobber (const_int 0))
               (clobber (reg:CC REG_CC))])]
   ""
   [(set_attr "isa" "rjmp,rjmp,jmp")])

Note the (clobber (match_dup 0)). When building

$ avr-gcc -mmcu=avr51 gcc/gcc/testsuite/gcc.c-torture/compile/930120-1.c -O3 
-funroll-loops -fdump-rtl-all

The web pass transforms this insn from

(jump_insn 120 538 124 25 (parallel [
             (set (pc)
                 (unspec:HI [
                         (reg:HI 138)
                     ] UNSPEC_INDEX_JMP))
             (use (label_ref 121))
             (clobber (reg:HI 138))
             (clobber (const_int 0 [0]))
         ]) "gcc/gcc/testsuite/gcc.c-torture/compile/930120-1.c":55:5 779 
{*tablejump_split}
      (expr_list:REG_DEAD (reg:HI 138)
         (expr_list:REG_UNUSED (reg:HI 138)
             (nil)))
  -> 121)

to

  Web oldreg=138 newreg=279
Updating insn 120 (138->279)
<snip>
(jump_insn 120 538 124 25 (parallel [
             (set (pc)
                 (unspec:HI [
                         (reg:HI 138)
                     ] UNSPEC_INDEX_JMP))
             (use (label_ref 121))
             (clobber (reg:HI 279))
             (clobber (const_int 0 [0]))
         ]) "gcc/gcc/testsuite/gcc.c-torture/compile/930120-1.c":55:5 779 
{*tablejump_split}
      (expr_list:REG_DEAD (reg:HI 138)
         (expr_list:REG_UNUSED (reg:HI 138)
             (nil)))
  -> 121)

Note the reg in the clobber is now 279, and not 138.

With classic reload, however, this gets set back to whatever hardreg was 
assigned to r138.
It is just a fortunate side effect of algorithm how the reload pass changes pseudos to their hard registers.
(jump_insn 120 538 121 26 (parallel [
             (set (pc)
                 (unspec:HI [
                         (reg/f:HI 30 r30 [138])
                     ] UNSPEC_INDEX_JMP))
             (use (label_ref 121))
             (clobber (reg/f:HI 30 r30 [138]))
             (clobber (const_int 0 [0]))
         ]) "gcc/gcc/testsuite/gcc.c-torture/compile/930120-1.c":55:5 779 
{*tablejump_split}
      (nil)
  -> 121)

With LRA, however, the pseudo reg remains unassigned, eventually causing an ICE 
in cselib_invalidate_regno.

(jump_insn 120 538 121 26 (parallel [
             (set (pc)
                 (unspec:HI [
                         (reg/f:HI 30 r30 [138])
                     ] UNSPEC_INDEX_JMP))
             (use (label_ref 121))
             (clobber (reg:HI 279))
             (clobber (const_int 0 [0]))
         ]) "gcc/gcc/testsuite/gcc.c-torture/compile/930120-1.c":55:5 779 
{*tablejump_split}
      (nil)
  -> 121)

Is this something that LRA should be able to fix?

No, LRA can not do this but it keeps match_dup correct after any reloads and insn transformations.

I think it is a web optimization bug.  RA assumes the insn recognition should give the same insn code as it present in the insn.

In my opinion any optimization pass can assume this at the pass start and should keep this condition after its work finish.


Reply via email to