From patchwork Mon Mar 5 15:56:38 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Georg-Johann Lay X-Patchwork-Id: 144707 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 77FDBB6F62 for ; Tue, 6 Mar 2012 03:01:12 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1331568075; h=Comment: DomainKey-Signature:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=dy81/HOtbFtinLeNCTI7eEPkGII=; b=bK6CJng4SURl6uu Lmlk93EMgNYvz+j089MX3GpVFwAmdUIQtvxrRb1pZwY6NRldsO8tLBTyKTf84SNb U2EVbsP5Sg8DO5P9YIQWRdrwoyjDFQRW/wjotulsNUsT0L4rlDc7egdOjDKJrDAR tMDWszQ7n1RishZEm0WN1uA5d1I0= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:X-RZG-AUTH:X-RZG-CLASS-ID:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=pf+SGF+6D2NxAMwzLuoplRJ4yu0/iny2xAm81PAaBVk3D7WHeVE7ArtuqQ5HKJ xHdo6SwK5qrZA9vZctSh+zD3fwq8RR+NihenUSUquA/+MNHpVMBGyM6i00Amo+kz 3nMFd20/MC8qndZ1t/XxekiXYQViJv1fTa3Oyb9h9q9CA=; Received: (qmail 15854 invoked by alias); 5 Mar 2012 16:01:02 -0000 Received: (qmail 15824 invoked by uid 22791); 5 Mar 2012 16:00:58 -0000 X-SWARE-Spam-Status: No, hits=0.0 required=5.0 tests=AWL, BAYES_50, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE, SARE_SUB_OBFU_Z, TW_LB, TW_RJ 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; Mon, 05 Mar 2012 16:00:36 +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 (fruni mo48) (RZmta 27.7 AUTH) with ESMTPA id R0306fo25Fo2cQ ; Mon, 5 Mar 2012 16:56:39 +0100 (MET) Message-ID: <4F54E236.2030907@gjlay.de> Date: Mon, 05 Mar 2012 16:56:38 +0100 From: Georg-Johann Lay User-Agent: Thunderbird 2.0.0.24 (X11/20100302) MIME-Version: 1.0 To: Richard Guenther CC: gcc-patches@gcc.gnu.org, Denis Chertykov , Eric Weddington Subject: Re: [Patch, AVR, trunk, 4.7] PR52461: Fix RAMPZ clobbering and RAMP* in epilogue References: <4F53B053.8070302@gjlay.de> <4F54C99A.7010305@gjlay.de> <4F54DAD6.3080209@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 Richard Guenther wrote: > On Mon, Mar 5, 2012 at 4:25 PM, Georg-Johann Lay wrote: >> Richard Guenther wrote: >> >>> All commits to the 4.7 branch need explicit release manager approval. AVR >>> isn't primary/secondary so please do not change anything before is >>> released 4.7.0 for it. >>> >>> Thanks, >>> Richard. >> What is the exact procedure in that case? >> Wait until approve from release manager in that case? >> Who is the release manager, and should I CC for such changes? >> Or just hope the patch is not overseen. > > The exact procedure is to do bugfixing during stage3/4, for release blockers > that pop up after a release candidate is created (like now), CC a release > manager (Jakub, me, Joseph) for patches that you like to get in even > though the branch is frozen. Usually only bugs that prevent basic functionality > (like building a target) can be fixed at this point, for everything > else you have > to wait until after 4.7.0 is released and the branch opens again for regression > fixes. > > Richard. I was not aware that the 4.7.0 branch is completely frozen for the next 3 weeks; I thought the usual rules for backporting patches do apply... The patch changes only in libgcc/config/avr and gcc/config/avr The patch does not fix a blocker in the sense that without it avr cannot be built, but the changes are essential. Johann libgcc/ PR target/52461 * config/avr/lib1funcs.S (__do_copy_data): Clear RAMPZ after usage if RAMPZ affects reading from RAM. (__tablejump_elpm__): Ditto. (.xload): Ditto. (__movmemx_hi): Ditto. (__do_global_ctors): Right condition for RAMPZ usage is "have ELPM". (__do_global_dtors): Ditto. (__xload_1, __xload_2, __xload_3, __xload_4): Ditto. And make weak. (__movmemx_hi): Ditto. And fix RAM-loop label. (__xload_1): Never read unintentionally from RAM. gcc/ PR target/52461 * gcc/config/avr/avr.c (expand_prologue): Depend save/restore of RAMPZ on HAVE_RAMPD, not HAVE_RAMPZ. (expand_epilogue): Ditto. And fix order of restoration to: RAMPZ, RAMPY, RAMPX, RAMPD. (avr_xload_libgcc_p): Always load __memx by lilbgcc call on big-RAM devices. (avr_out_lpm): Clear RAMPZ after usage if RAMPZ affects reading from RAM. (avr_out_xload): Never read unintentionally from RAM. * config/avr/avr.md (xload_8): Adjust insn length. Index: libgcc/config/avr/lib1funcs.S =================================================================== --- libgcc/config/avr/lib1funcs.S (revision 184887) +++ libgcc/config/avr/lib1funcs.S (working copy) @@ -1853,7 +1853,7 @@ DEFUN __do_copy_data cpi r26, lo8(__data_end) cpc r27, r17 brne .L__do_copy_data_loop -#elif !defined(__AVR_HAVE_ELPMX__) && defined(__AVR_HAVE_ELPM__) +#elif defined(__AVR_HAVE_ELPM__) ldi r17, hi8(__data_end) ldi r26, lo8(__data_start) ldi r27, hi8(__data_start) @@ -1873,7 +1873,7 @@ DEFUN __do_copy_data cpi r26, lo8(__data_end) cpc r27, r17 brne .L__do_copy_data_loop -#elif !defined(__AVR_HAVE_ELPMX__) && !defined(__AVR_HAVE_ELPM__) +#else /* !ELPM */ ldi r17, hi8(__data_end) ldi r26, lo8(__data_start) ldi r27, hi8(__data_start) @@ -1892,7 +1892,11 @@ DEFUN __do_copy_data cpi r26, lo8(__data_end) cpc r27, r17 brne .L__do_copy_data_loop -#endif /* !defined(__AVR_HAVE_ELPMX__) && !defined(__AVR_HAVE_ELPM__) */ +#endif /* ELPMX / ELPM / LPM */ +#if defined (__AVR_HAVE_ELPM__) && defined (__AVR_HAVE_RAMPD__) + ;; Reset RAMPZ to 0 so that EBI devices don't read garbage from RAM + out __RAMPZ__, __zero_reg__ +#endif /* ELPM && RAMPD */ ENDF __do_copy_data #endif /* L_copy_data */ @@ -1920,7 +1924,7 @@ ENDF __do_clear_bss #ifdef L_ctors .section .init6,"ax",@progbits DEFUN __do_global_ctors -#if defined(__AVR_HAVE_RAMPZ__) +#if defined(__AVR_HAVE_ELPM__) ldi r17, hi8(__ctors_start) ldi r28, lo8(__ctors_end) ldi r29, hi8(__ctors_end) @@ -1939,7 +1943,7 @@ DEFUN __do_global_ctors ldi r24, hh8(__ctors_start) cpc r16, r24 brne .L__do_global_ctors_loop -#else +#else /* !ELPM */ ldi r17, hi8(__ctors_start) ldi r28, lo8(__ctors_end) ldi r29, hi8(__ctors_end) @@ -1953,14 +1957,14 @@ DEFUN __do_global_ctors cpi r28, lo8(__ctors_start) cpc r29, r17 brne .L__do_global_ctors_loop -#endif /* defined(__AVR_HAVE_RAMPZ__) */ +#endif /* defined(__AVR_HAVE_ELPM__) */ ENDF __do_global_ctors #endif /* L_ctors */ #ifdef L_dtors .section .fini6,"ax",@progbits DEFUN __do_global_dtors -#if defined(__AVR_HAVE_RAMPZ__) +#if defined(__AVR_HAVE_ELPM__) ldi r17, hi8(__dtors_end) ldi r28, lo8(__dtors_start) ldi r29, hi8(__dtors_start) @@ -1979,7 +1983,7 @@ DEFUN __do_global_dtors ldi r24, hh8(__dtors_end) cpc r16, r24 brne .L__do_global_dtors_loop -#else +#else /* !ELPM */ ldi r17, hi8(__dtors_end) ldi r28, lo8(__dtors_start) ldi r29, hi8(__dtors_start) @@ -1993,7 +1997,7 @@ DEFUN __do_global_dtors cpi r28, lo8(__dtors_end) cpc r29, r17 brne .L__do_global_dtors_loop -#endif /* defined(__AVR_HAVE_RAMPZ__) */ +#endif /* defined(__AVR_HAVE_ELPM__) */ ENDF __do_global_dtors #endif /* L_dtors */ @@ -2001,18 +2005,21 @@ ENDF __do_global_dtors #ifdef L_tablejump_elpm DEFUN __tablejump_elpm__ -#if defined (__AVR_HAVE_ELPM__) -#if defined (__AVR_HAVE_LPMX__) +#if defined (__AVR_HAVE_ELPMX__) elpm __tmp_reg__, Z+ elpm r31, Z mov r30, __tmp_reg__ +#if defined (__AVR_HAVE_RAMPD__) + ;; Reset RAMPZ to 0 so that EBI devices don't read garbage from RAM + out __RAMPZ__, __zero_reg__ +#endif /* RAMPD */ #if defined (__AVR_HAVE_EIJMP_EICALL__) eijmp #else ijmp -#endif +#endif /* EICALL */ -#else +#elif defined (__AVR_HAVE_ELPM__) elpm adiw r30, 1 push r0 @@ -2021,10 +2028,9 @@ DEFUN __tablejump_elpm__ #if defined (__AVR_HAVE_EIJMP_EICALL__) in __tmp_reg__, __EIND__ push __tmp_reg__ -#endif +#endif /* EICALL */ ret -#endif -#endif /* defined (__AVR_HAVE_ELPM__) */ +#endif /* ELPM */ ENDF __tablejump_elpm__ #endif /* defined (L_tablejump_elpm) */ @@ -2114,11 +2120,19 @@ ENDF __load_4 adiw r30, 1 .endif #endif +#if defined (__AVR_HAVE_ELPM__) && defined (__AVR_HAVE_RAMPD__) +.if \dest == D0+\n-1 + ;; Reset RAMPZ to 0 so that EBI devices don't read garbage from RAM + out __RAMPZ__, __zero_reg__ +.endif +#endif .endm ; .xload #if defined (L_xload_1) DEFUN __xload_1 -#if defined (__AVR_HAVE_LPMX__) && !defined (__AVR_HAVE_RAMPZ__) +.weak __xload_1 +#if defined (__AVR_HAVE_LPMX__) && !defined (__AVR_HAVE_ELPM__) + sbrc HHI8, 7 ld D0, Z sbrs HHI8, 7 lpm D0, Z @@ -2126,24 +2140,25 @@ DEFUN __xload_1 #else sbrc HHI8, 7 rjmp 1f -#if defined (__AVR_HAVE_RAMPZ__) +#if defined (__AVR_HAVE_ELPM__) out __RAMPZ__, HHI8 -#endif /* __AVR_HAVE_RAMPZ__ */ +#endif /* __AVR_HAVE_ELPM__ */ .xload D0, 1 ret 1: ld D0, Z ret -#endif /* LPMx && ! RAMPZ */ +#endif /* LPMx && ! ELPM */ ENDF __xload_1 #endif /* L_xload_1 */ #if defined (L_xload_2) DEFUN __xload_2 +.weak __xload_2 sbrc HHI8, 7 rjmp 1f -#if defined (__AVR_HAVE_RAMPZ__) +#if defined (__AVR_HAVE_ELPM__) out __RAMPZ__, HHI8 -#endif /* __AVR_HAVE_RAMPZ__ */ +#endif /* __AVR_HAVE_ELPM__ */ .xload D0, 2 .xload D1, 2 ret @@ -2155,11 +2170,12 @@ ENDF __xload_2 #if defined (L_xload_3) DEFUN __xload_3 +.weak __xload_3 sbrc HHI8, 7 rjmp 1f -#if defined (__AVR_HAVE_RAMPZ__) +#if defined (__AVR_HAVE_ELPM__) out __RAMPZ__, HHI8 -#endif /* __AVR_HAVE_RAMPZ__ */ +#endif /* __AVR_HAVE_ELPM__ */ .xload D0, 3 .xload D1, 3 .xload D2, 3 @@ -2173,11 +2189,12 @@ ENDF __xload_3 #if defined (L_xload_4) DEFUN __xload_4 +.weak __xload_4 sbrc HHI8, 7 rjmp 1f -#if defined (__AVR_HAVE_RAMPZ__) +#if defined (__AVR_HAVE_ELPM__) out __RAMPZ__, HHI8 -#endif /* __AVR_HAVE_RAMPZ__ */ +#endif /* __AVR_HAVE_ELPM__ */ .xload D0, 4 .xload D1, 4 .xload D2, 4 @@ -2205,6 +2222,7 @@ ENDF __xload_4 #define LOOP 24 DEFUN __movmemx_qi +.weak __movmemx_qi ;; #Bytes to copy fity in 8 Bits (1..255) ;; Zero-extend Loop Counter clr LOOP+1 @@ -2212,6 +2230,7 @@ DEFUN __movmemx_qi ENDF __movmemx_qi DEFUN __movmemx_hi +.weak __movmemx_hi ;; Read from where? sbrc HHI8, 7 @@ -2219,7 +2238,7 @@ DEFUN __movmemx_hi ;; Read from Flash -#if defined (__AVR_HAVE_RAMPZ__) +#if defined (__AVR_HAVE_ELPM__) out __RAMPZ__, HHI8 #endif @@ -2243,6 +2262,10 @@ DEFUN __movmemx_hi st X+, r0 sbiw LOOP, 1 brne 0b +#if defined (__AVR_HAVE_ELPM__) && defined (__AVR_HAVE_RAMPD__) + ;; Reset RAMPZ to 0 so that EBI devices don't read garbage from RAM + out __RAMPZ__, __zero_reg__ +#endif /* ELPM && RAMPD */ ret ;; Read from RAM @@ -2252,7 +2275,7 @@ DEFUN __movmemx_hi ;; and store that Byte to RAM Destination st X+, r0 sbiw LOOP, 1 - brne 0b + brne 1b ret ENDF __movmemx_hi Index: gcc/config/avr/avr.md =================================================================== --- gcc/config/avr/avr.md (revision 184919) +++ gcc/config/avr/avr.md (working copy) @@ -363,6 +363,11 @@ (define_split ;;======================================================================== ;; Move stuff around +;; "loadqi_libgcc" +;; "loadhi_libgcc" +;; "loadpsi_libgcc" +;; "loadsi_libgcc" +;; "loadsf_libgcc" (define_expand "load_libgcc" [(set (match_dup 3) (match_dup 2)) @@ -377,7 +382,12 @@ (define_expand "load_libgcc" operands[1] = replace_equiv_address (operands[1], operands[3]); set_mem_addr_space (operands[1], ADDR_SPACE_FLASH); }) - + +;; "load_qi_libgcc" +;; "load_hi_libgcc" +;; "load_psi_libgcc" +;; "load_si_libgcc" +;; "load_sf_libgcc" (define_insn "load__libgcc" [(set (reg:MOVMODE 22) (match_operand:MOVMODE 0 "memory_operand" "m,m"))] @@ -418,6 +428,11 @@ (define_insn_and_split "xload8_A" DONE; }) +;; "xloadqi_A" +;; "xloadhi_A" +;; "xloadpsi_A" +;; "xloadsi_A" +;; "xloadsf_A" (define_insn_and_split "xload_A" [(set (match_operand:MOVMODE 0 "register_operand" "=r") (match_operand:MOVMODE 1 "memory_operand" "m")) @@ -461,7 +476,7 @@ (define_insn "xload_8" { return avr_out_xload (insn, operands, NULL); } - [(set_attr "length" "3,4") + [(set_attr "length" "4,4") (set_attr "adjust_len" "*,xload") (set_attr "isa" "lpmx,lpm") (set_attr "cc" "none")]) Index: gcc/config/avr/avr.c =================================================================== --- gcc/config/avr/avr.c (revision 184887) +++ gcc/config/avr/avr.c (working copy) @@ -1123,11 +1123,11 @@ expand_prologue (void) emit_push_sfr (rampy_rtx, false /* frame-related */, true /* clr */); } - if (AVR_HAVE_RAMPZ + if (AVR_HAVE_RAMPZ && TEST_HARD_REG_BIT (set, REG_Z) && TEST_HARD_REG_BIT (set, REG_Z + 1)) { - emit_push_sfr (rampz_rtx, false /* frame-related */, true /* clr */); + emit_push_sfr (rampz_rtx, false /* frame-related */, AVR_HAVE_RAMPD); } } /* is_interrupt is_signal */ @@ -1347,12 +1347,12 @@ expand_epilogue (bool sibcall_p) /* Restore RAMPZ/Y/X/D using tmp_reg as scratch. The conditions to restore them must be tha same as in prologue. */ - if (AVR_HAVE_RAMPX - && TEST_HARD_REG_BIT (set, REG_X) - && TEST_HARD_REG_BIT (set, REG_X + 1)) + if (AVR_HAVE_RAMPZ + && TEST_HARD_REG_BIT (set, REG_Z) + && TEST_HARD_REG_BIT (set, REG_Z + 1)) { emit_pop_byte (TMP_REGNO); - emit_move_insn (rampx_rtx, tmp_reg_rtx); + emit_move_insn (rampz_rtx, tmp_reg_rtx); } if (AVR_HAVE_RAMPY @@ -1364,12 +1364,12 @@ expand_epilogue (bool sibcall_p) emit_move_insn (rampy_rtx, tmp_reg_rtx); } - if (AVR_HAVE_RAMPZ - && TEST_HARD_REG_BIT (set, REG_Z) - && TEST_HARD_REG_BIT (set, REG_Z + 1)) + if (AVR_HAVE_RAMPX + && TEST_HARD_REG_BIT (set, REG_X) + && TEST_HARD_REG_BIT (set, REG_X + 1)) { emit_pop_byte (TMP_REGNO); - emit_move_insn (rampz_rtx, tmp_reg_rtx); + emit_move_insn (rampx_rtx, tmp_reg_rtx); } if (AVR_HAVE_RAMPD) @@ -2446,7 +2446,8 @@ avr_xload_libgcc_p (enum machine_mode mo int n_bytes = GET_MODE_SIZE (mode); return (n_bytes > 1 - || avr_current_device->n_flash > 1); + || avr_current_device->n_flash > 1 + || AVR_HAVE_RAMPD); } @@ -2762,7 +2763,14 @@ avr_out_lpm (rtx insn, rtx *op, int *ple break; /* POST_INC */ } /* switch CODE (addr) */ + + if (xop[4] == xstring_e && AVR_HAVE_RAMPD) + { + /* Reset RAMPZ to 0 so that EBI devices don't read garbage from RAM */ + avr_asm_len ("out __RAMPZ__,__zero_reg__", xop, plen, 1); + } + return ""; } @@ -2782,8 +2790,9 @@ avr_out_xload (rtx insn ATTRIBUTE_UNUSED if (plen) *plen = 0; - avr_asm_len ("ld %3,%a2" CR_TAB - "sbrs %1,7", xop, plen, 2); + avr_asm_len ("sbrc %1,7" CR_TAB + "ld %3,%a2" CR_TAB + "sbrs %1,7", xop, plen, 3); avr_asm_len (AVR_HAVE_LPMX ? "lpm %3,%a2" : "lpm", xop, plen, 1);