2011/7/8 Georg-Johann Lay <a...@gjlay.de>: > CCed Eric and Bernd. > > Denis Chertykov wrote: >>> Did you decide about the fix for PR46779? >>> >>> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00810.html >>> >>> Is it ok to commit? >> >> I forgot about testsuite regressions for this patch. >> >> Denis. > > > There were no new regressions: > http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00747.html > > However, with the actual trunk (SVN 175991), I get two more > spill fails for following sources: > > ./gcc.c-torture/compile/pr32349.c -O1 -mmcu=atmega128 > > pr30338.c: In function 'testload_func': > pr30338.c:13:1: error: unable to find a register to spill in class > 'POINTER_REGS' > pr30338.c:13:1: error: this is the insn: > (insn 14 13 15 2 (set (reg:QI 24 r24 [orig:73 *D.1963_37 ] [73]) > (mem:QI (subreg:HI (reg:SI 71) 0) [0 *D.1963_37+0 S1 A8])) > pr30338.c:9 4 {*movqi} > (expr_list:REG_DEAD (reg:SI 71) > (nil))) > pr30338.c:13:1: internal compiler error: in spill_failure, at > reload1.c:2120 > > > > ./gcc.c-torture/compile/pr32349.c -S -O3 -funroll-loops > > pr32349.c: In function 'foo': > pr32349.c:26:1: error: unable to find a register to spill in class > 'POINTER_REGS' > pr32349.c:26:1: error: this is the insn: > (insn 175 197 177 10 (set (reg/v:SI 234 [ m ]) > (mem:SI (post_inc:HI (reg:HI 16 r16 [orig:192 ivtmp.18 ] > [192])) [3 MEM[base: D.1996_74, offset: 0B]+0 S4 A8])) pr32349.c:18 12 > {*movsi} > (expr_list:REG_INC (reg:HI 16 r16 [orig:192 ivtmp.18 ] [192]) > (nil))) > pr32349.c:26:1: internal compiler error: in spill_failure, at > reload1.c:2120 > > > (1) > I can fix *both* fails with additional test in avr_hard_regno_mode_ok: > > + if (GET_MODE_SIZE (mode) >= 4 > + && regno >= REG_X) > + return 0; > > (2) > I can fix the first fail but *not* the second by not allow SUBREGs in > avr_legitimate_address_p: > > - if (!strict && GET_CODE (x) == SUBREG) */ > - x = SUBREG_REG (x); */ > > > (2) Looks very reasonble, Eric Botcazou proposed it because he ran > into problems: > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01367.html > > (1) Appears to be hackish, but it should be ok. If code breaks > because of that is's *definitely* a reload bug (e.g. SI-subreg of DI). > > Even the original avr_hard_regno_mode_ok is ok IMO because if a > machine says "I can hold HI in 28 but not QI in 29" reload has to > handle it (except a machine must allow word_mode in *all* it's > GENERAL_REGS, don't know if that's a must). > > I made a patch for reload, too: > http://gcc.gnu.org/ml/gcc/2011-06/msg00005.html > > Because IRA generates SUBREG of hardreg (which old lreg/greg handled > ok) and reload does not handle it correctly. It generates a spill but > without the needed input reload so that one part of the register is > missing. > > reload blames IRA or BE, IRA blames reload, BE blames IRA, etc... > > > I didn't rerun the testsuite with (1) or/and (2), I'd like both (1) > and (2) in the compiler. What do you think?
I think that AVR is a stress test for GCC core. We are on the edge. IMHO your patch is a change one tweaks to another. It's not needed if it adds regressions. Denis.