AVR-GCC: Code wird totoptimiert

W.A. schrieb:
> Inline Assembler?

Coole Idee - Danke!

@ Karl K. (karl_k69)

>folgendes Codefragment optimiert mir der AVR-GCC kaputt:


>       while (((volatile unsigned int)pulseOffCtr)>0)

Solche Cast sind immer ein klares Zeichen von Murks.

>pulseOffCtr wird in einer Timer-getriggerten ISR verändert. Aus
>Geschwindigkeitsgründen ist die Variable selber NICHT als volatile
>definiert.

Das ist ein Oxymoron. Mach die Variable volatile und fertig. Es wird 
nicht nennenswert langsamer, denn deine ISR braucht so oder so einige 
Takte.

> Der Compiler hält den Code in der while-Schleife jetzt für
>Unfug und rationalisiert diesen weg.

Recht hat er.

>pulseOffCtr generell als volatile zu definieren oder den

Ja.

Falk B. schrieb:
> Mach die Variable volatile und fertig. Es wird
> nicht nennenswert langsamer, denn deine ISR braucht so oder so einige
> Takte.

Außerdem kann man sich auch in der ISR auf einfache Weise mit Hilfe 
einer lokalen Variable des volatile entledigen.

Karl K. schrieb:
> Spannenderweise hilft es auch
> nicht, vor Ort auf volatile zu casten (so wie im Codebeispiel oben).

Nicht weiter erstaunlich. Du castest auf diese Art nicht die Variable 
selbst, sondern den gelesenen Wert. Das ist zu spät. Allenfalls
  *(volatile ... *)&variable
brächte dich weiter. Nur wirst du dann möglicherweise feststellen, dass 
du das an diversen anderen Stellen auch machen musst und dich jenem Code 
näherst, denn du auch mit "volatile" erhältst.

> Wie kann ich den GCC davon abhalten, den Codeteil kaputtzuoptimieren?

GCC passend für deine Interpretation von C umschreiben.

Übrigens kann der Code auch mit "volatile" immer noch ab und zu in die 
Hose gehen, sobald diese 2-Byte-Variable Werte ausserhalb 0..255 
annimmt. Weil der Zugriff darauf nicht atomar ist. Siehe
https://www.mikrocontroller.net/articles/Interrupt#Atomarer_Datenzugriff

Karl K. schrieb:
> pulseOffCtr wird in einer Timer-getriggerten ISR verändert.

Dann mußt Du den Zugriff atomar kapseln (int = 2 Byte).

@Dr. Sommer (Gast)

>Du sollst auch nicht das Ergebnis der Variable nach dem Auslesen
>casten, sondern die Variable selbt nach volatile casten:

>while (*((volatile unsigned int*) &pulseOffCtr) > 0) { ... }

Und wo ist dann der Vorteil zu einer einfachen volatile Deklaration?

Wie man Probleme mit Casts löst, die man ohne sie nie hätte . . .

Bastler schrieb:
> Volatile bleibt volatile.
> Wenn ich in einer ISR verhindern will, daß volatile den GCC am
> optimieren hindert, dann Kopier ich mir die Variable in eine lokale
> Variable und am Ende wieder zurück. GCC hält die in der Regel in einem
> Register und erzeugt damit sehr kompakten Code. Wichtig ist nur: keine
> andere ISR darf auch an der Variable herumspielen. Und die Hauptschleife
> sieht die Variable, wie sie ist: ohne erkennbaren Zusammenhang zur Zeit.

Querleser: mir ist die Option eine Variable "automar" zusetzen noch nie 
untergekommen, um das Umzusetzen kann man sich hier an die Möglichkeit 
von Stefan U. halten?

Stefan U. schrieb:
> Korrekturvorschlag:
> static volatine unsigned int pulseCtr=0;
>
> unsigned int getPulseCounter()
> {
>     cli();
>     unsinged int copy=pulseCtr;
>     sei();
>     return copy;
> }
>
> main()
> {
>        unsigned int tmp;
>        while (tmp=getPulseCounter())
>        {
>            if (tmp>6)
>            usbPoll();
>        }
> }

?

Stefan U. schrieb:
> unsinged int copy=pulseCtr;

sehr unmusikalischer Code ;)

Besser so:

1

#include <util/atomic.h>

2

3

  ATOMIC_BLOCK(ATOMIC_FORCEON){

4

    return pulseCtr;

5

  }

Henry P. schrieb:
> Querleser: mir ist die Option eine Variable "automar" zusetzen noch nie
> untergekommen, um das Umzusetzen kann man sich hier an die Möglichkeit
> von Stefan U. halten?

Das ist so nicht ganz in Ordnung. Wenn man so eine Funktion ...
Stefan U. schrieb:
> unsigned int getPulseCounter()
..., z.B., in einer ISR aufrufen würde, hätte man danach die Interrupts 
eingeschaltet.

