Patchwork [Patch.AVR,4.6] Fix PR51002

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:


+  { "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.