From patchwork Fri Dec 2 19:25:07 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Georg-Johann Lay X-Patchwork-Id: 128941 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id C43E0B6F62 for ; Sat, 3 Dec 2011 06:26:00 +1100 (EST) Received: (qmail 30395 invoked by alias); 2 Dec 2011 19:25:58 -0000 Received: (qmail 30373 invoked by uid 22791); 2 Dec 2011 19:25:56 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mo-p00-ob.rzone.de (HELO mo-p00-ob.rzone.de) (81.169.146.160) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 02 Dec 2011 19:25:41 +0000 X-RZG-AUTH: :LXoWVUeid/7A29J/hMvvT2k715jHQaJercGObUOFkj18odoYNahU4Q== X-RZG-CLASS-ID: mo00 Received: from [192.168.0.22] (business-188-111-022-002.static.arcor-ip.net [188.111.22.2]) by smtp.strato.de (cohen mo25) (RZmta 26.10 AUTH) with ESMTPA id 204776nB2JMSv7 ; Fri, 2 Dec 2011 20:25:08 +0100 (MET) Message-ID: <4ED92613.4090100@gjlay.de> Date: Fri, 02 Dec 2011 20:25:07 +0100 From: Georg-Johann Lay User-Agent: Thunderbird 2.0.0.24 (X11/20100302) MIME-Version: 1.0 To: Denis Chertykov CC: gcc-patches@gcc.gnu.org, Wim Lewis , Eric Weddington , Joerg Wunsch Subject: Re: [Patch.AVR,4.6] Fix PR51002 References: <4ED4BEDE.4000600@gjlay.de> In-Reply-To: X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Denis Chertykov wrote: > 2011/11/29 Georg-Johann Lay: >> For devices with 8-bit SP reading the high byte SP_H of SP will get garbage. >> >> The patch uses CLR instead of IN SP_H to "read" the high part of SP. >> >> There are two issues with this patch: >> >> == 1 == >> >> I cannot really test it because for devices that small running test suite >> does not give usable results. So I just looked at the patch and the >> small test case like the following compiled >> >> $ avr-gcc-4.6.2 -Os -mmcu=attiny26 -m[no]call-prologues >> >> long long a, b; >> >> long long __attribute__((noinline,noclione)) >> bar (char volatile *c) >> { >> *c = 1; >> return a+b; >> } >> >> long long foo() >> { >> char buf[16]; >> return bar (buf); >> } >> >> >> int main (void) >> { >> return foo(); >> } >> >> >> The C parts look fine but... >> >> >> == 2 == >> >> The libgcc parts will still read garbage to R29 as explained in the >> FIXMEs there. >> >> Solving the FIXMEs can only be achieved by splitting multilibs avr2 and avr25, >> i.e. the mutlilibs that mix devices with/without SP.H, into avr2, avr21, avr24, >> avr25, say. >> >> I don't think it's a good idea to have real 8-bit SP/FP and that it would cause >> all sorts of trouble. > > I'm agree. > >> Ok to commit to 4.6? > > Approved. http://gcc.gnu.org/viewcvs?view=revision&revision=181936 Installed as gcc-4_6-branch r181936 with the following change: + { "attiny4313", ARCH_AVR25, "__AVR_ATtiny4313__", 0, 0x0060, "tn4313" }, { "attiny44", ARCH_AVR25, "__AVR_ATtiny44__", 0, 0x0060, "tn44" }, { "attiny44a", ARCH_AVR25, "__AVR_ATtiny44A__", 0, 0x0060, "tn44a" }, { "attiny84", ARCH_AVR25, "__AVR_ATtiny84__", 0, 0x0060, "tn84" }, @@ -88,7 +88,7 @@ const struct mcu_type_s avr_mcu_types[] { "attiny87", ARCH_AVR25, "__AVR_ATtiny87__", 0, 0x0100, "tn87" }, { "attiny48", ARCH_AVR25, "__AVR_ATtiny48__", 0, 0x0100, "tn48" }, { "attiny88", ARCH_AVR25, "__AVR_ATtiny88__", 0, 0x0100, "tn88" }, - { "at86rf401", ARCH_AVR25, "__AVR_AT86RF401__", 1, 0x0060, "86401" }, + { "at86rf401", ARCH_AVR25, "__AVR_AT86RF401__", 0, 0x0060, "86401" }, /* Classic, > 8K, <= 64K. */ { "avr3", ARCH_AVR3, NULL, 0, 0x0060, "43355" }, { "at43usb355", ARCH_AVR3, "__AVR_AT43USB355__", 0, 0x0060, "43355" }, As it turned out, ATtiny4313 and AT86RF401 have a 16-bit stack pointer and their manual is bogus in stating their SP has 8 bits only. This is not a complete fix to the SPH issue because PR51345 is still open: libgcc might happily read gabage into R29 from IO[0x3e]. Johann >> What about splitting multilibs? > > Seems that splitting multilibs is a right way. Opened PR51345 for it. >> Is this appropriate for 4.7? > > As I understand, any changes appropriate for our port in any stage. > >> Johann >> >> PR target/51002 >> * config/avr/libgcc.S (__prologue_saves__, __epilogue_restores__): >> Enclose parts using __SP_H__ in defined (__AVR_HAVE_8BIT_SP__). >> Add FIXME comments. >> * config/avr/avr.md (movhi_sp_r_irq_off, movhi_sp_r_irq_on): Set >> insn condition to !AVR_HAVE_8BIT_SP. >> * config/avr/avr.c (output_movhi): "clr%B0" instead of "in >> %B0,__SP_H__" if AVR_HAVE_8BIT_SP. >> (avr_file_start): Only print "__SP_H__ = 0x3e" if !AVR_HAVE_8BIT_SP. >> > > Denis. Index: config/avr/avr-devices.c =================================================================== --- config/avr/avr-devices.c (revision 181783) +++ config/avr/avr-devices.c (working copy) @@ -70,7 +70,7 @@ const struct mcu_type_s avr_mcu_types[] { "attiny2313a", ARCH_AVR25, "__AVR_ATtiny2313A__", 1, 0x0060, "tn2313a" }, { "attiny24", ARCH_AVR25, "__AVR_ATtiny24__", 1, 0x0060, "tn24" }, { "attiny24a", ARCH_AVR25, "__AVR_ATtiny24A__", 1, 0x0060, "tn24a" }, - { "attiny4313", ARCH_AVR25, "__AVR_ATtiny4313__", 1, 0x0060, "tn4313" },