Hi, folgendes Codefragment optimiert mir der AVR-GCC kaputt: static unsigned int pulseOffCtr=0; main () { ... while (((volatile unsigned int)pulseOffCtr)>0) { if (((volatile unsigned int)pulseOffCtr)>6) usbPoll(); } ... } pulseOffCtr wird in einer Timer-getriggerten ISR verändert. Aus Geschwindigkeitsgründen ist die Variable selber NICHT als volatile definiert. Der Compiler hält den Code in der while-Schleife jetzt für Unfug und rationalisiert diesen weg. Spannenderweise hilft es auch nicht, vor Ort auf volatile zu casten (so wie im Codebeispiel oben). Wie kann ich den GCC davon abhalten, den Codeteil kaputtzuoptimieren? pulseOffCtr generell als volatile zu definieren oder den Optimierungslevel runterzuschalten kommt leider nicht in Frage.
>pulseOffCtr generell als volatile zu definieren
Dann lebe damit.
> pulseOffCtr generell als volatile zu definieren
Warum?
Warum nicht gleich richtig machen?
Karl K. schrieb: > Wie kann ich den GCC davon abhalten, den Codeteil kaputtzuoptimieren? Inline Assembler?
@ 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: > Aus Geschwindigkeitsgründen Sinnlose Mikrooptimierung Karl K. schrieb: > folgendes Codefragment optimiert mir der AVR-GCC kaputt: Wenn du kaputten Code schreibst, brauchst du dich nicht beschweren, wenn das, was dabei rauskommt, nicht funktioniert. Den GCC trifft da keine Schuld. Wenn du eine Funktion (den GCC) mit falschen Daten (dein Code) fütterst, brauchst du dich nicht über ein falsches Ergebnis (Code, der nicht so funktioniert wie du das gerne hättest) wundern. Der Compiler weiß nicht was du dir denkst, der Compiler wertet aus, was du da als Code hinschreibst. Wenn der geschriebene Code nicht das ist, was du denkst, warum sollte der Compiler schuld sein?
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.
:
Bearbeitet durch User
Karl K. schrieb: > static unsigned int pulseOffCtr=0; > > main () > { > ... > while (((volatile unsigned int)pulseOffCtr)>0) > { > if (((volatile unsigned int)pulseOffCtr)>6) > usbPoll(); > } > ... > } Du sollst auch nicht das Ergebnis der Variable nach dem Auslesen casten, sondern die Variable selbt nach volatile casten:
1 | while (*((volatile unsigned int*) &pulseOffCtr) > 0) { ... } |
Falls du C++ verwendest gehts etwas schöner:
1 | while (const_cast<volatile unsigned int&> (pulseOffCtr) > 0) { ... } |
Beim Verwenden von (unsigned) int auf 8-Bit-AVR ist natürlich besondere Vorsicht geboten, da Zugriffe darauf nicht atomar sind. Du musst vor dem Zugriff die Interrupts sperren.
Was Dir eventuell hilft, ist ein Hybrid aus beiden. Du deklarierst die Variable als volarile, verwendest aber für if() einen zwischengespeicherten Wert:
1 | static volatine unsigned int pulseCtr=0; |
2 | |
3 | main() |
4 | {
|
5 | unsigned int tmp; |
6 | while (tmp=pulseCtr) |
7 | {
|
8 | if (tmp>6) |
9 | usbPoll(); |
10 | } |
11 | } |
So wird die "langsame" Variable nur einmal pro Schleifendurchlauf gelesen. Bei der if-Abfrage wird der Wert verwendet, der im Kopf der Schleife gelesen wurde. Der Compiler wird tmp in ein Register speichern, was schneller erreichbar ist, als eine Variable im RAM. Der Kopiervorgang im Schleifenkopf ist zwangsläufg ohnehin nötig. Hier ist noch ein kleiner Fehler: Die Variable ist größer als 8 Bit, du solltest den Lesezugriff daher Atomar machen. Sonst kann es passieren, dass der Counter während des Lesens verändert wird, und das führt je nach Wert zu völlig falscher Funktion. Wenn zum Beispiel der Wert mittem beim Lesezugriff von 255 nach 256 wechselt, bekommst du (tmp) unerwartet den Wert 511. Den fehler hatte ich selbst neulich gemacht und wurde von anderen Forenmitgliedern korrigiert. Nun gebe ich das gelernte gerne weiter :-) Korrekturvorschlag:
1 | static volatine unsigned int pulseCtr=0; |
2 | |
3 | unsigned int getPulseCounter() |
4 | {
|
5 | cli(); |
6 | unsinged int copy=pulseCtr; |
7 | sei(); |
8 | return copy; |
9 | } |
10 | |
11 | main() |
12 | {
|
13 | unsigned int tmp; |
14 | while (tmp=getPulseCounter()) |
15 | {
|
16 | if (tmp>6) |
17 | usbPoll(); |
18 | } |
19 | } |
In diesem Zusammenhang könnte es interessant sein, sich in das Thema "inline" einzulesen.
Ü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. Aus > Geschwindigkeitsgründen ist die Variable selber NICHT als volatile > definiert. Schau mal in generierten ASM Code nach, ob die ISR einfach nur schneller wird weil der Compiler pulseOffCtr einfach rausschmeißt... Zeig* uns mal deine ISR, vielleicht ist sie ja nur ungeschickt geschrieben. *Mit C-Formatierung
1 | [c] C-Code [/c] |
Karl K. schrieb: > pulseOffCtr wird in einer Timer-getriggerten ISR verändert. Dann mußt Du den Zugriff atomar kapseln (int = 2 Byte).
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.
@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 . . .
Falk B. schrieb: > Und wo ist dann der Vorteil zu einer einfachen volatile Deklaration? das sehen wir leider nicht, weil er die ISR nicht gepostet hat. Diese wird ohne das volatile zumindest schneller. Ich mag diese globale volatile auch nicht, weil es viel zu viel von der vorhanden Optimierung verhindert.
Peter II schrieb: > Diese > wird ohne das volatile zumindest schneller. Da diese nicht unterbrochen werden kann, hilft der Trick mit der lokalen Variablen, wenn viel mit der globalen Variablen gerechnet wird.
Tom schrieb: > Peter II schrieb: >> Diese >> wird ohne das volatile zumindest schneller. > > Da diese nicht unterbrochen werden kann, hilft der Trick mit der lokalen > Variablen, wenn viel mit der globalen Variablen gerechnet wird. ist Geschmacksache, 2 Variabel mit dem gleichen Inhalt die man nicht vergessen darf zurückzuschreiben (mal schnell ein return reingeschrieben und auf einmal geht nichts mehr) oder das hässliche cast.
Der Cast macht es auf alle Fälle leichter, sich in dessen Funktion zu verschätzen. Wenn die ISR so unübersichtlich wird, daß man eingestreute return's nicht erkennt, ... Zudem gibt es ja auch die Möglichkeit, daß man die globale Variable gar nicht immer verändern will.
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(); > } > } ?
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? Atomarer Zugriff: Wenn meine Anweisung für einen (kritischen/parallelen) Variablenzugriff in mehr als einen ASM-Befehl übersetzt wird, muss ich diesen Zugriff gegen Interrupts schützen, also unteilbar / atomar machen - Richtig. Bei AVRs mit
1 | cli(); // Interrupts sperren |
2 | DoSomethingDangerous(); |
3 | sei; // Interrupts freigeben |
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>.
> Besser so:
Wieso besser? Das erzeugt exakt den gleichen Rahmen mit cli() und sei().
> in einer ISR aufrufen würde, hätte man danach die > Interrupts eingeschaltet. So war das ja nicht gedacht. In der ISR soll man natürlich direkt auf die Variable zugreifen.
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.)
:
Bearbeitet durch User
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!
:
Bearbeitet durch User
Ralf G. schrieb: > 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>. Die lib kannte ich nicht.. Aber natürlich hast du vollkommen recht Ich programmiere schon länger nur noch Ärmchen, da mache ich das tatsächlich auch immer genau so. Vor allem wenn man irgendwelche Treiber für Peripheriezugriffe auch in ISRs nutzen muss, und evtl. übergeordnete Funktionen daher auch Interrupts ein/ausschalten müssen ists natürlich fatal, einfach immer sei(); zu nutzen.. Allerdings würde ich bei sowas dann lieber nur die tatsächlich gefährdeten Interrupts sperren..
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.
:
Bearbeitet durch User
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.
:
Bearbeitet durch User
Falk B. schrieb: > Solche Cast sind immer ein klares Zeichen von Mur Klar, der AVR-GCC ist ja auch Murks
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.
Thread beobachten
Seitenaufteilung abschaltenBitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.