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:konf...@efton.sk] > 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: > >> avr-libc-dev-bounces+eric.weddington=atmel....@nongnu.org > >> [mailto:avr-libc-dev-bounces+eric.weddington=atmel....@nongnu. > >> org] On Behalf Of David Brown > >> Sent: Thursday, June 03, 2010 9:03 AM > >> To: avr-libc-dev@nongnu.org > >> 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 > >> > > >> > > >> > > >> > > > > > _______________________________________________ AVR-libc-dev mailing list AVR-libc-dev@nongnu.org http://lists.nongnu.org/mailman/listinfo/avr-libc-dev
