RE: [avr-gcc-list] Avr-gcc Produces Incorrect Code with -Os

[Weddington, Eric]

> -----Original Message-----
> From: Paulo Marques [mailto:address@hidden 
> Sent: Friday, May 16, 2008 12:07 PM
> To: Weddington, Eric
> Cc: Mark Litwack; address@hidden; Joerg Wunsch
> Subject: Re: [avr-gcc-list] Avr-gcc Produces Incorrect Code with -Os
> 
> Weddington, Eric wrote:
> > [...]
> >> If ATOMIC_BLOCK() is *the* way to do it, perhaps the docs
> >> could be improved a little by emphasizing the use of
> >> ATOMIC_BLOCK() as a necessity rather than as a convenience.
> > 
> > I'll go a step further: Is there any reason to NOT include 
> the memory
> > barrier in the interrupt macros? Would that even work? 
> > 
> > Like this,
> > 
> > # define sei()  __asm__ __volatile__ ("sei" ::: "memory")
> > # define cli()  __asm__ __volatile__ ("cli" ::: "memory")
> 
> This has been discussed before (on the avr-libc list, December 2005, 
> just search for '"cli" and "sei" should clobber memory'), and 
> this exact 
> solution was actually proposed by me.

Oh! Thanks for the reference! :-) That was a while ago...

> The downside is that a memory clobber like that drops all 
> optimizations 
> of global variables that the compiler might be temporarily storing in 
> registers. So, it actually hurts performance for code 
> unrelated to the 
> critical section.
> 
> Another thing we could do to help the common cases of reading 
> / writing 
> multi-byte vars (or even single byte non-volatile vars) was to write 
> accessors like (totaly untested):
> 
> static inline uint16_t atomic_read16(uint16_t *var)
> {
>    uint16_t ret;
>    uint8_t old_sreg = SREG;
>    cli();
>    ret = *((volatile uint16_t *)var);
>    SREG = old_sreg;
> }
> 
> static inline void atomic_write16(uint16_t *var, uint16_t value)
> {
>    uint8_t old_sreg = SREG;
>    cli();
>    *((volatile uint16_t *)var) = value;
>    SREG = old_sreg;
> }
> 
> ....(repeat for other sizes)....
> 
> I'd have to check this code better, because I always end up 
> placing the 
> volatile in the wrong place and casting the pointer to 
> volatile instead 
> of the variable ;)
> 
> Anyway, the advantage of something like this over 
> ATOMIC_BLOCK() is that 
> it doesn't need any memory clobbers, since all operations are already 
> volatile. So, this should produce slightly better code.

Interesting...