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

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, Mar 5, 2012 at 3:11 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
> 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?

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.

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