AVR-gcc. Optimierung ignoriert Vergleich mit volatile Variable

[Burkhard (Gast)]

Habe ein Problem mit einer Routine, welche eine volatile Variable mit 
einer anderen vergleicht. Das ganze sollte eine FIFO für den USART1 
darstellen.
Hier das problematische Codesegment
(vollständiger Teil und generiertes Assemblerlisting im Anhang) :

#define BT_RXBUFSIZE  64
volatile unsigned char bt_rxbuffer[BT_RXBUFSIZE];  // Bluetooth-FIFO-Empfangspuffer
volatile unsigned char *bt_rx_wrptr;      // Zeiger auf nächste freie Schreibposition
unsigned char *bt_rx_rdptr;        // Zeiger auf nächste Leseposition. 

// Ein Zeichen aus dem RX-Puffer lesen.
// Return 0 : OK, 1 : Timeout (in 50usec Schritten)
uint8_t get_btrx(uint8_t *data, uint16_t timeout)
{
  do {
    if(bt_rx_rdptr!=bt_rx_wrptr) {  // Daten vorhanden
      *data=*bt_rx_rdptr++;
      if(bt_rx_rdptr==bt_rxbuffer+BT_RXBUFSIZE)
        bt_rx_rdptr=(unsigned char *)bt_rxbuffer;
      return 0;
    }      
    _delay_us(50);
  } while(--timeout);
  return 1;
}

/* Zeichen vom BT-Modul empfangen */
ISR(USART1_RX_vect)
{
  if((UCSR1A & ((1 << RXC1) | (1 << FE1) | (1 << DOR1) | (1 << UPE1))) != (1 << RXC1)) {
    UDR1;        // Lesen und wegwerfen ;-)
    *bt_rx_wrptr++=0;    // Bei Fehler 0-Zeichen
  } else
    *bt_rx_wrptr++=UDR1;
  if(bt_rx_wrptr==bt_rxbuffer+BT_RXBUFSIZE)
    bt_rx_wrptr=bt_rxbuffer;
}

Das Problem liegt in der Funktion get_btrx(). Wenn ich den mit -Os 
Optimierung erzeugten Assembler-Code richtig interpretiere, wird der 
if(bt_rx_rdptr!=bt_rx_wrptr) - Vergleich nur einmal aufgerufen und die
do-while-Schleife springt wieder zum Anfang des _delay_us(50) statt
zurück zum if-Vergleich. Bei der Optimierung -O0 ist alles OK,
bei -O2 funktioniert es auch nicht.
Die Deklaration des bt_rx_rdptr Zeigers als volatile bringt keine
Änderung (ist auch nicht sinnvoll, da der USART-Rx-Interrupt sie auch
nicht ändert).

Die USART-Konfiguration und die ISR für den USART1_RX_vect habe ich 
getestet. Das Zeichen wird korrekt empfangen und auch im Puffer 
gespeichert. Der erste Aufruf von get_btrx() ergibt ein Timeout (obwohl
vor Ablauf des Timeouts ein Zeichen über die USART empfangen wurde). 
Nachfolgende Aufrufe liefern das vorherige Zeichen aus dem Ringpuffer.

Ich finde keinen Fehler im C-Code und vermute ein Bug im gcc.
Kann ich als Workaround die Optimierung für diesen Bereich gezielt 
abschalten ?


AVR-GCC-Version : 4.4.3 , avr-libc 1.6.8
Zielprozessor : ATMega128A
Host: Ubuntu-Linux 8.04


Gruesse aus Mülheim ;-)

Burkhard

[der mechatroniker (Gast)]

Klassischer Fehler:

volatile typ *variable
ist etwas anderes als
volatile typ *volatile variable

sprich bt_rx_wrptr ist nicht volatile.

[Oliver (Gast)]

der mechatroniker schrieb:
> volatile typ *variable
> ist etwas anderes als
> volatile typ *volatile variable

und gewollt ist vermutlich

typ *volatile variable


Oliver

[Peter D. (peda)]

Und zusätzlich noch den Zugriff auf bt_rx_wrptr im Main atomar kapseln, 
da 2 Byte!


Peter

[Εrnst B. (ernst)]

Peter Dannegger schrieb:
> Und zusätzlich noch den Zugriff auf bt_rx_wrptr im Main atomar kapseln,
> da 2 Byte!
Und wenn er den Zugriff atomar kapselt, dann braucht er auch das 
volatile nicht mehr, da die entsprechenden Macros aus der util/atomic.h 
schon eine memory barrier enthalten:

 __asm__ volatile ("" ::: "memory");

[Burkhard (Gast)]

Danke für die schnelle Antwort. Werde ich morgen direkt probieren und 
berichten :-)

Kleine Verständnisfrage an 'der mechatroniker' bzw. Oliver :

Was bedeutet denn

volatile unsigned char *ptr; ?

Ich dachte immer es bedeutet, daß sich die Adresse des Zeigers 
bt_rx_wrptr durch einen Interrupt ändern kann (unabhängig vom Zeichen 
auf welches er zeigt).

Und daß

unsigned char volatile *ptr;

bedeutet, daß sich das Zeichen, auf welches der Pointer zeigt, durch 
einen Interrupt ändern kann.

Ist das korrekt ? Oder ist es genau anders herum ?

Danke auch an Peter Dannegger. Im Prinzip richtig, daß ich den 16 Bit- 
Vergleich als 'atomic' machen könnte. Da ich aber nur auf Gleichheit 
teste und nicht mit dem Wert rechne, spielt das in diesem Fall (denke 
ich) keine Rolle.