Die korrekte Herangehensweise ist:
- den Interruptstatus merken
- Interrupts abschalten
- de vorherigen Status wieder herstellen

Dafür gibt's (mit etwas Schreibarbeit) Makros in der <util/atomic.h>.

Ralf G. schrieb:
> - den Interruptstatus merken

In der Regel weiß man ja, daß man gerade einen Interrupthandler schreibt 
und kann direkt auf die Variable zugreifen, da der AVR ohne dirty Tricks 
keine nested Interrupts kann.

Ich hab das ATOMIC_RESTORESTATE aber einmal gebraucht, da Code erst im 
Init vor der Interruptfreigabe und dann im Main aufgerufen wurde.

Ralf G. schrieb:
> ... würde, hätte ...

Stefan U. schrieb:
> ... natürlich ...

Es steht eben an der Funktion nicht dran, das die für ISRs strengstens 
verboten ist! (Abgesehen davon, dass Funktionsaufrufe in einer ISR 
eher 'nicht so optimal' sind.)

Stefan U. schrieb:
> Wieso besser?

Es erschlägt das volatile gleich mit.
Es läßt sich leichter auf andere CPUs portieren.

Peter D. schrieb:
> In der Regel weiß man ja, ...
Ja. Aber man muss wissen warum.

Stefan U. schrieb:
> Wieso besser?
Es geht nicht darum, mal einfach [ aus Spaß ;) ] cli() und sei() 
aufzurufen. Es soll ein atomarer Zugriff erfolgen. Und da helfen die 
Makros aus der <util/atomic.h>, das ordentlich zu dokumentieren!

dunno.. schrieb:
> Allerdings würde ich bei sowas dann lieber nur die
> tatsächlich gefährdeten Interrupts sperren..

Würde ich gerade nicht.
Die ARM können ja Interruptlevel. Würde man nur einen Interrupt sperren, 
könnte sich ein langsamer und unwichtiger Interrupt vordrängeln und so 
den gesperrten zu lange verzögern.
Nur die globale Sperre sichert, daß es nicht zu einer 
Prioritätsinversion kommt. Sie verkürzt außerdem die Sperre auf die 
minimal mögliche Dauer.

Peter D. schrieb:
> Nur die globale Sperre sichert, daß es nicht zu einer
> Prioritätsinversion kommt. Sie verkürzt außerdem die Sperre auf die
> minimal mögliche Dauer.

Zumindest bei den Cortexen empfiehlt sich eine dritte Variante, die 
höher priorisierte Interrupts nicht behindert.

Die haben mit dem BASEPRI/BASEPRI_MAX Register eine sehr einfache 
Möglichkeit, den aktuellen Level auf den des betreffenden Interrupts zu 
setzen. Dann sind höher priorisierte Interrupts zulässig, alle anderen 
gesperrt.

Also sinngemäss:
  unsigned savedPRIO = BASEPRI;
  BASEPRI_MAX = meinePRIO;
  ... der betroffene Code ...
  BASEPRI = savedPRIO;
Dank spezieller Eigenschaften des BASEPRI_MAX Registers darf das auch 
verschachtelt werden, d.h. das funktioniert selbst dann, wenn im 
geschützten Code in einer aufgerufenen Funktion diese Sequenz mit einem 
anderen Level auftritt. Die Sequenz ist auch in ISRs zulässig.

A. K. schrieb:
> Dann sind
> höher priorisierte Interrupts zulässig, alle anderen gesperrt.

Ich wüßte nicht, was das für einen Vorteil hätte. Es würde aber das 
Atomic Macro unnötig verkomplizieren.

Das BASEPRIE ist allerdings eine elegante Möglichkeit, das Atomic Macro 
zu realisieren.
Soweit ich mich erinnere, ist das globale Enable Flag privilegiert, d.h. 
ein Zugriff darauf mit erheblichem Aufwand verbunden.

Peter D. schrieb:
> Ich wüßte nicht, was das für einen Vorteil hätte.

Eigentlich denjenigen, den du selbst genannt hast: Wenn man alle 
Interrupts abschaltet, dann sind auch höher priorisierte Interrupts für 
diese Dauer gesperrt, d.h. werden entsprechend verzögert. Mit BASEPRI 
hingegen kommen die unverändert durch, d.h. werden in keinster Weise 
verzögert.

A. K. schrieb:
> Wenn man alle
> Interrupts abschaltet, dann sind auch höher priorisierte Interrupts für
> diese Dauer gesperrt, d.h. werden entsprechend verzögert.

Ich nehme Atomic aber nicht dafür, ganze Unterfunktionen mit komplexen 
Berechnungen damit zu kapseln, sondern nur, um Daten konsistent zu 
halten, also einige LD oder ST Operationen lang. Da halte ich eine 
Unterbrechbarkeit für unnötig. Schon ein Interrupt-Prolog/-Epilog 
dürften länger brauchen, als nur ein paar LD/ST.