[avr-libc-dev] Suggested improvement to <avr/pgmspace.h>

[Marko Mäkelä]

Hi all,

I am trying to revive my AVR hobby again, this time using the avr-libc,
mainly to introduce my kids to low-level programming. I am using a
couple of Arduino boards with the ATmega328P.

I started with a simple "hello world" program that outputs a
NUL-terminated string to the UART:

static void msg(const char* PROGMEM msg)
{
char c;
while ((c = pgm_read_byte_postinc (&msg))) {
UDR0 = c;
loop_until_bit_is_set(UCSR0A, UDRE0);
}
}

For optimal AVR implementation, this would require some inline assembly:

static char pgm_read_byte_postinc (const char** PROGMEM s)
{
#ifdef __AVR_HAVE_LPMX__
char c;
asm volatile ("lpm %0, %a1+" "\n\t" : "=r" (c), "+z" (*s));
return c;
#else
return pgm_read_byte((*s)++);
#endif
}

I would like to have pre/post-increment/decrement variants of all
pgm_read_* functions in <avr/pgmspace.h> that are natively supported on
some AVR platform. I wonder what would be the practical way to achieve
this. Contribute a patch myself? I hereby contribute the above idea to
the public domain.

Best regards,

Marko

[Georg-Johann Lay]

Post by Marko Mäkelä
Hi all,
I am trying to revive my AVR hobby again, this time using the avr-libc,
mainly to introduce my kids to low-level programming. I am using a
couple of Arduino boards with the ATmega328P.
I started with a simple "hello world" program that outputs a
static void msg(const char* PROGMEM msg)
{
char c;
while ((c = pgm_read_byte_postinc (&msg))) {
UDR0 = c;
loop_until_bit_is_set(UCSR0A, UDRE0);
}
}

You can use the __flash address space instead of PROGMEM + pgm_read, cf.

https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html

In your case, the function would read:

static void msg (const __flash char *msg)
{
char c;
while ((c = *msg++))
{
UDR0 = c;
loop_until_bit_is_set (UCSR0A, UDRE0);
}
}

The __flash address space is implemented by the compiler as a GNU
extension to the C language, and hence -std=gnu99 (or higher) is
needed. There is no guarantee for an optimal code being generated
for that example (as applies to any other C code).

Also notice that PROGMEM is just a variable attribute; its only
effect is to locate data to .progmem.data for variable definitions
which are attributed progmem. Specifying it with prototypes or
declarations is void.

The advantage is that less typing is needed and the feature is more
trqansparent w.r.t. code that only addresses generic address space,
e.g. to deflate to .rodata (which is located in RAM for avr) you
only need to

#define __flash /* empty */


Johann

Post by Marko Mäkelä
static char pgm_read_byte_postinc (const char** PROGMEM s)
{
#ifdef __AVR_HAVE_LPMX__
char c;
asm volatile ("lpm %0, %a1+" "\n\t" : "=r" (c), "+z" (*s));
return c;
#else
return pgm_read_byte((*s)++);
#endif
}
I would like to have pre/post-increment/decrement variants of all
pgm_read_* functions in <avr/pgmspace.h> that are natively supported on
some AVR platform. I wonder what would be the practical way to achieve
this. Contribute a patch myself? I hereby contribute the above idea to
the public domain.
Best regards,
Marko
_______________________________________________
AVR-libc-dev mailing list
https://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Marko Mäkelä]

Post by Georg-Johann Lay
You can use the __flash address space instead of PROGMEM + pgm_read, cf.
https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html
static void msg (const __flash char *msg)
{
char c;
while ((c = *msg++))
{
UDR0 = c;
loop_until_bit_is_set (UCSR0A, UDRE0);
}
}
The __flash address space is implemented by the compiler as a GNU
extension to the C language, and hence -std=gnu99 (or higher) is
needed.

Thank you, this is much cleaner, and like you pointed out, this allows
portable code to be written when using typedef or #define. I guess
<avr/pgmspace.h> now only exists for backward compatibility with old
code?

I would like to follow up with some avr-gcc or avr-g++ remarks (sorry
for the off-topic discussion).

It looks like avr-g++ does not accept the __flash qualifier. Are there
plans to support it?

Post by Georg-Johann Lay
There is no guarantee for an optimal code being generated for that
example (as applies to any other C code).

It turns out that the compiled code is identical with my asm-accelerated
version. The entry into the loop looks like this:

ae: 85 91 lpm r24, Z+
b0: 81 11 cpse r24, r1
b2: f7 cf rjmp .-18 ; 0xa2 <main+0x12>
b4: f3 cf rjmp .-26 ; 0x9c <main+0xc>

The above is also emitted for -mmcu=attiny2313. With -mmcu=at90s2313,
the "mov" from the temporary register r0 could be omitted:

48: c8 95 lpm
4a: 80 2d mov r24, r0
4c: 31 96 adiw r30, 0x01 ; 1
4e: 81 11 cpse r24, r1
50: f8 cf rjmp .-16 ; 0x42 <__SREG__+0x3>
52: f4 cf rjmp .-24 ; 0x3c <main+0x8>

Best regards,

Marko

[George Spelvin]

Post by Marko Mäkelä
I started with a simple "hello world" program that outputs a
static void msg(const char* PROGMEM msg)
{
char c;
while ((c = pgm_read_byte_postinc (&msg))) {
UDR0 = c;
loop_until_bit_is_set(UCSR0A, UDRE0);
}
}

You could also try using a compiler feature and skipping the pgm_read
function entirely.

static void msg(const char __flash *msg)
{
char c;
while ((c = *msg++)) {
UDR0 = c;
loop_until_bit_is_set(UCSR0A, UDRE0);
}
}

Of course, GCC is generating terrible code for me on this (not only
is it not using Z+, it's copying the pointer to a different register to
increment it and using a subi/sbci sequence to do it), but it's definitely
more legible source code.

I also wonder if we could do something like:

static inline char pgm_read_byte(char const * PROGMEM p)
{
char c;
asm("lpm %0,%1" : "=r" (c) : "m<>" (*p));
return c;
}

which would tell GCC how to make "pgm_read_byte(msg++)" use
a postincrement addressing mode, without having to add another
function.


(We should also update inttypes.h and pgmspace.h to use
__int24/__uint24 for far addresses.)