Created attachment 59149 [details] pr45830.c: C test case This ICE does not occur with the old reload (-mno-lra): $ avr-gcc pr45830.c -S -mlra -O2 -funroll-loops during RTL pass: postreload pr45830.c: In function 'bar': pr45830.c:79:1: internal compiler error: in cselib_invalidate_regno, at cselib.cc:2545 79 | } | ^ 0x62b4a3 cselib_invalidate_regno ../../../source/gcc-master/gcc/cselib.cc:2545 0x62b8f7 cselib_invalidate_rtx(rtx_def*) ../../../source/gcc-master/gcc/cselib.cc:2699 0x62b938 cselib_invalidate_rtx_note_stores ../../../source/gcc-master/gcc/cselib.cc:2710 0x62c87c cselib_record_sets ../../../source/gcc-master/gcc/cselib.cc:3037 0x62d00d cselib_process_insn(rtx_insn*) ../../../source/gcc-master/gcc/cselib.cc:3194 0x7f083a10ed8f __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 0x7f083a10ee3f __libc_start_main_impl ../csu/libc-start.c:392 The respective lines in cselib.cc read: cselib_invalidate_regno (unsigned int regno, machine_mode mode) { unsigned int endregno; unsigned int i; /* If we see pseudos after reload, something is _wrong_. */ gcc_assert (!reload_completed || regno < FIRST_PSEUDO_REGISTER || reg_renumber[regno] < 0); Target: avr Configured with: ../../source/gcc-master/configure --target=avr --disable-nls --with-dwarf2 --with-gnu-as --with-gnu-ld --disable-shared --enable-languages=c,c++ Thread model: single Supported LTO compression algorithms: zlib gcc version 15.0.0 20240918 (experimental) (GCC)
Probably we have a wring definition of "*tablejump_split" The patch: diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md index aae8a696a63..a26309650f2 100644 --- a/gcc/config/avr/avr.md +++ b/gcc/config/avr/avr.md @@ -7653,7 +7653,7 @@ (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_scratch:HI 2 "=X,X,0")) (clobber (const_int 0))] "!AVR_HAVE_EIJMP_EICALL" "#" @@ -7759,7 +7759,7 @@ (parallel [(set (pc) (unspec:HI [(match_dup 7)] UNSPEC_INDEX_JMP)) (use (label_ref (match_dup 3))) - (clobber (match_dup 7)) + (clobber (scratch:HI)) (clobber (match_dup 8))])] "" {
Created attachment 59602 [details] pr116781-gjl.diff (In reply to Denis Chertykov from comment #1) > Probably we have a wring definition of "*tablejump_split" > > The patch: > > diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md > index aae8a696a63..a26309650f2 100644 > --- a/gcc/config/avr/avr.md > +++ b/gcc/config/avr/avr.md > @@ -7653,7 +7653,7 @@ > (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_scratch:HI 2 "=X,X,0")) > (clobber (const_int 0))] > "!AVR_HAVE_EIJMP_EICALL" > "#" > @@ -7759,7 +7759,7 @@ > (parallel [(set (pc) > (unspec:HI [(match_dup 7)] UNSPEC_INDEX_JMP)) > (use (label_ref (match_dup 3))) > - (clobber (match_dup 7)) > + (clobber (scratch:HI)) > (clobber (match_dup 8))])] > "" > { I am not sure whether that is expressing what is being clobbered (operand 0). The casesi expander must clobber the registers for operand 7 which it is cooking up, so the casesi expander looks correct to me. The tablejump insn for the 2-byte PC case would go something like: diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md index 550b01b36fb..7ba43030570 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 "register_operand" "=0,0,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 "register_operand" "=0,0,0")) (clobber (const_int 0)) (clobber (reg:CC REG_CC))] "!AVR_HAVE_EIJMP_EICALL && reload_completed" The reason is that it has to clobber regs it already has; the regs are NOT provided by reload. For the split pattern it should not matter whether (match_dup 0) or (match_dup 2) is used because they must be the same.
(In reply to Georg-Johann Lay from comment #2) > Created attachment 59602 [details] > pr116781-gjl.diff > > (In reply to Denis Chertykov from comment #1) > > Probably we have a wring definition of "*tablejump_split" > > > > The patch: > > > > diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md > > index aae8a696a63..a26309650f2 100644 > > --- a/gcc/config/avr/avr.md > > +++ b/gcc/config/avr/avr.md > > @@ -7653,7 +7653,7 @@ > > (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_scratch:HI 2 "=X,X,0")) > > (clobber (const_int 0))] > > "!AVR_HAVE_EIJMP_EICALL" > > "#" > > @@ -7759,7 +7759,7 @@ > > (parallel [(set (pc) > > (unspec:HI [(match_dup 7)] UNSPEC_INDEX_JMP)) > > (use (label_ref (match_dup 3))) > > - (clobber (match_dup 7)) > > + (clobber (scratch:HI)) > > (clobber (match_dup 8))])] > > "" > > { > > I am not sure whether that is expressing what is being clobbered (operand 0). > > The casesi expander must clobber the registers for operand 7 which it is > cooking up, so the casesi expander looks correct to me. > > The tablejump insn for the 2-byte PC case would go something like: > > diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md > index 550b01b36fb..7ba43030570 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 "register_operand" "=0,0,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)) IMHO: here we can have a `(clobber (match_dup 0))' because reload_completed (as you mentioned at the end of message). > (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 "register_operand" "=0,0,0")) > (clobber (const_int 0)) > (clobber (reg:CC REG_CC))] > "!AVR_HAVE_EIJMP_EICALL && reload_completed" > > The reason is that it has to clobber regs it already has; the regs are NOT > provided by reload. > > For the split pattern it should not matter whether (match_dup 0) or > (match_dup 2) is used because they must be the same. I'm agree with your patch.
(In reply to Georg-Johann Lay from comment #2) > Created attachment 59602 [details] > pr116781-gjl.diff > > (In reply to Denis Chertykov from comment #1) > > Probably we have a wring definition of "*tablejump_split" > > > > The patch: > > > > diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md > > index aae8a696a63..a26309650f2 100644 > > --- a/gcc/config/avr/avr.md > > +++ b/gcc/config/avr/avr.md > > @@ -7653,7 +7653,7 @@ > > (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_scratch:HI 2 "=X,X,0")) > > (clobber (const_int 0))] > > "!AVR_HAVE_EIJMP_EICALL" > > "#" > > @@ -7759,7 +7759,7 @@ > > (parallel [(set (pc) > > (unspec:HI [(match_dup 7)] UNSPEC_INDEX_JMP)) > > (use (label_ref (match_dup 3))) > > - (clobber (match_dup 7)) > > + (clobber (scratch:HI)) > > (clobber (match_dup 8))])] > > "" > > { > > I am not sure whether that is expressing what is being clobbered (operand 0). > > The casesi expander must clobber the registers for operand 7 which it is > cooking up, so the casesi expander looks correct to me. > > The tablejump insn for the 2-byte PC case would go something like: > > diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md > index 550b01b36fb..7ba43030570 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 "register_operand" "=0,0,0")) I'm not sure that we need `match_operand:HI 2 "register_operand"' here. In this case gcc will generate pseudo for this operand from the first RTL pass. In case of `(clobber (match_scratch' the pseudo will be generated only by register allocator and after allocation it will be a HARDREG. The cost is unnecessary pseudo. > (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 "register_operand" "=0,0,0")) And here too
The master branch has been updated by Georg-Johann Lay <gjl@gcc.gnu.org>: https://gcc.gnu.org/g:083892ba18452383a1f240072d2a96dd72321a4f commit r15-5353-g083892ba18452383a1f240072d2a96dd72321a4f Author: Georg-Johann Lay <avr@gjlay.de> Date: Sat Nov 16 14:26:02 2024 +0100 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.
Fixed on the trunk (where -mlra exists).