From patchwork Tue Nov 29 16:09:04 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: 128299 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 0C8B6B6F6F for ; Wed, 30 Nov 2011 03:10:08 +1100 (EST) Received: (qmail 23444 invoked by alias); 29 Nov 2011 16:09:58 -0000 Received: (qmail 23400 invoked by uid 22791); 29 Nov 2011 16:09:50 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE, TW_OV, TW_PG, TW_VH, TW_VQ 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 16:09:34 +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 mo19) (RZmta 26.10 AUTH) with ESMTPA id C04329nATFC1fH ; Tue, 29 Nov 2011 17:09:05 +0100 (MET) Message-ID: <4ED503A0.9070908@gjlay.de> Date: Tue, 29 Nov 2011 17:09:04 +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] Fix PR51002 (gcc part) References: <4ED4BEDE.4000600@gjlay.de> In-Reply-To: <4ED4BEDE.4000600@gjlay.de> 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 Georg-Johann Lay wrote: > 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. > This is similar patch for trunk: libgcc/ PR target/51345 PR target/51002 * config/avr/lib1funcs.S (__prologue_saves__, __epilogue_restores__, __divdi3_moddi3): Enclose parts using __SP_H__ in defined (__AVR_HAVE_8BIT_SP__). Add FIXME comments. gcc/ PR target/51002 * config/avr/avr.md (movhi_sp_r): Set insn condition to !AVR_HAVE_8BIT_SP. * config/avr/avr.c (output_movhi): Use "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: gcc/config/avr/avr.md =================================================================== --- gcc/config/avr/avr.md (revision 181773) +++ gcc/config/avr/avr.md (working copy) @@ -620,7 +620,7 @@ (define_insn "movhi_sp_r" (unspec_volatile:HI [(match_operand:HI 1 "register_operand" "r,r") (match_operand:HI 2 "const_int_operand" "L,P")] UNSPECV_WRITE_SP))] - "" + "!AVR_HAVE_8BIT_SP" "@ out __SP_H__,%B1\;out __SP_L__,%A1 cli\;out __SP_H__,%B1\;sei\;out __SP_L__,%A1" Index: gcc/config/avr/avr.c =================================================================== --- gcc/config/avr/avr.c (revision 181773) +++ gcc/config/avr/avr.c (working copy) @@ -2872,78 +2872,77 @@ output_movqi (rtx insn, rtx operands[], const char * -output_movhi (rtx insn, rtx operands[], int *l) +output_movhi (rtx insn, rtx xop[], int *plen) { - int dummy; - rtx dest = operands[0]; - rtx src = operands[1]; - int *real_l = l; + rtx dest = xop[0]; + rtx src = xop[1]; + + gcc_assert (GET_MODE_SIZE (GET_MODE (dest)) == 2); if (avr_mem_pgm_p (src) || avr_mem_pgm_p (dest)) { - return avr_out_lpm (insn, operands, real_l); + return avr_out_lpm (insn, xop, plen); } - if (!l) - l = &dummy; - - if (register_operand (dest, HImode)) + if (REG_P (dest)) { - if (register_operand (src, HImode)) /* mov r,r */ - { - if (test_hard_reg_class (STACK_REG, dest)) - { - if (AVR_HAVE_8BIT_SP) - return *l = 1, AS2 (out,__SP_L__,%A1); - /* Use simple load of stack pointer if no interrupts are - used. */ - else if (TARGET_NO_INTERRUPTS) - return *l = 2, (AS2 (out,__SP_H__,%B1) CR_TAB - AS2 (out,__SP_L__,%A1)); - *l = 5; - return (AS2 (in,__tmp_reg__,__SREG__) CR_TAB - "cli" CR_TAB - AS2 (out,__SP_H__,%B1) CR_TAB - AS2 (out,__SREG__,__tmp_reg__) CR_TAB - AS2 (out,__SP_L__,%A1)); - } - else if (test_hard_reg_class (STACK_REG, src)) - { - *l = 2; - return (AS2 (in,%A0,__SP_L__) CR_TAB - AS2 (in,%B0,__SP_H__)); - } + if (REG_P (src)) /* mov r,r */ + { + if (test_hard_reg_class (STACK_REG, dest)) + { + if (AVR_HAVE_8BIT_SP) + return avr_asm_len ("out __SP_L__,%A1", xop, plen, -1); + + /* Use simple load of SP if no interrupts are used. */ + + return TARGET_NO_INTERRUPTS + ? avr_asm_len ("out __SP_H__,%B1" CR_TAB + "out __SP_L__,%A1", xop, plen, -2) + + : avr_asm_len ("in __tmp_reg__,__SREG__" CR_TAB + "cli" CR_TAB + "out __SP_H__,%B1" CR_TAB + "out __SREG__,__tmp_reg__" CR_TAB + "out __SP_L__,%A1", xop, plen, -5); + } + else if (test_hard_reg_class (STACK_REG, src)) + { + return AVR_HAVE_8BIT_SP + ? avr_asm_len ("in %A0,__SP_L__" CR_TAB + "clr %B0", xop, plen, -2) + + : avr_asm_len ("in %A0,__SP_L__" CR_TAB + "in %B0,__SP_H__", xop, plen, -2); + } - if (AVR_HAVE_MOVW) - { - *l = 1; - return (AS2 (movw,%0,%1)); - } - else - { - *l = 2; - return (AS2 (mov,%A0,%A1) CR_TAB - AS2 (mov,%B0,%B1)); - } - } + return AVR_HAVE_MOVW + ? avr_asm_len ("movw %0,%1", xop, plen, -1) + + : avr_asm_len ("mov %A0,%A1" CR_TAB + "mov %B0,%B1", xop, plen, -2); + } /* REG_P (src) */ else if (CONSTANT_P (src)) { - return output_reload_inhi (operands, NULL, real_l); + return output_reload_inhi (xop, NULL, plen); + } + else if (MEM_P (src)) + { + return out_movhi_r_mr (insn, xop, plen); /* mov r,m */ } - else if (GET_CODE (src) == MEM) - return out_movhi_r_mr (insn, operands, real_l); /* mov r,m */ } - else if (GET_CODE (dest) == MEM) + else if (MEM_P (dest)) { rtx xop[2]; xop[0] = dest; xop[1] = src == const0_rtx ? zero_reg_rtx : src; - return out_movhi_mr_r (insn, xop, real_l); + return out_movhi_mr_r (insn, xop, plen); } + fatal_insn ("invalid insn:", insn); + return ""; } @@ -7272,16 +7271,19 @@ avr_file_start (void) default_file_start (); + if (!AVR_HAVE_8BIT_SP) + fprintf (asm_out_file, + "__SP_H__ = 0x%02x\n" + -sfr_offset + SP_ADDR + 1); + fprintf (asm_out_file, - "__SREG__ = 0x%02x\n" - "__SP_H__ = 0x%02x\n" "__SP_L__ = 0x%02x\n" + "__SREG__ = 0x%02x\n" "__RAMPZ__ = 0x%02x\n" "__tmp_reg__ = %d\n" "__zero_reg__ = %d\n", - -sfr_offset + SREG_ADDR, - -sfr_offset + SP_ADDR + 1, -sfr_offset + SP_ADDR, + -sfr_offset + SREG_ADDR, -sfr_offset + RAMPZ_ADDR, TMP_REGNO, ZERO_REGNO); Index: libgcc/config/avr/lib1funcs.S =================================================================== --- libgcc/config/avr/lib1funcs.S (revision 181700) +++ libgcc/config/avr/lib1funcs.S (working copy) @@ -1149,7 +1149,14 @@ DEFUN __divdi3_moddi3 4: ;; Epilogue: Restore the Z = 12 Registers and return in r28, __SP_L__ +#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). + clr r29 +#else in r29, __SP_H__ +#endif /* #SP = 8/16 */ ldi r30, 12 XJMP __epilogue_restores__ + ((18 - 12) * 2) @@ -1229,6 +1236,15 @@ DEFUN __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 + out __SP_L__,r28 + clr r29 +#else in r28,__SP_L__ in r29,__SP_H__ sub r28,r26 @@ -1238,6 +1254,8 @@ DEFUN __prologue_saves__ out __SP_H__,r29 out __SREG__,__tmp_reg__ out __SP_L__,r28 +#endif /* #SP = 8/16 */ + #if defined (__AVR_HAVE_EIJMP_EICALL__) eijmp #else @@ -1270,6 +1288,15 @@ DEFUN __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__ @@ -1280,6 +1307,7 @@ DEFUN __epilogue_restores__ out __SP_L__,r28 mov_l r28, r26 mov_h r29, r27 +#endif /* #SP = 8/16 */ ret ENDF __epilogue_restores__ #endif /* defined (L_epilogue) */