On 7/20/23 07:17, [email protected] 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.