From patchwork Sun Nov 6 18:39:08 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: 123955 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 B05A0B6F62 for ; Mon, 7 Nov 2011 05:44:19 +1100 (EST) Received: (qmail 5631 invoked by alias); 6 Nov 2011 18:44:16 -0000 Received: (qmail 5596 invoked by uid 22791); 6 Nov 2011 18:44:07 -0000 X-SWARE-Spam-Status: No, hits=0.1 required=5.0 tests=AWL, BAYES_50, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE, TW_GM, TW_HR, TW_LQ, TW_OV, TW_PG, TW_RQ, 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.162) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 06 Nov 2011 18:43:45 +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 mo59) (RZmta 26.10 AUTH) with ESMTPA id g03d75nA6IHjsV ; Sun, 6 Nov 2011 19:39:09 +0100 (MET) Message-ID: <4EB6D44C.2020609@gjlay.de> Date: Sun, 06 Nov 2011 19:39:08 +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, Ulrich Weigand , Eric Weddington , Joerg Wunsch , Anatoly Sokolov Subject: Re: [Patch]: PR49868: Named address space support for AVR References: <4E8DC2C0.8040703@gjlay.de> <4EAA8450.9080207@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/10/28 Georg-Johann Lay : >> Georg-Johann Lay schrieb: >> >>> This patch adds named address space support to read data from flash (aka. >>> progmem) to target AVR. >>> >>> The patch has two parts: >>> >>> The first part is a repost of Ulrich's work from >>> http://gcc.gnu.org/ml/gcc/2011-08/msg00131.html >>> with the needed changes to ./gcc and ./gcc/doc >>> >>> This patch is needed because the target hooks MODE_CODE_BASE_REG_CLASS and >>> REGNO_MODE_CODE_OK_FOR_BASE_P don't distinguish between different address >>> spaces. Ulrich's patch adds respective support to these hooks. >>> >>> The second part is the AVR dependent part that adds __pgm as address space >>> qualifier for address space AS1. >>> >>> The AVR part is just the worker code. If there is agreement that AS support >>> for AVR is okay in principle and Ulrich's work will go into GCC, I will supply >>> test programs and updates to the user manual, of course. >>> >>> The major drawbacks of the current AS implementation are: >>> >>> - It works only for C. >>> For C++, a language extension would be needed as indicated in >>> ISO/IEC DTR 18037 >>> Annex F - C++ Compatibility and Migration issues >>> F.2 Multiple Address Spaces Support >>> >>> - Register allocation does not a good job. AS1 can only be addressed >>> byte-wise by one single address register (Z) as per *Z or *Z++. >> This flaw from register allocator are filed as PR50775 now. >> >>> The AVR part does several things: >>> >>> - It locates data in AS1 into appropriate section, i.e. somewhere in >>> .progmem >>> >>> - It does early sanity checks to ensure that __pgm is always accompanied >>> with const so that writing to AS1 in not possible. >>> >>> - It prints LPM instructions to access flash memory. >> The attached patch is an update merge so that it fits without conflicts. >> >> The patch requires Ulrich's works which is still in review >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50775 >> >> The regression tests run with this patch and the new ChangeLog enttry si >> written as if Ulrich's patch was applied. >> >> Tests pass without regression. >> >> Besides the update to a nor up-to-date SVN version, the patch sets a built-in >> define __PGM so that it is easy for users to test if or if not the feature is >> available. >> >> Documentation and test cases will follow in separate patch. >> >> Ok for trunk after Ulrich's work has been approved? >> >> Johann >> >> PR target/49868 >> * config/avr/avr.h (ADDR_SPACE_PGM): New define for address space AS1. >> (REGISTER_TARGET_PRAGMAS): New define. >> * config/avr/avr-protos.h (avr_mem_pgm_p): New prototype. >> (avr_register_target_pragmas): New prototype. >> (avr_log_t): Add field "progmem". Order alphabetically. >> * config/avr/avr-log.c (avr_log_set_avr_log): Set avr_log.progmem. >> * config/avr/avr-c.c (langhooks.h): New include. >> (avr_register_target_pragmas): New function. Register address >> space AS1 as "__pgm". >> (avr_cpu_cpp_builtins): Add built-in define __PGM. >> * config/avr/avr.c: Include "c-family/c-common.h". >> (TARGET_LEGITIMATE_ADDRESS_P): Remove define. >> (TARGET_LEGITIMIZE_ADDRESS): Remove define. >> (TARGET_ADDR_SPACE_SUBSET_P): Define to... >> (avr_addr_space_subset_p): ...this new static function. >> (TARGET_ADDR_SPACE_CONVERT): Define to... >> (avr_addr_space_convert): ...this new static function. >> (TARGET_ADDR_SPACE_ADDRESS_MODE): Define to... >> (avr_addr_space_address_mode): ...this new static function. >> (TARGET_ADDR_SPACE_POINTER_MODE): Define to... >> (avr_addr_space_pointer_mode): ...this new static function. >> (TARGET_ADDR_SPACE_LEGITIMATE_ADDRESS_P): Define to... >> (avr_addr_space_legitimate_address_p): ...this new static function. >> (TARGET_ADDR_SPACE_LEGITIMIZE_ADDRESS): Define to... >> (avr_addr_space_legitimize_address): ...this new static function. >> (avr_mode_code_base_reg_class): Handle AS1. >> (avr_regno_mode_code_ok_for_base_p): Handle AS1. >> (lpm_addr_reg_rtx, lpm_reg_rtx): New static GTYed variables. >> (avr_decl_pgm_p): New static function. >> (avr_mem_pgm_p): New function. >> (avr_asm_len): Return always "" instead of void. >> (avr_out_lpm_no_lpmx): New static function. >> (avr_out_lpm): New static function. >> (output_movqi, output_movhi, output_movsisf): Call avr_out_lpm to >> handle loads from progmem. >> (avr_progmem_p): Test if decl is in AS1. >> (avr_pgm_pointer_const_p): New static function. >> (avr_pgm_check_var_decl): New static function. >> (avr_insert_attributes): Use it. Change error message to report >> cause (progmem or AS1) when code wants to write to AS1. >> (avr_section_type_flags): Unset section flag SECTION_BSS for >> data in progmem. >> * config/avr/avr.md (LPM_REGNO): New define_constants. >> (movqi, movhi, movsi, movsf): Skip if code would write to AS1. >> (movmemhi): Ditto. Propagate address space information to newly >> created MEM. >> (split-lpmx): New split. >> > > Approved. > > Denis. The patch from above is still not upstream because it needs the following work which is still pending review: http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01874.html The patch you find in this message contains the above patch with the following minor changes: * The patch is rebased to a newer revision * avr_out_lpm_no_lpmx and avr_out_lpm can also load the new 3-byte PSImode that got upstream meanwhile. * If there is no LMPX instruction and there are more than 2 bytes to load, a libgcc function is called. Moreover, the patch introduces the following major changes: * There is support for a new 24-bit address space called __pgmx * There is support for new 16-bit address spaces that represent the different 64k flash segments. Their names are __pgm2 .. __pgm5 * There are libgcc functions that load from flash for the expensive cases, i.e. there is no ELPMX. The reason why the __pgm address spaces are there is because there is very little to do to support them and there is no need for 24-bit pointers. The work is not yet complete, but I'd like to post it before stage 1 is over. Sorry for the inconvenience for just another review of the matter... What's missing is selection of sections for the numbered address spaces. The libgcc is independent of the not-yet-reviewed patch from Ulrich. Is the libgcc part ok for trunk? Is the gcc part ok provided Ulrich had been reviewed? I'd like to do the remaining as follow-up patches: * Release Notes * Documentation * Test cases * Section handling for numbered address spaces Johann gcc/ PR target/49868 * config/avr/avr.h (base_arch_s): Add field n_segments. (ADDR_SPACE_PGM, 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. (REGISTER_TARGET_PRAGMAS): New define. * config/avr/avr-protos.h (avr_mem_pgm_p, avr_mem_pgmx_p): New. (avr_out_xload, avr_xload_libgcc_p): New. (asm_output_external_libcall): Remove. (avr_register_target_pragmas): New. (avr_log_t): Add field "progmem". Order alphabetically. * config/avr/avr-log.c (avr_log_set_avr_log): Set avr_log.progmem. * config/avr/avr-c.c (langhooks.h): New include. (avr_register_target_pragmas): New function. Register address spaces __pgm, __pgm2, __pgm3, __pgm4 __pgm5, __pgmx. (avr_cpu_cpp_builtins): Add built-in defines __PGM, __PGM1, __PGM2, __PGM3, __PGM4, __PGM5, __PGMX. * config/avr/avr-devices.c (avr_arch_types): Set field n_segments. * config/avr/avr.c: Include "c-family/c-common.h". (TARGET_LEGITIMATE_ADDRESS_P): Remove define. (TARGET_LEGITIMIZE_ADDRESS): Remove define. (TARGET_ADDR_SPACE_SUBSET_P): Define to... (avr_addr_space_subset_p): ...this new static function. (TARGET_ADDR_SPACE_CONVERT): Define to... (avr_addr_space_convert): ...this new static function. (TARGET_ADDR_SPACE_ADDRESS_MODE): Define to... (avr_addr_space_address_mode): ...this new static function. (TARGET_ADDR_SPACE_POINTER_MODE): Define to... (avr_addr_space_pointer_mode): ...this new static function. (TARGET_ADDR_SPACE_LEGITIMATE_ADDRESS_P): Define to... (avr_addr_space_legitimate_address_p): ...this new static function. (TARGET_ADDR_SPACE_LEGITIMIZE_ADDRESS): Define to... (avr_addr_space_legitimize_address): ...this new static function. (avr_mode_code_base_reg_class): Handle address spaces. (avr_regno_mode_code_ok_for_base_p): Ditto. (lpm_addr_reg_rtx, lpm_reg_rtx, xstring_empty, xstring_e, all_regs_rtx): New static GTYed variables. (avr_option_override): Initialize them. (avr_pgm_segment): New static function. (avr_decl_pgm_p, avr_decl_pgmx_p): New static functions. (avr_mem_pgm_p, avr_mem_pgmx_p): New functions. (avr_asm_len): Return always "" instead of void. (avr_out_lpm, avr_out_lpm_no_lpmx, avr_out_xload, avr_find_unused_d_reg): New static functions. (output_movqi, output_movhi, output_movsisf, avr_out_movpsi): Call avr_out_lpm to handle loads from progmem. (print_operand): Hande CONST_STRING. (avr_xload_libgcc_p): New static function. (avr_progmem_p): Test if decl is in flash. (avr_pgm_pointer_const_p): New static function. (avr_nonconst_pointer_addrspace): New static function. (avr_pgm_check_var_decl): New static function. (avr_insert_attributes): Use it. Change error message to report cause (progmem or address space) when code wants to write to flash. (avr_section_type_flags): Unset section flag SECTION_BSS for data in progmem. (adjust_insn_length): Handle ADJUST_LEN_XLOAD. (avr_const_address_lo16): New static function. (avr_assemble_integer): Use it to handle 3-byte integers. (avr_file_star): Print set for __RAMPZ__. (avr_emit_move): New function. * config/avr/predicates.md (hh8_operand): New predicate. (nop_general_operand): New predicate. (nox_general_operand): New predicate. * config/avr/avr.md (LPM_REGNO): New define_constants. (adjust_len): Add xload. (load_libgcc): New expander. (xload8_A, xload_A, *xload.): New insns. (*load..libgcc, *xload.qi, *xload..libgcc): New insn-and-splits. (mov): Call avr_emit_move to expand. (movmemhi): Ditto. Propagate address space information to newly created MEM. (movqi_insn, *movhi, *movpsi, *movsi, *movsf): Change predicate #1 to nox_general_operand. (ashrqi3, ashrhi3, ashrsi3): Change predicate #1 to nop_general_operand. (ashlqi3, *ashlqi3, ashlhi3, ashlsi3): Ditto. (lshrqi3, *lshrqi3, lshrhi3, lshrsi3): Ditto. (split-lpmx): New split. (*ashlhi3_const, *ashlsi3_const, *ashrhi3_const, *ashrsi3_const, *lshrhi3_const, *lshrsi3_const): Indent, unquote C. (n_extendhipsi2): New insn-and-split. libgcc/ PR target/49868 * config/avr/t-avr (LIB1ASMFUNCS): Add _load_3, _load_4, _xload_2 _xload_3 _xload_4. * config/avr/lib1funcs.S (__load_3, __load_4, __xload_2, __xload_3, __xload_4): New functions. Index: ../libgcc/config/avr/lib1funcs.S =================================================================== --- ../libgcc/config/avr/lib1funcs.S (revision 180962) +++ ../libgcc/config/avr/lib1funcs.S (working copy) @@ -1175,6 +1175,120 @@ DEFUN __tablejump_elpm__ ENDF __tablejump_elpm__ #endif /* defined (L_tablejump_elpm) */ +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; 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_* && ELPM */ + + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Loading n bytes from Flash; n = 3,4 +;; R22... = Flash[Z] +;; Clobbers: __tmp_reg__ + +#if (defined (L_load_3) \ + || defined (L_load_4)) \ + && !defined (__AVR_HAVE_LPMX__) + +;; Destination +#define D0 22 +#define D1 D0+1 +#define D2 D0+2 +#define D3 D0+3 + +.macro .load dest, n + lpm + mov \dest, r0 +.if \dest != D0+\n-1 + adiw r30, 1 +.else + sbiw r30, \n-1 +.endif +.endm + +#if defined (L_load_3) +DEFUN __load_3 + push D3 + XCALL __load_4 + pop D3 + ret +ENDF __load_3 +#endif /* L_load_3 */ + +#if defined (L_load_4) +DEFUN __load_4 + .load D0, 4 + .load D1, 4 + .load D2, 4 + .load D3, 4 + ret +ENDF __load_4 +#endif /* L_load_4 */ + +#endif /* L_load_3 || L_load_3 */ + .section .text.libgcc.builtins, "ax", @progbits Index: ../libgcc/config/avr/t-avr =================================================================== --- ../libgcc/config/avr/t-avr (revision 180962) +++ ../libgcc/config/avr/t-avr (working copy) @@ -21,6 +21,8 @@ LIB1ASMFUNCS = \ _cleanup \ _tablejump \ _tablejump_elpm \ + _load_3 _load_4 \ + _xload_2 _xload_3 _xload_4 \ _copy_data \ _clear_bss \ _ctors \