From patchwork Tue Nov 29 11:15:42 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: 128278 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 58F62B6F6F for ; Tue, 29 Nov 2011 22:16:22 +1100 (EST) Received: (qmail 11839 invoked by alias); 29 Nov 2011 11:16:20 -0000 Received: (qmail 11826 invoked by uid 22791); 29 Nov 2011 11:16:18 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE, TW_OV, TW_VH 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; Tue, 29 Nov 2011 11:16:01 +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 (klopstock mo59) (RZmta 26.10 AUTH) with ESMTPA id g057f8nATAr8mn ; Tue, 29 Nov 2011 12:15:43 +0100 (MET) Message-ID: <4ED4BEDE.4000600@gjlay.de> Date: Tue, 29 Nov 2011 12:15:42 +0100 From: Georg-Johann Lay User-Agent: Thunderbird 2.0.0.24 (X11/20100302) MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org CC: Denis Chertykov , Wim Lewis , Eric Weddington , Joerg Wunsch Subject: [Patch.AVR,4.6] Fix PR51002 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 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. Ok to commit to 4.6? What about splitting multilibs? Is this appropriate for 4.7? 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. Index: config/avr/libgcc.S =================================================================== --- config/avr/libgcc.S (revision 181783) +++ config/avr/libgcc.S (working copy) @@ -582,6 +582,15 @@ __prologue_saves__: push r17 push r28 push r29 +#if defined (__AVR_HAVE_8BIT_SP__) +;; FIXME: __AVR_HAVE_8BIT_SP__ is set on device level, not on core level +;; so this lines are dead code. To make it work, devices without +;; SP_H must get their own multilib(s). + in r28,__SP_L__ + sub r28,r26 + clr r29 + out __SP_L__,r28 +#else in r28,__SP_L__ in r29,__SP_H__ sub r28,r26 @@ -591,6 +600,7 @@ __prologue_saves__: out __SP_H__,r29 out __SREG__,__tmp_reg__ out __SP_L__,r28 +#endif #if defined (__AVR_HAVE_EIJMP_EICALL__) eijmp #else @@ -625,6 +635,15 @@ __epilogue_restores__: ldd r16,Y+4 ldd r17,Y+3 ldd r26,Y+2 +#if defined (__AVR_HAVE_8BIT_SP__) +;; FIXME: __AVR_HAVE_8BIT_SP__ is set on device level, not on core level +;; so this lines are dead code. To make it work, devices without +;; SP_H must get their own multilib(s). + ldd r29,Y+1 + add r28,r30 + out __SP_L__,r28 + mov r28, r26 +#else ldd r27,Y+1 add r28,r30 adc r29,__zero_reg__ @@ -635,6 +654,7 @@ __epilogue_restores__: out __SP_L__,r28 mov_l r28, r26 mov_h r29, r27 +#endif ret .endfunc #endif /* defined (L_epilogue) */ Index: config/avr/avr.md =================================================================== --- config/avr/avr.md (revision 181783) +++ config/avr/avr.md (working copy) @@ -299,7 +299,7 @@ (define_insn "movhi_sp_r_irq_off" [(set (match_operand:HI 0 "stack_register_operand" "=q") (unspec_volatile:HI [(match_operand:HI 1 "register_operand" "r")] UNSPECV_WRITE_SP_IRQ_OFF))] - "" + "!AVR_HAVE_8BIT_SP" "out __SP_H__, %B1 out __SP_L__, %A1" [(set_attr "length" "2") @@ -309,7 +309,7 @@ (define_insn "movhi_sp_r_irq_on" [(set (match_operand:HI 0 "stack_register_operand" "=q") (unspec_volatile:HI [(match_operand:HI 1 "register_operand" "r")] UNSPECV_WRITE_SP_IRQ_ON))] - "" + "!AVR_HAVE_8BIT_SP" "cli out __SP_H__, %B1 sei Index: config/avr/avr.c =================================================================== --- config/avr/avr.c (revision 181783) +++ config/avr/avr.c (working copy) @@ -1879,9 +1879,12 @@ output_movhi (rtx insn, rtx operands[], } else if (test_hard_reg_class (STACK_REG, src)) { - *l = 2; - return (AS2 (in,%A0,__SP_L__) CR_TAB - AS2 (in,%B0,__SP_H__)); + *l = 2; + return AVR_HAVE_8BIT_SP + ? (AS2 (in,%A0,__SP_L__) CR_TAB + AS1 (clr,%B0)) + : (AS2 (in,%A0,__SP_L__) CR_TAB + AS2 (in,%B0,__SP_H__)); } if (AVR_HAVE_MOVW) @@ -5173,10 +5176,10 @@ avr_file_start (void) default_file_start (); -/* fprintf (asm_out_file, "\t.arch %s\n", avr_mcu_name);*/ - fputs ("__SREG__ = 0x3f\n" - "__SP_H__ = 0x3e\n" - "__SP_L__ = 0x3d\n", asm_out_file); + fputs ("__SREG__ = 0x3f\n", asm_out_file); + if (!AVR_HAVE_8BIT_SP) + fputs ("__SP_H__ = 0x3e\n", asm_out_file); + fputs ("__SP_L__ = 0x3d\n", asm_out_file); fputs ("__tmp_reg__ = 0\n" "__zero_reg__ = 1\n", asm_out_file);