From patchwork Thu Nov 17 12:22:18 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: 126197 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 43083B71FF for ; Thu, 17 Nov 2011 23:23:49 +1100 (EST) Received: (qmail 17709 invoked by alias); 17 Nov 2011 12:23:41 -0000 Received: (qmail 17549 invoked by uid 22791); 17 Nov 2011 12:23:27 -0000 X-SWARE-Spam-Status: No, hits=1.6 required=5.0 tests=AWL, BAYES_50, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, KAM_VIAGRA1, RCVD_IN_DNSWL_NONE, TW_BX, TW_GM, TW_OV, TW_PG, TW_RJ, TW_SB, 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; Thu, 17 Nov 2011 12:23:00 +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 mo37) (RZmta 26.10 AUTH) with ESMTPA id 404b30nAHBxBhy ; Thu, 17 Nov 2011 13:22:22 +0100 (MET) Message-ID: <4EC4FC7A.9080309@gjlay.de> Date: Thu, 17 Nov 2011 13:22:18 +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, Eric Weddington , Joerg Wunsch , Anatoly Sokolov Subject: Re: [Patch]: PR49868: Named address space support for AVR, #5 References: <4E8DC2C0.8040703@gjlay.de> <4EAA8450.9080207@gjlay.de> <4EB6D44C.2020609@gjlay.de> <4EB7BC8A.8000409@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: > Let's wait for > http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01874.html > Denis. This are yet more intrinsic named address spaces: * __pgm1, ... __pgm5 are 16-bit address spaces that refer to the n-th 64k chunk of flash. Counting starts at 0. The 0-th address space __pgm is already upstream. * __pgmx is a 24-bit address space located in flash. The annoyance in this patch is the movmemhi insn: * Register allocator does a bad job and might lead to spills inside the copy loop so that it is no more guaranteed that tmp_reg contains the value to be copied because move insns use that register implicitly. Besides that, spilling in the copy loop leads to unfortunate code. See respective FIXMEs in avr.c:avr_emit_movmemhi() * Using match_dup in respective patterns shreds web.c to that the insns need up to with 11 operands. Besides that there are caveats and binutils is missing some support: * To place variables in __pgm1 ... __pgm5 in appropriate sections a custom linker script is needed. Data is put in sections progmem1.data ... __progmem5.data, respectively, and these sections must be treated in the linker script. * Address computation for the 24-bit address space is performed as signed 16-bit. Thus, accessing an array var[i] for example it is not possible to reach locations that are farther away than +/- 32768 bytes from var[0]. * It is not possible to assemble a 24-bit address, see the assembler warning generated in avr.c:avr_assemble_integer(). This warning is triggered for code like extern const __pgmx int ivar; const __pgmx void * var = &ivar; .global var .data .type var, @object .size var, 3 var: .word ivar .warning "24-bit address needs binutils extension for hh8(ivar)" .byte 0 ; hh8(ivar) The patch passes C tests with one FAIL less (SVN 181349): gcc.dg/pr43300.c (internal compiler error) ./gcc.dg/pr43300.c: In function 'foo': ./gcc.dg/pr43300.c:19:1: internal compiler error: in commit_one_edge_insertion, at cfgrtl.c:1582 This tests passes now; seems that the changes to movmemhi allow that test to PASS now (for whatever reason). Ok for trunk? Johann gcc/ PR target/49868 * config/avr/avr.h (base_arch_s): Add field n_segments. (ADDR_SPACE_PGM1, ADDR_SPACE_PGM2, ADDR_SPACE_PGM3, ADDR_SPACE_PGM4, ADDR_SPACE_PGM5, ADDR_SPACE_PGMX): New address spaces. (AVR_HAVE_ELPM, AVR_HAVE_ELPMX): New defines. (INIT_EXPANDERS): New define. * config/avr/avr-protos.h (avr_mem_pgmx_p): New. (avr_init_expanders): New. (avr_emit_movmemhi, avr_out_movmem): New. (avr_xload_libgcc_p): New. * config/avr/avr-c.c (avr_register_target_pragmas): Register address spaces __pgm1, __pgm2, __pgm3, __pgm4 __pgm5, __pgmx. (avr_cpu_cpp_builtins): Add built-in defines __PGM1, __PGM2, __PGM3, __PGM4, __PGM5, __PGMX. * config/avr/avr-devices.c (avr_arch_types): Set field n_segments. * config/avr/avr.c (AVR_SECTION_PROGMEM): Change define to cover 3 bits instead of just 1. (xstring_empty, xstring_e, rampz_rtx): New static GTYed variables. (progmem_section): Change from section to array of sections. (progmem_section_prefix): New static variable. (avr_file_start): Print set for __RAMPZ__ (avr_option_override): Move initialization of RTXes from here... (avr_init_expanders): ...to this new function. (avr_pgm_segment): New static function. (avr_decl_pgm_p): Handle error_mark_node. (avr_mem_pgmx_p, avr_decl_pgmx_p): New static functions. (avr_out_xload, avr_find_unused_d_reg): New static functions. (expand_prologue, expand_epilogue): Use rampz_rtx. (print_operand): Hande CONST_STRING. (avr_xload_libgcc_p): New static function. (avr_out_lpm_no_lpmx, avr_out_lpm): Handle ELPM. (avr_progmem_p): Return 2 for 24-bit flash address space. (avr_out_sbxx_branch): Clean-up code from ASn macros. (out_movqi_r_mr, out_movqi_mr_r): Ditto. And recognize RAMPZ's address and print symbolically. (avr_asm_named_section, avr_section_type_flags, avr_encode_section_info, avr_asm_select_section, avr_addr_space_address_mode, avr_addr_space_pointer_mode, avr_addr_space_legitimate_address_p, avr_addr_space_convert, avr_addr_space_legitimize_address): Handle new address spaces. (avr_output_progmem_section_asm_op): New static function. (avr_asm_init_sections): Initialize progmem_section[]. (adjust_insn_length): Handle ADJUST_LEN_XLOAD, ADJUST_LEN_MOVMEM. (avr_const_address_lo16): New static function. (avr_assemble_integer): Use it to handle 3-byte integers. (avr_emit_movmemhi, avr_out_movmem): New functions. * config/avr/constraints.md (Cpp): New constraint. * config/avr/predicates.md (nox_general_operand): Handle new address spaces. * config/avr/avr.md (unspec): Add UNSPEC_MOVMEM. (adjust_len): Add xload, movmem. (SP_ADDR): New define_constants. (isa): Add "lpm", "lpmx", "elpm", "elpmx". (enabled): Handle them. (load_libgcc): New expander. (*load..libgcc): Rename to load__libgcc. (xload8_A, xload_A, n_extendhipsi2): New insn-and-splits. (xload_8, xload__libgcc, xload_, loadmem_elpm): New insns. (mov): Handle new address spaces. (movmemhi): Rewrite using avr_emit_movmemhi. (MOVMEM_r_d): New mode attribute. (movmem_, movmem_qi_elpm): New insns. (setmemhi, *clrmemqi, *clrmemhi, strlenhi, *strlenhi): Unquote C-code. Use label instead of hard-coded instrunction lengths. libgcc/ PR target/49868 * config/avr/t-avr (LIB1ASMFUNCS): Add _xload_2 _xload_3 _xload_4. * config/avr/lib1funcs.S (__xload_2, __xload_3, __xload_4): New functions. Index: ../libgcc/config/avr/lib1funcs.S =================================================================== --- ../libgcc/config/avr/lib1funcs.S (revision 181378) +++ ../libgcc/config/avr/lib1funcs.S (working copy) @@ -1227,6 +1227,73 @@ ENDF __load_4 #endif /* L_load_3 || L_load_3 */ +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Loading n bytes from Flash; n = 2,3,4 +;; R22... = Flash[R21:Z] +;; Clobbers: __tmp_reg__, R21, R30, R31 + +#if (defined (L_xload_2) \ + || defined (L_xload_3) \ + || defined (L_xload_4)) \ + && defined (__AVR_HAVE_ELPM__) \ + && !defined (__AVR_HAVE_ELPMX__) + +#if !defined (__AVR_HAVE_RAMPZ__) +#error Need RAMPZ +#endif /* have RAMPZ */ + +;; Destination +#define D0 22 +#define D1 D0+1 +#define D2 D0+2 +#define D3 D0+3 + +;; Register containing bits 16+ of the address + +#define HHI8 21 + +.macro .xload dest, n + elpm + mov \dest, r0 +.if \dest != D0+\n-1 + adiw r30, 1 + adc HHI8, __zero_reg__ + out __RAMPZ__, HHI8 +.endif +.endm + +#if defined (L_xload_2) +DEFUN __xload_2 + out __RAMPZ__, HHI8 + .xload D0, 2 + .xload D1, 2 + ret +ENDF __xload_2 +#endif /* L_xload_2 */ + +#if defined (L_xload_3) +DEFUN __xload_3 + out __RAMPZ__, HHI8 + .xload D0, 3 + .xload D1, 3 + .xload D2, 3 + ret +ENDF __xload_3 +#endif /* L_xload_3 */ + +#if defined (L_xload_4) +DEFUN __xload_4 + out __RAMPZ__, HHI8 + .xload D0, 4 + .xload D1, 4 + .xload D2, 4 + .xload D3, 4 + ret +ENDF __xload_4 +#endif /* L_xload_4 */ + +#endif /* L_xload_{2|3|4} && ELPM */ + .section .text.libgcc.builtins, "ax", @progbits Index: ../libgcc/config/avr/t-avr =================================================================== --- ../libgcc/config/avr/t-avr (revision 181378) +++ ../libgcc/config/avr/t-avr (working copy) @@ -22,6 +22,7 @@ LIB1ASMFUNCS = \ _tablejump \ _tablejump_elpm \ _load_3 _load_4 \ + _xload_2 _xload_3 _xload_4 \ _copy_data \ _clear_bss \ _ctors \