RE: [avr-libc-dev] New pgm_read_ptr() macro?

[Weddington, Eric]

Hi Jan,

Be sure to keep posts on-list.

I would be ok with adding this and adding Dean's new macro too. Removing 
functionality from avr-libc gets into backwards-compatibility issues.

However, I want to make sure that this has been tested out, and that 
documentation is included. In both patches.

Eric Weddington

> -----Original Message-----
> From: Jan Waclawek [mailto:address@hidden 
> Sent: Thursday, June 03, 2010 10:04 AM
> To: Weddington, Eric
> Subject: RE: [avr-libc-dev] New pgm_read_ptr() macro?
> 
> Eric,
> 
> this macro is in fact only a wrapper over the existing 
> pgm_read_xxx macros/functions, alleviating the user from 
> chosing the "right one". All its magic happens BEFORE any 
> code is generated, so there's no reason why would this be 
> more or less effective size-wise (nor speed-wise).
> 
> The real benefit is in reduced source size (less typing), 
> especially if the "argument" is NOT a pointer but the object 
> itself (see end remark of original David's post).
> 
> The real drawback is that the user needs to understand its 
> limitations (=> need for proper documentation).
> 
> Jan Waclawek
> 
> 
> ----- Original Message ---------------
> >
> >I think it's not so much a philosophical change that 
> concerns me. I'm interested in how resulting code size would 
> be affected, as suggested by someone else on this list 
> (sorry, I've forgotten who it was).
> >
> >Have you done some testing about resulting code size using 
> this macro compared to the more specific macros?
> >
> >Code size is probably the *most* important factor that I 
> hear users focus on.
> >
> >Eric Weddington 
> >
> >> -----Original Message-----
> >> From: 
> >> address@hidden 
> >> [mailto:address@hidden
> >> org] On Behalf Of David Brown
> >> Sent: Thursday, June 03, 2010 9:03 AM
> >> To: address@hidden
> >> Subject: Re: [avr-libc-dev] New pgm_read_ptr() macro?
> >> 
> >> Hi,
> >> 
> >> Are there any opinions about whether a macro such as the 
> one I gave 
> >> below should be part of avrlibc?  It's not as clear-cut as Dean's 
> >> pgm_read_ptr() macro, which follows the existing patterns in the 
> >> headers.  Macros like mine are more like template programming 
> >> than the 
> >> fixed-size macros, and thus are a slight change in philosophy.
> >> 
> >> If the idea of a generic pgm_read(&x) macro does appeal, then 
> >> the same 
> >> tricks can be used for other accesses such as wrapping an 
> access in 
> >> "volatile", reading and writing eeprom, and atomically 
> accessing data.
> >> 
> >> mvh.,
> >> 
> >> David
> >> 
> >> 
> >> >
> >> > How about a more general solution based on typeof and sizeof?
> >> >
> >> > #define pgm_read(v) ((typeof(*v)) \
> >> > (__builtin_choose_expr(sizeof(*v) == 1, pgm_read_byte(v), \
> >> > __builtin_choose_expr(sizeof(*v) == 2, pgm_read_word(v), \
> >> > __builtin_choose_expr(sizeof(*v) == 4, pgm_read_dword(v), \
> >> > ({ char error[((sizeof(*v) == 1) || (sizeof(*v) == 2) || \
> >> > (sizeof(*v) == 4)) ? 1 : -1]; error[0]; }) \
> >> > )))))
> >> >
> >> >
> >> > The macro above looks a bit ugly, but it's not too hard 
> to follow.
> >> > Basically, we are using sizeof(*v) to figure out the size 
> >> of the data
> >> > you are wanting, and using it to pick the correct 
> >> pgm_read_xxx function
> >> > for the data size. We use __builtin_choose_expr instead of 
> >> ?: to avoid
> >> > any unwanted expression evaluations or side effects, as 
> >> well as to allow
> >> > different sized return values. The "char error[...]" part 
> >> is a way to
> >> > force a compile-time error if the macro is used with 
> something whose
> >> > size is not 1, 2 or 4 bytes.
> >> >
> >> > The result is that you can write:
> >> >
> >> > char x;
> >> > char* PROGMEM FlashPointer = &x;
> >> > char* FlashPtr = pgm_read(&FlashPointer);
> >> >
> >> > You can equally write:
> >> >
> >> > static const PROGMEM uint8_t x8 = 8;
> >> > static const PROGMEM uint16_t x16 = 16;
> >> > static const PROGMEM uint32_t x32 = 32;
> >> >
> >> > void test(void) {
> >> > volatile uint8_t y8 = pgm_read(&x8);
> >> > volatile uint16_t y16 = pgm_read(&x16);
> >> > volatile uint32_t y32 = pgm_read(&x32);
> >> > }
> >> >
> >> >
> >> > Now your pgm_read's are independent of the type, assuming 
> >> they have a
> >> > compatible size.
> >> >
> >> >
> >> > Personally, my preference would be to change the semantics 
> >> to remove the
> >> > "&", so that you would write:
> >> >
> >> > char* FlashPtr = pgm_read(FlashPointer);
> >> >
> >> > This is just a small change to the macro, but I think it's 
> >> neater - you
> >> > are reading an object from flash, rather than reading 
> data from an
> >> > address in flash. But such a change would require a name 
> >> change to avoid
> >> > accidents.
> >> >
> >> > mvh.,
> >> >
> >> > David
> >> >
> >> >
> >> >
> >> >
> >
> 
>