volatile wirklich nötig?

[Philipp (Gast)]

Hallo,

Ich habe ein externes ADC welches über einen Timer ausgelesen und der 
Wert in einem Buffer gespeichert wird.
Von Zeit zu Zeit lass ich den Buffer auslesen, das Ergebnis aus dem 
Buffer aufsummieren und bilde den Mittelwert.

struct adcBuffer
{
  int16 data[ 8 ][ 8 ];
  uint8 index;
};

...
cli( );
for( uint8 i = 0; i < 8; i++ )
{
  result += _buffer.data[ ch ][ i ];
}
sei( );
..

Eigentlich müsste es auch funktionieren wenn "data[]" im Buffer nicht 
volatile deklariert ist, da meiner Ansicht nach (bin kein Assembler 
spezialist) der ist, dass er den Z Pointer jedesmal neu beschreibt, 
anstatt ihn zu inkrementieren.

Ohne volatile:

ld  r24, Z+
ld  r25, Z+
add  r18, r24
adc  r19, r25
cp  r30, r20
cpc  r31, r21
brne  .-14

mitm volatile:

movw  r30, r22
add  r30, r20
adc  r31, r21
add  r30, r30
adc  r31, r31
subi  r30, 0xEF  ; 239
sbci  r31, 0xFD  ; 253
ldd  r24, Z+9  ; 0x09
ldd  r25, Z+10  ; 0x0a
add  r18, r24
adc  r19, r25
subi  r20, 0xFF  ; 255
sbci  r21, 0xFF  ; 255
cpi  r20, 0x08  ; 8
cpc  r21, r1
brne  .-32       ; 0x12d4

Wenn ich das richtig verstanden habe, welchen grund hat der Compiler 
hier, den Z Pointer jedesmal neu zu beschreiben?
Oder bin ich mit meinen Überlegungen komplett falsch?

grüße
Philipp

[Timmo H. (masterfx)]

AVR-GCC-Codeoptimierung: Puffern von volatile-Variablen

[Andreas B. (andreas_b77)]

Philipp schrieb:
> Eigentlich müsste es auch funktionieren wenn "data[]" im Buffer nicht
> volatile deklariert ist, da meiner Ansicht nach (bin kein Assembler
> spezialist) der ist, dass er den Z Pointer jedesmal neu beschreibt,
> anstatt ihn zu inkrementieren.

Was der Compiler macht oder nicht kann sich mit Optionen und Versionen 
ändern. Anhand des erzeugten Assembler-Code darauf zu schließen, was man 
machen kann wird früher oder später in Tränen enden.

Hier braucht man kein volatile, solange auf adcBuffer wie oben 
zugegriffen wird, also nur im Interrupt und außerhalb nur in cli()/sei() 
geschützten Bereichen.

Im Interrupt braucht man es nicht, da es eine Funktion ist, die nur 
aufgerufen, durchgelaufen und beendet wird ohne dass es unterbrochen 
wird oder parallel etwas anderes läuft. Also genau so, wie es der 
Compiler annimmt.

Außerhalb braucht man das volatile nicht, weil durch das cli() auch 
nichts unterbricht und nichts parallel läuft. Ein Problem wäre, dass der 
Compiler Daten aus dem Array vor dem cli() oder nach dem sei() laden 
oder speichern könnte. Das passiert hier nicht, weil die cli() und sei() 
Makros zweckmäßigerweise als Speicherbarrieren definiert sind, d.h. der 
Compiler darf beim Optimieren niemals Speicherzugriffe über diese beiden 
Makro-Aufrufe hinweg verschieben.

Also wird hier kein volatile gebraucht. Auch wenn man es an einzelnen 
Stellen brauchen würde, muss man nicht gleich die ganze Variable 
volatile machen, man kann auch bei den entsprechenden Zugriffen die 
Variable nach volatile casten.

[Jörg W. (dl8dtl) (Moderator)]

Andreas B. schrieb:
> Hier braucht man kein volatile, solange auf adcBuffer wie oben
> zugegriffen wird, also nur im Interrupt und außerhalb nur in cli()/sei()
> geschützten Bereichen.

Der Interruptschutz ist für volatile unerheblich.  Interessant ist,
ob der Compiler an der gegebenen Stelle noch implizite Annahmen
machen kann, welchen Wert _buffer tatsächlich hat bzw. ob er
schließen kann, dass das Beschreiben eigentlich für die Katz' ist
und daher auch weggelassen werden darf (weil sich an der Funktion
des sich ergebenden Codes aus seiner Sicht dann nichts ändert).

Mit dem Inkrementieren irgendwelcher Zeiger oder anderer
Implementierungsdetails hat das alles ohnehin nichts (direkt) zu
tun.  volatile ist dafür da, dem Compiler anzuzeigen, dass das
damit gekennzeichnete Objekt auf eine Weise geändert werden könnte,
die für ihn aus dem aktuellen Programmfluss nicht ersichtlich ist
(bspw. direkt durch die Hardware, wie bei IO-Registern, oder durch
andere Threads, wie eben in einer ISR).