Hallo Ernst !
Den Trick kenne ich auch noch nicht. Werde ich mal ausprobieren :-)

Gruss
Burkhard

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

Burkhard schrieb:

> Ist das korrekt ? Oder ist es genau anders herum ?

Weder noch. Ob du
  volatile type * name;
oder
  type volatile * name;
schreibst ist egal. In beiden Fällen ist das worauf der Zeiger zeigt 
volatil. Wenn der Zeiger selbst volatil sein soll, dann heisst es
  type * volatile name;

Das sieht nicht wirklich elegant aus, aber als C nachträglich die 
const/volatile qualifier verpasst bekam war die Syntax nicht mehr zu 
retten.

[Karl H. (kbuchegg)]

Burkhard schrieb:

> Ist das korrekt ? Oder ist es genau anders herum ?

const und volatile beziehen sich immer auf das was links von ihnen 
steht. Es sei denn, diese Qualifier sind bereits ganz links, dann 
beziehen sie sich ausnahmsweise auf den Teil des Datentyps rechts von 
ihnen (was aber aufs gleiche rauskommt)

Also:

  char volatile i;

  i                    da ist ein i
  volatile i           dieses i ist volatile
  char volatile i      dieses i ist ein volatile char

(Und das ist identisch zu   volatile char i;
 i ist ein char. Und dieser char ist volatile)

  char volatile * i;

 i                      es gibt ein i
 * i                    dieses i ist ein Pointer.
 volatile * i           dieser Pointer zeigt auf etwas was volatile ist
 char volatile * i      Nämlich einen char

also ist i ein Pointer der auf einen volatile char zeigt

  char * volatile i;

 i                      es gibt ein i
 volatile i             dieses i ist volatile
 * volatile i           dieses i ist ein volatiler Pointer
 char * volatile i      und dieser volatile Pointer zeigt auf einen char
                        (der selbst aber nicht volatile ist)

  char volatile * volatile i

 i                             es gibt ein i
 volatile i                    dieses i ist volatile
 * volatile i                  und dieses i ist ein volatile Pointer
 volatile * volatile i         und dieser volatile Pointer zeigt auf 
etwas,
                               was selbst auch volatile ist
 char volatile * volatile i    nämlich einen char

i ist also ein volatiler Pointer, der auf einen volatilen char zeigt.

immer beim Variablennamen anfangen und sich nach aussen durcharbeiten!

[Burkhard (Gast)]

Entschuldigung. Flüchtigkeitsfehler: 'unsigned char volatile *ptr;' 
sollte auch 'unsigned char * volatile ptr;' heissen.

Wenn ich Dich also richtig verstehe bezieht sich alles NACH dem '*' auf 
den Pointer selbst und das VOR dem '*' auf das Element, auf welches er 
zeigt ?!

Gab es zu meiner Kernighan-Ritchie-Zeit irgendwie noch nicht da ;-).

Aber nochmals besten Dank !


Burkhard

[Karl H. (kbuchegg)]

Burkhard schrieb:

> Wenn ich Dich also richtig verstehe bezieht sich alles NACH dem '*' auf
> den Pointer selbst und das VOR dem '*' auf das Element, auf welches er
> zeigt ?!

  was ist ein

  int ** volatile * ptr;

:-)

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

Karl heinz Buchegger schrieb:

>   was ist ein
>   int ** volatile * ptr;

Und was ist (bzw war, anno DOS/Win16) ein

    int * far * volatile * ptr;

[Peter D. (peda)]

Burkhard schrieb:
> Da ich aber nur auf Gleichheit
> teste und nicht mit dem Wert rechne, spielt das in diesem Fall (denke
> ich) keine Rolle.

In diesem Fall könnte man es weglassen.
Es müßte grad das 256 Byte eintrudeln, damit eine falsche Gleichheit 
entsteht. Und dann verlierst Du auch nur 50µs bis zum nächsten richtigen 
Vergleich, ist also nicht tragisch.

Allerdings könnte es sich lohnen statt volatile atomic zu nehmen, 
immerhin macht der Interrupt 6 Zugriffe auf diese Variable.
Oder man nimmt eine Tempvariable im Interrupt.


Peter

[Burkhard (Gast)]

OK - Habe ich verstanden ...

Werde von C wieder nach Assembler zurück wechseln :-).

[Burkhard (Gast)]

Hallo Peter ...

das mit dem Atomic (wie auch Ernst meinte) werde ich mir morgen
auf jeden Fall im generierten Assemblercode ansehen.

Gruss

[Klaus (Gast)]

A. K. schrieb:
> int  far  volatile * ptr;

Karl heinz Buchegger schrieb:
> int ** volatile * ptr;


Solche Ausdrücke sind doch gegen die Menschenwürde!!! oO

[Burkhard (Gast)]

Danke nochmal an alle C-Gurus

Beide Lösungsvorschläge funktionieren :

- Das Kapseln als 'Atomic' klappt :

ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
  if(bt_rx_rdptr!=bt_rx_wrptr) {  // Daten vorhanden
    *data=*bt_rx_rdptr++;
    if(bt_rx_rdptr==bt_rxbuffer+BT_RXBUFSIZE)
      bt_rx_rdptr=(unsigned char *)bt_rxbuffer;
    return 0;
  }
}

und bei korrekter Deklaration des Pointers ist auch alles OK :


volatile unsigned char * volatile bt_rx_wrptr;