From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19794 invoked by alias); 8 Jul 2011 10:20:15 -0000 Received: (qmail 19784 invoked by uid 22791); 8 Jul 2011 10:20:14 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-ew0-f47.google.com (HELO mail-ew0-f47.google.com) (209.85.215.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 08 Jul 2011 10:19:56 +0000 Received: by ewy5 with SMTP id 5so697116ewy.20 for ; Fri, 08 Jul 2011 03:19:55 -0700 (PDT) Received: by 10.14.98.131 with SMTP id v3mr551695eef.38.1310120395319; Fri, 08 Jul 2011 03:19:55 -0700 (PDT) MIME-Version: 1.0 Received: by 10.14.101.143 with HTTP; Fri, 8 Jul 2011 03:19:35 -0700 (PDT) In-Reply-To: <4E16D4FD.8020806@gjlay.de> References: <4DF0FAB5.6070704@gjlay.de> <4DF11D20.4030907@gjlay.de> <4DF1ED76.4030507@gjlay.de> <4DF650B7.3030705@gjlay.de> <4DF73490.2080709@gjlay.de> <4DF7D2B5.1090708@gjlay.de> <4DF8ED42.1030706@redhat.com> <4DF918A9.4070003@gjlay.de> <4DF92AEA.4000906@redhat.com> <4DF93B17.8020008@redhat.com> <4E03B658.8020509@redhat.com> <4E078F93.7060901@gjlay.de> <4E084291.4030300@gjlay.de> <4E157B72.1000304@gjlay.de> <4E16D4FD.8020806@gjlay.de> From: Denis Chertykov Date: Fri, 08 Jul 2011 10:25:00 -0000 Message-ID: Subject: Re: [Patch, AVR]: Fix PR46779 To: Georg-Johann Lay Cc: Richard Henderson , gcc-patches@gcc.gnu.org, Anatoly Sokolov , "Eric B. Weddington" , Eric Botcazou , Bernd Schmidt Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-07/txt/msg00609.txt.bz2 2011/7/8 Georg-Johann Lay : > 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: > =C2=A0http://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=3Datmega128 > > =C2=A0pr30338.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]) > =C2=A0 =C2=A0 =C2=A0 =C2=A0(mem:QI (subreg:HI (reg:SI 71) 0) [0 *D.1963_3= 7+0 S1 A8])) > pr30338.c:9 4 {*movqi} > =C2=A0 =C2=A0 (expr_list:REG_DEAD (reg:SI 71) > =C2=A0 =C2=A0 =C2=A0 =C2=A0(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 ]) > =C2=A0 =C2=A0 =C2=A0 =C2=A0(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} > =C2=A0 =C2=A0 (expr_list:REG_INC (reg:HI 16 r16 [orig:192 ivtmp.18 ] [192= ]) > =C2=A0 =C2=A0 =C2=A0 =C2=A0(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: > > + =C2=A0 if (GET_MODE_SIZE (mode) >=3D 4 > + =C2=A0 =C2=A0 =C2=A0 && regno >=3D REG_X) > + =C2=A0 =C2=A0 return 0; > > (2) > I can fix the first fail but *not* the second by not allow SUBREGs in > avr_legitimate_address_p: > > - =C2=A0 if (!strict && GET_CODE (x) =3D=3D SUBREG) */ > - =C2=A0 =C2=A0 =C2=A0 x =3D SUBREG_REG (x); */ > > > (2) Looks very reasonble, Eric Botcazou proposed it because he ran > into problems: > =C2=A0 http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01367.html > > (1) Appears to be hackish, but it should be ok. =C2=A0If 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: > =C2=A0 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. =C2=A0It generates a spill b= ut > 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. =C2=A0What 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.