Hi Alex >@2009.04.22_09:51:59_+0200
> Hi,
>
> >>> That's way too much code... Its fairly obvious where the optimisations
> >>> should be, although I can see that some have been done already.
> >>>
> >> I can't see much possibility for improving the above code (except by
> >> removing the push and zeroing of r1). You asked for "status" to be a
> >> volatile 32-bit int, so that's what you got. The code below is
> >> semantically different, and thus compiles differently.
> >>
> > I don't believe it is semantically different. Which is one reason I
> > raised this. I am using the version below in existing code and it
> > behaves correctly.
> >
> > Loading 4 registers and then storing back 3 that are unchanged makes no
> > sense at all.
> >
> > Where volatile comes in here is that the optimisation shouldn't use any
> > previous loads or modify register values more than once without
> > reloading/storing etc. Here, the value is loaded once, one byte is
> > changed and then all 4 are stored. That's wasteful.
>
> but that is what you demand with volatile. No read and no write
> can be removed. On some Hardware there will be a special reaction
> if you write a byte, so it would be semantical different when the
> compiler removes it.
>
> Maybe you can change it to use something like (not tested)
>
> union status_union
> {
> volatile uint32_t s;
> volatile uint8_t b[4];
> } status;
>
>
> then you can operate on single volatile bytes with:
>
That's effectively the same as I changed it to by using the pointer.
> //Set Bit 17 in Status
> status.b[1] |= _BV(2);
>
> without writing the other bytes. An you can use status.s for
> 32-Bit Access.
>
> Maybe it would be easyier to use 4 seperate status bytes, most
> of the time it is better to think more 8-bitish if you write
> Atmelcode.
>
I replied to some of these points in David's post.
And yes, I agree its probably more about staying close to 8-bit
operations than anything else.
Reads or writes, I believe, can be removed when updating a volatile that
is greater than 8-bits.
There are other issues that may make this code unsafe when used outside
of an interrupt routine. This applies to any volatile data that is
greater than 8-bits used on an 8-bit processor.
I guess that its not enough to declare the 32-bit variable volatile. It
needs to be wrapped in an atomic cli/sei section.
If the load sequence of 4 registers is interrupted halfway and an
interrupt changes one of the bytes that was already loaded into a
register, then the whole volatile concept is meaningless anyway.
Perhaps the warning should be, that if a volatile is more than 8-bits,
and its used both in and outside of an interrupt, it should always be
protected by a block of code that is guaranteed atomic.
> -Alex
>
>
> _______________________________________________
> AVR-GCC-list mailing list
> AVR-GCC-list@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/avr-gcc-list
>
_______________________________________________
AVR-GCC-list mailing list
AVR-GCC-list@nongnu.org
http://lists.nongnu.org/mailman/listinfo/avr-gcc-list
