Denis Chertykov wrote:
> 2011/11/29 Georg-Johann Lay:
>> For devices with 8-bit SP reading the high byte SP_H of SP will get garbage.
>>
>> The patch uses CLR instead of IN SP_H to "read" the high part of SP.
>>
>> There are two issues with this patch:
>>
>> == 1 ==
>>
>> I cannot really test it because for devices that small running test suite
>> does not give usable results. So I just looked at the patch and the
>> small test case like the following compiled
>>
>> $ avr-gcc-4.6.2 -Os -mmcu=attiny26 -m[no]call-prologues
>>
>> long long a, b;
>>
>> long long __attribute__((noinline,noclione))
>> bar (char volatile *c)
>> {
>> *c = 1;
>> return a+b;
>> }
>>
>> long long foo()
>> {
>> char buf[16];
>> return bar (buf);
>> }
>>
>>
>> int main (void)
>> {
>> return foo();
>> }
>>
>>
>> The C parts look fine but...
>>
>>
>> == 2 ==
>>
>> The libgcc parts will still read garbage to R29 as explained in the
>> FIXMEs there.
>>
>> Solving the FIXMEs can only be achieved by splitting multilibs avr2 and
>> avr25,
>> i.e. the mutlilibs that mix devices with/without SP.H, into avr2, avr21,
>> avr24,
>> avr25, say.
>>
>> I don't think it's a good idea to have real 8-bit SP/FP and that it would
>> cause
>> all sorts of trouble.
>
> I'm agree.
>
>> Ok to commit to 4.6?
>
> Approved.
http://gcc.gnu.org/viewcvs?view=revision&revision=181936
Installed as gcc-4_6-branch r181936 with the following change:
Index: config/avr/avr-devices.c
===================================================================
--- config/avr/avr-devices.c (revision 181783)
+++ config/avr/avr-devices.c (working copy)
@@ -70,7 +70,7 @@ const struct mcu_type_s avr_mcu_types[]
{ "attiny2313a", ARCH_AVR25, "__AVR_ATtiny2313A__", 1, 0x0060,
"tn2313a" },
{ "attiny24", ARCH_AVR25, "__AVR_ATtiny24__", 1, 0x0060,
"tn24" },
{ "attiny24a", ARCH_AVR25, "__AVR_ATtiny24A__", 1, 0x0060,
"tn24a" },
- { "attiny4313", ARCH_AVR25, "__AVR_ATtiny4313__", 1, 0x0060,
"tn4313" },
+ { "attiny4313", ARCH_AVR25, "__AVR_ATtiny4313__", 0, 0x0060,
"tn4313" },
{ "attiny44", ARCH_AVR25, "__AVR_ATtiny44__", 0, 0x0060,
"tn44" },
{ "attiny44a", ARCH_AVR25, "__AVR_ATtiny44A__", 0, 0x0060,
"tn44a" },
{ "attiny84", ARCH_AVR25, "__AVR_ATtiny84__", 0, 0x0060,
"tn84" },
@@ -88,7 +88,7 @@ const struct mcu_type_s avr_mcu_types[]
{ "attiny87", ARCH_AVR25, "__AVR_ATtiny87__", 0, 0x0100,
"tn87" },
{ "attiny48", ARCH_AVR25, "__AVR_ATtiny48__", 0, 0x0100,
"tn48" },
{ "attiny88", ARCH_AVR25, "__AVR_ATtiny88__", 0, 0x0100,
"tn88" },
- { "at86rf401", ARCH_AVR25, "__AVR_AT86RF401__", 1, 0x0060,
"86401" },
+ { "at86rf401", ARCH_AVR25, "__AVR_AT86RF401__", 0, 0x0060,
"86401" },
/* Classic, > 8K, <= 64K. */
{ "avr3", ARCH_AVR3, NULL, 0, 0x0060,
"43355" },
{ "at43usb355", ARCH_AVR3, "__AVR_AT43USB355__", 0, 0x0060,
"43355" },
As it turned out, ATtiny4313 and AT86RF401 have a 16-bit stack pointer and
their manual is bogus in stating their SP has 8 bits only.
This is not a complete fix to the SPH issue because PR51345 is still open:
libgcc might happily read gabage into R29 from IO[0x3e].
Johann
>> What about splitting multilibs?
>
> Seems that splitting multilibs is a right way.
Opened PR51345 for it.
>> Is this appropriate for 4.7?
>
> As I understand, any changes appropriate for our port in any stage.
>
>> Johann
>>
>> PR target/51002
>> * config/avr/libgcc.S (__prologue_saves__, __epilogue_restores__):
>> Enclose parts using __SP_H__ in defined (__AVR_HAVE_8BIT_SP__).
>> Add FIXME comments.
>> * config/avr/avr.md (movhi_sp_r_irq_off, movhi_sp_r_irq_on): Set
>> insn condition to !AVR_HAVE_8BIT_SP.
>> * config/avr/avr.c (output_movhi): "clr%B0" instead of "in
>> %B0,__SP_H__" if AVR_HAVE_8BIT_SP.
>> (avr_file_start): Only print "__SP_H__ = 0x3e" if !AVR_HAVE_8BIT_SP.
>>
>
> Denis.
