Bug 52461

Summary: [avr] XMEGA+EBI: RAMPZ clobbered while reading from flash
Product: gcc Reporter: Georg-Johann Lay <gjl>
Component: targetAssignee: Georg-Johann Lay <gjl>
Status: RESOLVED FIXED    
Severity: normal CC: darkdragon2000, eric.weddington
Priority: P4 Keywords: wrong-code
Version: 4.7.0   
Target Milestone: 4.7.1   
Host: Target: avr
Build: Known to work:
Known to fail: Last reconfirmed: 2012-03-07 00:00:00

Description Georg-Johann Lay 2012-03-02 12:19:40 UTC
To read from flash on devices with more than 64KiB of flash, RAMPZ is set appropriately before using ELPM instruction.

Devices with external bus interface (EBI) use RAMPZ as high part of the RAM address while reading from RAM.

Thus, and RAM read on EBI device that is performed via Z will fail after data has been read from Flash.
Comment 1 Georg-Johann Lay 2012-03-02 12:22:56 UTC
Eric, would you confirm this is actually a problem and set bug status to NEW?
Or set to INVALID if I misunderstood the EBI.

Can the EBI be switched off, so that RAMPZ affects ELPM but not LD?
Comment 2 Georg-Johann Lay 2012-03-03 14:11:04 UTC
*** Bug 44940 has been marked as a duplicate of this bug. ***
Comment 3 Markus Faust 2012-03-06 07:55:39 UTC
(In reply to comment #1)
> Eric, would you confirm this is actually a problem and set bug status to NEW?

For me this seems to be a really severe problem, because as soon as my code size reaches 65536 bytes, RAMPZ seems to be set to 1. On the other hand, I/O register access is (sometimes) done via STD with Z register and will go wrong if RAMPZ is not 0. This is mostly the same behaviour as described by darkdragon2000 in bug 44940, except that it also happens in the application section if code size is >=65536 bytes.

device: ATxmega128A1
gcc version 4.5.1 (AVR_8_bit_GNU_Toolchain_3.2.3_314) (from Atmel)
Windows 2000

It would be very helpful if someone could provide a fix, as I am stuck at work because of this problem.

I have put a "RAMPZ=0;" at the very beginning of main(), which seems to do the trick for the moment, but I think this is a very ugly hack and potentially dangerous.
Comment 4 Georg-Johann Lay 2012-03-06 12:43:00 UTC
(In reply to comment #3)
> 
> This is mostly the same behaviour as described by
> darkdragon2000 in bug 44940, except that it also happens in the application
> section if code size is >=65536 bytes.
> 
> device: ATxmega128A1
> gcc version 4.5.1 (AVR_8_bit_GNU_Toolchain_3.2.3_314) (from Atmel)
> Windows 2000

XMEGA support is is added in GCC 4.7.0 and is still tentative an not well tested.

If you use a vendor specific GCC port like mentioned above, please report bugs to the respective vendor support address or bug tracker, i.e. avr@atmel.com in this case.

This bug tracker if for the official GCC hosted by FSF, not for private ports.

This means that this PR refers to the FSF hosted version of the compiler, not to Atmel's fork of the compiler which might or might not expose similar problems.
Comment 5 Markus Faust 2012-03-07 10:12:41 UTC
(In reply to comment #4)
> This bug tracker if for the official GCC hosted by FSF, not for private ports.
Sorry about that.

Yesterday I found Stefan's binaries (Thanks, Stefan):
gcc version 4.7.0 20120217 - by SRMeister (GCC)

It seems to show the same behaviour.
ELPM is used in the __do_copy_data section (the only ELPM in my whole object code, btw.).

(In reply to comment #1)
> Can the EBI be switched off, so that RAMPZ affects ELPM but not LD?

Actually, the EBI is switched off by default (and I'm not using it), but obviously RAMPZ does still affect the adressing of the data space (LD/ST with Z). I think switching EBI on/off only affects the function of the corresponding port pins.

Further more, I didn't find any documentation if RAMPZ affects addressing of data space on devices without EBI, but it is documented that (for LD/ST with increment)
> The RAMPZ Register in the I/O area is updated in parts with more than 64K bytes data space or more than 64K bytes Program memory, [...].
(AVR Instruction Set, doc0856i)
Comment 6 Georg-Johann Lay 2012-03-07 10:33:24 UTC
Author: gjl
Date: Wed Mar  7 10:33:19 2012
New Revision: 185030

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185030
Log:
libgcc/
	PR target/52461
	PR target/52508
	* 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.
	(__movmemx_hi): Ditto.
gcc/	
	PR target/52461
	* gcc/config/avr/avr.c (avr_out_lpm): Clear RAMPZ after usage
	if RAMPZ affects reading from RAM.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/libgcc/ChangeLog
    trunk/libgcc/config/avr/lib1funcs.S
Comment 7 Stefan Reichardt 2012-03-07 11:07:33 UTC
nice!
i willl build it and publish it on avrfreaks soon, so Markus can try it out
Comment 8 Georg-Johann Lay 2012-03-07 12:35:17 UTC
(In reply to comment #7)
> nice!
> i willl build it and publish it on avrfreaks soon, so Markus can try it out

Stefan, please noteice other PRs that won't get into 4.7.0 and fix problems with new features: PR52496, PR52505,  PR52506,  PR52507,  PR52508.

Backporting to 4.7 will happen if 4.7-branch is open for bugfixing which will be in about 3 weeks after 4.7.0 release.
Comment 9 Georg-Johann Lay 2012-03-21 13:20:25 UTC
Author: gjl
Date: Wed Mar 21 13:20:20 2012
New Revision: 185605

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185605
Log:
	PR rtl-optimization/52543
	PR target/52461
	* config/avr/avr-protos.h (avr_load_lpm): New prototype.
	* config/avr/avr.c (avr_mode_dependent_address_p): New function.
	(TARGET_MODE_DEPENDENT_ADDRESS_P): New define.
	(avr_load_libgcc_p): Restrict to __flash loads.
	(avr_out_lpm): Only handle 1-byte loads from __flash.
	(avr_load_lpm): New function.
	(avr_find_unused_d_reg): Remove.
	(avr_out_lpm_no_lpmx): Remove.
	(adjust_insn_length): Handle ADJUST_LEN_LOAD_LPM.
	* config/avr/avr.md (unspec): Add UNSPEC_LPM.
	(load_<mode>_libgcc): Use UNSPEC_LPM instead of MEM.
	(load_<mode>, load_<mode>_clobber): New insns.
	(mov<mode>): For multi-byte move from non-generic
	16-bit address spaces: Expand to load_<mode> resp.
	load_<mode>_clobber.
	(load<mode>_libgcc): Remove expander.
	(split-lpmx): Remove split.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr-protos.h
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.md
Comment 10 Georg-Johann Lay 2012-03-22 15:07:21 UTC
Author: gjl
Date: Thu Mar 22 15:06:57 2012
New Revision: 185697

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185697
Log:
libgcc/
	Backport from 2012-03-07 mainline r185033.

	PR target/52507
	* config/avr/lib1funcs.S (__movmemx_hi): Fix loop label in RAM-part.

	Backport from 2012-03-07 mainline r185031.

	PR target/52505
	* config/avr/lib1funcs.S (__xload_1): Don't read unintentionally
	from RAM.

	Backport from 2012-03-07 mainline r185030.

	PR target/52461
	PR target/52508
	* 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.
	(__movmemx_hi): Ditto.

gcc/
	Backport from 2012-03-22 mainline r185692.

	PR target/52496
	* config/avr/avr.md (unspec): Remove UNSPEC_MEMORY_BARRIER.
	(unspecv): Add UNSPECV_MEMORY_BARRIER.
	(cli_sei): Use unspec_volatile instead of unspec for memory barrier.
	(delay_cycles_1, delay_cycles_2): Ditto.
	(delay_cycles_3, delay_cycles_4): Ditto.
	(nopv, *nopv): Ditto.
	(sleep, *sleep): Ditto.
	(wdr, *wdr): Ditto.

	Backport from 2012-03-21 mainline r185605.

	PR rtl-optimization/52543
	PR target/52461
	* config/avr/avr-protos.h (avr_load_lpm): New prototype.
	* config/avr/avr.c (avr_mode_dependent_address_p): New function.
	(TARGET_MODE_DEPENDENT_ADDRESS_P): New define.
	(avr_load_libgcc_p): Restrict to __flash loads.
	(avr_out_lpm): Only handle 1-byte loads from __flash.
	(avr_load_lpm): New function.
	(avr_find_unused_d_reg): Remove.
	(avr_out_lpm_no_lpmx): Remove.
	(adjust_insn_length): Handle ADJUST_LEN_LOAD_LPM.
	* config/avr/avr.md (unspec): Add UNSPEC_LPM.
	(load_<mode>_libgcc): Use UNSPEC_LPM instead of MEM.
	(load_<mode>, load_<mode>_clobber): New insns.
	(mov<mode>): For multi-byte move from non-generic
	16-bit address spaces: Expand to load_<mode> resp.
	load_<mode>_clobber.
	(load<mode>_libgcc): Remove expander.
	(split-lpmx): Remove split.

	Backport from 2012-03-13 mainline r185329.

	PR target/52488
	* config/avr/avr.c (avr_prologue_setup_frame): Cut down stack
	offset (size) to a value the insns can deal with.
	(expand_epilogue): Ditto.

	Backport from 2012-03-12 mainline r185256.

	PR target/52499
	* config/avr/avr.c (avr_mode_code_base_reg_class): Change return
	type from reg_class_t to enum reg_class.
	* config/avr/avr-protos.h (avr_mode_code_base_reg_class): Ditto.

	Backport from 2012-03-12 mainline r185253.

	PR target/52148
	* config/avr/avr.c (avr_out_movmem): Fix typo in output template
	for the case ADDR_SPACE_FLASH and AVR_HAVE_LPMX introduced in
	r184615 from 2012-02-28.

	Backport from 2012-03-08 mainline r185105.

	* config/avr/avr.md (*addhi3, addhi3_clobber): Add "w" alternative
	for constants in [-63,63].

	Backport from 2012-03-08 mainline r185100.

	PR target/52496
	* config/avr/avr.c (avr_mem_clobber): New static function.
	(avr_expand_delay_cycles): Add memory clobber operand to
	delay_cycles_1, delay_cycles_2, delay_cycles_3, delay_cycles_4.
	* config/avr/avr.md (unspec): Add UNSPEC_MEMORY_BARRIER.
	(enable_interrupt, disable_interrupt): New expander.
	(nopv, sleep, wdr): New expanders.
	(delay_cycles_1): Add memory clobber.
	(delay_cycles_2): Add memory clobber.
	(delay_cycles_3): Add memory clobber.
	(delay_cycles_4): Add memory clobber.
	(cli_sei): New insn from former "enable_interrupt",
	"disable_interrupt" with memory clobber.
	(*wdt): New insn from former "wdt" with memory clobber.
	(*nopv): Similar, but for "nopv".
	(*sleep): Similar, but for "sleep".

	Backport from 2012-03-07 mainline r185043.

	PR target/52484
	* config/avr/avr.md (xload<mode>_A): Add R22... to register footprint.

	Backport from 2012-03-07 mainline r185032.

	PR target/52506
	* gcc/config/avr/avr.c (expand_epilogue): Fix order of restoration
	to: RAMPZ, RAMPY, RAMPX, RAMPD.
	(expand_prologue): Only clear RAMPZ if it has effect on RAM-read.

	Backport from 2012-03-07 mainline r185031.

	PR target/52505
	* config/avr/avr.c (avr_out_xload): Don't read unintentionally
	from RAM.
	* config/avr/avr.md (xload_8): Adjust insn length.

	Backport from 2012-03-07 mainline r185030.

	PR target/52461
	* gcc/config/avr/avr.c (avr_out_lpm): Clear RAMPZ after usage
	if RAMPZ affects reading from RAM.

	Backport from 2012-03-05 mainline r184919.

	* config/avr/avr.md (*umaddqihi4.2): New insn-and-split.


Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/avr/avr-protos.h
    branches/gcc-4_7-branch/gcc/config/avr/avr.c
    branches/gcc-4_7-branch/gcc/config/avr/avr.md
    branches/gcc-4_7-branch/libgcc/ChangeLog
    branches/gcc-4_7-branch/libgcc/config/avr/lib1funcs.S
Comment 11 Georg-Johann Lay 2012-03-22 15:50:55 UTC
Fixed in 4.7.1