Bug 16563 - The compiler doesnt the necessary push/restore of r18/r19
Summary: The compiler doesnt the necessary push/restore of r18/r19
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 3.4.0
: P2 normal
Target Milestone: 4.0.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-07-15 10:30 UTC by Martin Kruger
Modified: 2014-02-16 13:15 UTC (History)
5 users (show)

See Also:
Host: i686-pc-linux
Target: avr
Build:
Known to work:
Known to fail:
Last reconfirmed: 2004-08-31 18:35:16


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Kruger 2004-07-15 10:30:03 UTC
void  testfunc32(unsigned long);
void  testfunc16(int);
void testfunc8(char);

void testfunc(unsigned int a,char b,char c,unsigned long d)
{
        testfunc16(a);
        testfunc8(10);
        testfunc8(b);
        testfunc16(11);
        testfunc8(c);
        testfunc8(6);
        testfunc32(d);
}


void test()
{
testfunc(0xabcd,10,11,0xabacadae);
}


Compile the code with :

avr-gcc -c -Os file.c

Now we may look at the produced assembler directives.
This is the call to testfunc32():

mov     r25, r19
mov     r24, r18
mov     r23, r17
mov     r22, r16
rcall   testfunc32

All the other functions called before are free to change the content of 
r18/r19 wich contain the MSW of argument d.
The possibly modified value of d ist passed to testfunc32().

The gcc is configured with:
 --prefix=/usr/local/ --target=avr --enable-languages=c,c++ --disable-nls
Comment 1 Eric Weddington 2004-07-15 20:23:39 UTC
There has been traffic on the avr-gcc-list mailing list that suggests that this
is a regression against the 3.4 series:
<http://www.avr1.org/pipermail/avr-gcc-list/2004-July/007134.html>
<http://www.avr1.org/pipermail/avr-gcc-list/2004-July/007141.html>

Please note that Joerg Wunsch who did the testing in the first message above is
the FreeBSD maintainer of the AVR toolset.

The ABI for the AVR toolset is described here:
<http://www.nongnu.org/avr-libc/user-manual/FAQ.html#faq_reg_usage>
Comment 2 Jörg Wunsch 2004-07-15 20:41:20 UTC
I can confirm the bug, as well as the regression against GCC 3.3.x.

In GCC 3.3.x, parameter `d' (in Martin's test case) has been internally
copied over to r14 through r17, and that value was used then.  In GCC 3.4.x,
`d' is no longer saved, so wrong values will be passed on once a called
function has modified r18 and/or r19 (which it is free to do according to the
ABI specification).
Comment 3 denisc 2004-08-31 18:35:16 UTC
I have analyzed this bug and founded that it's not a target bug.
It's a bug in regrename.c
regrename.c have a wrong tracking of call-clobbered registers.
avr have defined r2, ... r16,r17 as call-saved and r18,r19,... as call-clobbered.
Register allocator (greg) produce right code and copy 'd' to r10,r11,r12,r13.
Register renaming (rnreg) produce a wrong code. Rnreg remove copy insn because
it have a wrong tracking of register r16 in SImode through call_insn's.
regrename.c use the `kill_value_regno' to remove information about all
call-clobbered registers from `value_data' structure, but `kill_value_regno'
can't remove all information
about r16 in SImode (r16,r17,r18,r19) because call-clobbered registers only r18,
r19, ...

In example:
We have a link from r10 to r16 in SImode (for avr port SImoode use r16,r17,r18,r19)
call to `kill_value_regno (18, vd)' do nothing because r18 havn't any link. (r16
have. Not r18 nor r17,r19). I think that it's wrong.

I don't know a right place for fixing code. 
`fix_value' probably have a code which can be used as a part for fix.
Comment 4 Emil 2004-09-08 13:57:28 UTC
I have found another bug which I think is related to this one:

typedef unsigned char u08;
typedef unsigned short u16;

#define PROGMEM __attribute__((__progmem__))

#define pgm_read_word(addr)                  \
({                                           \
     u16 __addr16 = (u16)(addr);             \
     u16 __result;                           \
     __asm__                                 \
     (                                       \
         "lpm %A0, Z+"   "\n\t"              \
         "lpm %B0, Z"    "\n\t"              \
         : "=r" (__result), "=z" (__addr16)  \
         : "1" (__addr16)                    \
     );                                      \
     __result;                               \
})

PROGMEM u16 crctbl[1] = { 0x0000 };

u08           crc[2];

void
crc16( u08 * bufptr, u16 len )
{
  *(u16*)crc = 0;
  for ( ; len--; bufptr++ )
//  *(u16*)crc = pgm_read_word( &crctbl[crc[1] ^ *bufptr] ) ^ ( crc[0] << 8 );
    *(u16*)crc = ( crc[0] << 8 ) ^ pgm_read_word( &crctbl[crc[1] ^ *bufptr] );
}

int
main( void )
{
  return 0;
}

Compiling the above .c generates wrong code for the following line
                                                                     
    *(u16*)crc = ( crc[0] << 8 ) ^ pgm_read_word( &crctbl[crc[1] ^ *bufptr] );

  9c:   80 91 60 00     lds     r24, 0x0060
  a0:   99 27           eor     r25, r25
  a2:   38 2f           mov     r19, r24
  a4:   22 27           eor     r18, r18
  a6:   20 91 61 00     lds     r18, 0x0061   <- r18 is lost here
  aa:   9d 91           ld      r25, X+
  ac:   29 27           eor     r18, r25
  ae:   e2 2f           mov     r30, r18
  b0:   ff 27           eor     r31, r31
  b2:   ee 0f           add     r30, r30
  b4:   ff 1f           adc     r31, r31
  b6:   ec 5a           subi    r30, 0xAC
  b8:   ff 4f           sbci    r31, 0xFF
  ba:   85 91           lpm     r24, Z+
  bc:   94 91           lpm     r25, Z
  be:   28 27           eor     r18, r24     <- uses the wrong value
  c0:   39 27           eor     r19, r25
  c2:   30 93 61 00     sts     0x0061, r19
  c6:   20 93 60 00     sts     0x0060, r18