Bezeichner, die mit Unterstrichen anfangen, sollte man (so man die
Regeln des C-Standards nicht gerade ganz genau kennt) tunlichst
vermeiden, da viele von ihnen "reserved for the implementation" sind.

[David .. (volatile)]

:(

[Michael B. (mb_)]

Andreas B. schrieb:
> Hier braucht man kein volatile, solange auf adcBuffer wie oben
> zugegriffen wird, also nur im Interrupt und außerhalb nur in cli()/sei()
> geschützten Bereichen.

> Ein Problem wäre, dass der
> Compiler Daten aus dem Array vor dem cli() oder nach dem sei() laden
> oder speichern könnte. Das passiert hier nicht, weil die cli() und sei()
> Makros zweckmäßigerweise als Speicherbarrieren definiert sind,

cli() und sei() sind nicht als memory barriers definiert:

mb@maggie:~$ egrep -e '(cli)|(sei)' /usr/lib/avr/include/avr/interrupt.h 
/** \def sei()
#define sei()
# define sei()  __asm__ __volatile__ ("sei" ::)
/** \def cli()
#define cli()
# define cli()  __asm__ __volatile__ ("cli" ::)

Der Compiler kann daher einen Variablenzugriff so verschieben, dass er 
nicht mehr innerhalb des IRQ-disable bereiches liegt.

[Johann L. (gjlayde)]

Michael Buesch schrieb:
> Andreas B. schrieb:
>> Makros zweckmäßigerweise als Speicherbarrieren definiert sind,
>
> cli() und sei() sind nicht als memory barriers definiert:
>
>

> mb@maggie:~$ egrep -e '(cli)|(sei)' /usr/lib/avr/include/avr/interrupt.h
> /** \def sei()
> #define sei()
> # define sei()  __asm__ __volatile__ ("sei" ::)
> /** \def cli()
> #define cli()
> # define cli()  __asm__ __volatile__ ("cli" ::)
>

>
> Der Compiler kann daher einen Variablenzugriff so verschieben, dass er
> nicht mehr innerhalb des IRQ-disable bereiches liegt.

Sagt wer? Ich halte mal dagegen mit avr-libc 1.7.1:

# 83 "./avr/include/avr/interrupt.h" 3
#define sei() __asm__ __volatile__ ("sei" ::: "memory")
# 103 "./avr/include/avr/interrupt.h" 3
#define cli() __asm__ __volatile__ ("cli" ::: "memory")

[Philipp (Gast)]

vielen Dank Andreas und Jörg für eure detaillierten Antworten.

[Michael B. (mb_)]

Johann L. schrieb:
>> cli() und sei() sind nicht als memory barriers definiert:
>>
>>

>> mb@maggie:~$ egrep -e '(cli)|(sei)' /usr/lib/avr/include/avr/interrupt.h
>> /** \def sei()
>> #define sei()
>> # define sei()  __asm__ __volatile__ ("sei" ::)
>> /** \def cli()
>> #define cli()
>> # define cli()  __asm__ __volatile__ ("cli" ::)
>>

>>
>> Der Compiler kann daher einen Variablenzugriff so verschieben, dass er
>> nicht mehr innerhalb des IRQ-disable bereiches liegt.
>
> Sagt wer?

ich?

> Ich halte mal dagegen mit avr-libc 1.7.1:
>
>

# 83 "./avr/include/avr/interrupt.h" 3
> #define sei() __asm__ __volatile__ ("sei" ::: "memory")
> # 103 "./avr/include/avr/interrupt.h" 3
> #define cli() __asm__ __volatile__ ("cli" ::: "memory")

Tja, wenn man portablen Code schreiben will, darf man sich eben nicht 
darauf verlassen. Aber schön zu sehen, dass dies in neueren avr-libc 
gefixt ist.

[(prx) A. K. (prx)]

Michael Buesch schrieb:

> Tja, wenn man portablen Code schreiben will, darf man sich eben nicht
> darauf verlassen. Aber schön zu sehen, dass dies in neueren avr-libc
> gefixt ist.

Portabler Code mit direktem Aufruf von cli/sei. Aber klar doch.

[Michael B. (mb_)]

A. K. schrieb:
> Michael Buesch schrieb:
>
>> Tja, wenn man portablen Code schreiben will, darf man sich eben nicht
>> darauf verlassen. Aber schön zu sehen, dass dies in neueren avr-libc
>> gefixt ist.
>
> Portabler Code mit direktem Aufruf von cli/sei. Aber klar doch.

Gehe nochmal zurück auf Los und lies worum es hier geht.