diff mbox

: PR49868: Named address space support for AVR

Message ID 4EB6D44C.2020609@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Nov. 6, 2011, 6:39 p.m. UTC 
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<n> 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<mode>_libgcc): New expander.
	(xload8_A, xload<mode>_A, *xload.<mode>): New insns.
	(*load.<mode>.libgcc, *xload.qi, *xload.<mode>.libgcc): New
	insn-and-splits.
	(mov<mode>): 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.
diff mbox

Patch

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 \