If you change the line and reverse the XOR then the generated code is fine:
   *(u16*)crc = pgm_read_word( &crctbl[crc[1] ^ *bufptr] ) ^ ( crc[0] << 8 );

I'm using avr-gcc (GCC) 3.4.1 and the following flags to compile:

CPFLAGS = -g -O3 -funsigned-char -funsigned-bitfields -fpack-struct 
-fshort-enums -Wall -W
+strict-prototypes -Wa,-ahlms=$(<:.c=.lst) -mmcu=atmega16



Comment 5 berndtrog 2004-10-06 16:48:28 UTC
It looks like with gcc version 4.0.0 20040917 it should 
work (again):

00000000 <testfunc>:
   0:   af 92           push    r10
   2:   bf 92           push    r11
   4:   cf 92           push    r12
   6:   df 92           push    r13
   8:   ef 92           push    r14
   a:   ff 92           push    r15
   c:   0f 93           push    r16
   e:   1f 93           push    r17
  10:   f6 2e           mov     r15, r22
  12:   e4 2e           mov     r14, r20
  14:   a0 2e           mov     r10, r16
  16:   b1 2e           mov     r11, r17
  18:   c2 2e           mov     r12, r18
  1a:   d3 2e           mov     r13, r19
  1c:   00 d0           rcall   .+0             ; 0x1e
  1e:   8a e0           ldi     r24, 0x0A       ; 10
  20:   00 d0           rcall   .+0             ; 0x22
  22:   8f 2d           mov     r24, r15
  24:   00 d0           rcall   .+0             ; 0x26
  26:   8b e0           ldi     r24, 0x0B       ; 11
  28:   90 e0           ldi     r25, 0x00       ; 0
  2a:   00 d0           rcall   .+0             ; 0x2c
  2c:   8e 2d           mov     r24, r14
  2e:   00 d0           rcall   .+0             ; 0x30
  30:   86 e0           ldi     r24, 0x06       ; 6
  32:   00 d0           rcall   .+0             ; 0x34
  34:   9d 2d           mov     r25, r13
  36:   8c 2d           mov     r24, r12
  38:   7b 2d           mov     r23, r11
  3a:   6a 2d           mov     r22, r10
  3c:   00 d0           rcall   .+0             ; 0x3e
  3e:   1f 91           pop     r17
  40:   0f 91           pop     r16
  42:   ff 90           pop     r15
  44:   ef 90           pop     r14
  46:   df 90           pop     r13
  48:   cf 90           pop     r12
  4a:   bf 90           pop     r11
  4c:   af 90           pop     r10
  4e:   08 95           ret
...
Comment 6 Eric Weddington 2007-02-14 16:48:10 UTC
Same results with GCC 4.1.1:

00000000 <testfunc>:
   0:	af 92       	push	r10
   2:	bf 92       	push	r11
   4:	cf 92       	push	r12
   6:	df 92       	push	r13
   8:	ef 92       	push	r14
   a:	ff 92       	push	r15
   c:	0f 93       	push	r16
   e:	1f 93       	push	r17
  10:	f6 2e       	mov	r15, r22
  12:	e4 2e       	mov	r14, r20
  14:	a0 2e       	mov	r10, r16
  16:	b1 2e       	mov	r11, r17
  18:	c2 2e       	mov	r12, r18
  1a:	d3 2e       	mov	r13, r19
  1c:	00 d0       	rcall	.+0      	; 0x1e <testfunc+0x1e>
  1e:	8a e0       	ldi	r24, 0x0A	; 10
  20:	00 d0       	rcall	.+0      	; 0x22 <testfunc+0x22>
  22:	8f 2d       	mov	r24, r15
  24:	00 d0       	rcall	.+0      	; 0x26 <testfunc+0x26>
  26:	8b e0       	ldi	r24, 0x0B	; 11
  28:	90 e0       	ldi	r25, 0x00	; 0
  2a:	00 d0       	rcall	.+0      	; 0x2c <testfunc+0x2c>
  2c:	8e 2d       	mov	r24, r14
  2e:	00 d0       	rcall	.+0      	; 0x30 <testfunc+0x30>
  30:	86 e0       	ldi	r24, 0x06	; 6
  32:	00 d0       	rcall	.+0      	; 0x34 <testfunc+0x34>
  34:	9d 2d       	mov	r25, r13
  36:	8c 2d       	mov	r24, r12
  38:	7b 2d       	mov	r23, r11
  3a:	6a 2d       	mov	r22, r10
  3c:	00 d0       	rcall	.+0      	; 0x3e <testfunc+0x3e>
  3e:	1f 91       	pop	r17
  40:	0f 91       	pop	r16
  42:	ff 90       	pop	r15
  44:	ef 90       	pop	r14
  46:	df 90       	pop	r13
  48:	cf 90       	pop	r12
  4a:	bf 90       	pop	r11
  4c:	af 90       	pop	r10
  4e:	08 95       	ret
Comment 7 Eric Weddington 2008-06-04 13:06:20 UTC
Bug closed based on comment #5.
Comment 8 Jackie Rosen 2014-02-16 13:15:44 UTC Comment hidden (spam)