Re: [avr-gcc-list] gcc builds wrong code?

[Theodore A. Roth]

On Sun, 25 Jul 2004, Theodore A. Roth wrote:

> On Sun, 25 Jul 2004, Klaus Rudolph wrote:
>
> > Hi all,
> >
> > I wrote the following testcode and found that if a table of pointers to
> > functions
> > recides in flash "attribute progmem" the code is not correct.
> >
> > Is the compiler doing the wrong or am I the problem :-)
> >
> > Compiler Version:
> > avr-gcc (GCC) 3.4.0
> >
> > Build my code with:
> > avr-gcc -O2 -g -mmcu=at90s8515 test.c -c
> > avr-gcc -O2 -g -mmcu=at90s8515 test.o -o go
> >
> > The code generation is wrong if version 1 is enabled, with version 2 the
> > code is
> > correct.
> >
> > Thanks
> >     Klaus
> >
> > ///////////////////////////////// test.c /////////////////////////////
> >
> >
> > #ifdef __AVR__
> > #include <avr/io.h>
> > #endif
> >
> > int f2(int a) {
> >     return a+1;
> > }
> >
> > int f3(int a) {
> >     return a*2;
> > }
> >
> > int f4(int a) {
> >     return a*2+1;
> > }
> >
> >
> >
> > volatile int all=0;
> >
> > typedef int(*fp_t)(int) ;
> >
> > // Version 1 fail
> > const fp_t __attribute__ ((progmem)) fp[]={f2,f3,f4,0} ;
> > //Version 2 ok
> > //const fp_t fp[]={f2,f3,f4,0} ;
> >
> >
> > int main() {
> >     int index=1;
> >     int tmp=1;
> >
> >     while (fp[index]) {
> >         tmp+=(fp[index++])(tmp);
> >     }
> >
> >     all=tmp;
> >
> >     return 0;
> > }
>
> I seem to recall there being a rather long discussion about exactly this
> issue a week or two ago. No one every posted a tested, working example.
>
> Attached is a tested working example. I've tried to reduce it down to
> the bare minimum example. The variable "func" in the inner loop is not
> needed, I only did that to force the compiler to not use registers so
> as to make it easier to debug with gdb using 'display func'.
>
> Tested on a mega128 with a stk500/501. Compliled with these commands:
>
>   $ avr-gcc -g -Os -Wall -mmcu=atmega128 -c -o f_ptr_pgm.o f_ptr_pgm.c
>   $ avr-gcc -g -Os -Wall -mmcu=atmega128 -Wl,-Map,f_ptr_pgm.map  -o 
> f_ptr_pgm.elf f_ptr_pgm.o
>
> The part that may or may not be a bug in gcc is the 'fp += 2;' statement
> to increment the fp variable. You would think that 'fp++;' would do the
> right thing, but gcc seems to want to convert the byte address to a word
> address and only increment by one instead of the expected 2.

The bug was in my example. :-\ I had declared fp to be this:

  func_t fp = (func_t) func_list;

when it should be this:

  func_t *fp = (func_t *) func_list;

func_list is declared as this:

  typedef void (*func_t) (void);
  func_t func_list [] PROGMEM = { f0, f1, f2, f3, f4, f5, f6, f7, NULL };

Once I made that change, the 'fp++;' statement worked like I expected.

The generated asm for the loop looks like this:

    while (1)
    {
        func_t func = (func_t) pgm_read_word (fp);
 18a:   fe 01           movw    r30, r28
 18c:   85 91           lpm     r24, Z+
 18e:   94 91           lpm     r25, Z

        if (func == NULL)
 190:   00 97           sbiw    r24, 0x00       ; 0
 192:   31 f0           breq    .+12            ; 0x1a0
            break;

        func ();
 194:   fc 01           movw    r30, r24
 196:   09 95           icall

        delay ();
 198:   0e 94 6e 00     call    0xdc

        fp++;
 19c:   22 96           adiw    r28, 0x02       ; 2
 19e:   f5 cf           rjmp    .-22            ; 0x18a
    }

I've attached the fixed example for anyone interested in seeing the
whole working example.

Klaus, were you able to get your test case working?

---
Ted Roth
PGP Key ID: 0x18F846E9
Jabber ID: address@hidden

f_ptr_pgm.c
Description: Text document