Re: [Patch,AVR] PR52461: Fix RAMPZ clobbering and RAMP* in epilogue

[Georg-Johann Lay <avr at gjlay dot de>]

Georg-Johann Lay wrote:
> This patch fixes several issues with RAMP registers:
> 
> * On Devices with more than 64 KiB RAM, RAMPZ is used as high-byte of
>   RAM address. If RAMPZ is used to read flash, it must be reset to 0
>   after the read so that RAM-read will operate correctly in the remainder.
>   There is no support for RAM > 64 Ki so RAMPZ = 0 is in order.
> 
> * The ISR epilogue restored RAMP* registers in the wrong order.
> 
> * As RAMPZ is used both in ELPM and LD/LDD on some xmega core, the right
>   condition to set RAMPZ prior to ELPM is "have ELPM", not "have RAMPZ".
> 
> * Never read unintentionally from RAM because a flash address interpreted
>   as a RAM address might point to the I/O area.
> 
> Ok for trunk and 4.7?

The following change in expand_prologue is wrong:

-      if (AVR_HAVE_RAMPZ
+      if (AVR_HAVE_RAMPD /* sic! */

similar in expand_epilogue.

Ok without that change?

> 
> 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.