Hi zusammen,
ich hab eine Timer-ISR, die mit 500Hz aufgerufen wird. Im Normalfall
startet die nur eine ADC-Wandlung (ein Bit setzen), software-mäßig wird
die Aufruffrequenz durch 5 geteilt und damit jedes 5te mal eine
komplexere Funktion aufgerufen. Damit die anderen 4 mal die ISR schnell
bleibt, hab ich diesen teil in eine Funktion ausgelagert.
Denkste: der gcc hat die Funktion natürlich inline gemacht. Also mit
__attribute__((noinline)) behandelt.
Jetzt hab ich eine sub, aber die rettet ihre Register nicht selbst auf
den Stack, hat aber eigentlich überhaupt keinen Pro- und Epilog, dafür
die ISR einen großen.
ich möchte aber in der ISR möglichst wenige Register hin- und
herschieben, weil das in 4 von 5 Fällen umsonst ist.
-fno-caller-saves hab ich schon probiert, bringt nix.
Code:
Michael Reinelt schrieb:> Damit die anderen 4 mal die ISR schnell> bleibt, hab ich diesen teil in eine Funktion ausgelagert.
Das ist aber ein non-sequitur ("folgt nicht")
Das eine hat mit dem andern nichts zu tun. Ob eine ISR schnell
abgearbeitet wird oder nicht, hat nichts damit zu tun, wieviel Code du
da reinschachtelst, sondern welcher Code tatsächlich ausgeführt wird.
> Denkste: der gcc hat die Funktion natürlich inline gemacht.
Na sei doch froh!
> Also mit> __attribute__((noinline)) behandelt.
Wenn du deinem Compiler Prügel zwischen die Beine wirfst, ist dir nicht
zu helfen.
Michael Reinelt schrieb:
> ich möchte aber in der ISR möglichst wenige Register hin- und> herschieben
Wie wäre es nur die Register zu retten die Du wirklich retten musst? Das
ist schon das minimum was Du machen musst. So wie ich dich verstehe
würdest du aber am liebsten komplett drauf verzichten. Pustekuche!
Das muss gemacht werden, da führt kein weg dran vorbei.
Michael Reinelt schrieb:
> weil das in 4 von 5 Fällen umsonst ist
Programmablauf überdenken?!
Michael Reinelt schrieb:
> Denkste: der gcc hat die Funktion natürlich inline gemacht
Ich verstehe dein Problem nicht.
Michael Reinelt schrieb:
> Damit die anderen 4 mal die ISR schnell bleibt,
Lass uns mal ganz kurz nachdenken:
Also, deine ISR wird mit 500Hz getriggert, dein µC (welchen auch immer
du benutzt, steht ja nirgends!) läuft schätzungsweise mit 8/16/20 MHz,
also mind. 16.000 mal schneller... und du machst dir da jetzt n Kopf um
ein paar Taktzyklen um Register "unnötigerweise" zu retten?!
Bis deine ISR das nächste mal getriggert wird, hat sich der µC doch
schon Äonen gelangweilt.
Naja, Zeit kann ja schonmal knapp werden, auch auf nem fixen uC.
Manche uC haben doch aber auch Autotrigger-Funktionen für z.B. den ADC.
Dann kannst du den ADC automatisch mit 500Hz starten und mit einem
unabhänigen Timer deine Funktion tatsächlich nur alle 100Hz aufrufen.
Forsche doch mal in die Richtung.
Gruß
Fabian
Hallo zusammen,
erstmal danke für die Antworten. Aber ich fürchte ich hab mich unklar
ausgedrückt, und werde deshalb mißverstanden....
> Das eine hat mit dem andern nichts zu tun. Ob eine ISR schnell> abgearbeitet wird oder nicht, hat nichts damit zu tun, wieviel Code du> da reinschachtelst, sondern welcher Code tatsächlich ausgeführt wird.
Richtig.
>> Denkste: der gcc hat die Funktion natürlich inline gemacht.>> Na sei doch froh!>>> Also mit>> __attribute__((noinline)) behandelt.>> Wenn du deinem Compiler Prügel zwischen die Beine wirfst, ist dir nicht> zu helfen.
Nö, keine Prügel.
ich versuchs nochmal zu erklären wo mein Problem liegt:
meine komplexe Funktion braucht viele Register, die deshalb gesichert
und wieder zurückgesichert werden (die push und pop-Schlangen)
Diese komplexe Funktion wird aber nur selten (jedes fünfte mal)
aufgerufen, deshalb ist das sichern und rücksichern der Register in den
anderen vier Fällen umsonst, weil die Register gar nicht geändert
werden.
der jedesmal ausgeführte Teil der ISR braucht nur R24. deshalb sollte im
Prolog der ISR nur ein "push R24" und im Epilog ein "pop R24" stehen
(ausgenommen natürlich der "Standard Pro/Epilog mit __zero_reg usw), die
ganzen anderen Register sollten im Pro-/Epilog der "RSM_worker()" (und
damit nur bei Bedarf) gesichert/rückgesichert werden.
Kaj schrieb:> Michael Reinelt schrieb:>> ich möchte aber in der ISR möglichst wenige Register hin- und>> herschieben>> Wie wäre es nur die Register zu retten die Du wirklich retten musst? Das> ist schon das minimum was Du machen musst. So wie ich dich verstehe> würdest du aber am liebsten komplett drauf verzichten. Pustekuche!> Das muss gemacht werden, da führt kein weg dran vorbei.
nachdem ich in C programmiere, kümmert sich da der Compiler drum, aber
er tut hier zuviel des Guten, vermute ich.
> Michael Reinelt schrieb:>> weil das in 4 von 5 Fällen umsonst ist>> Programmablauf überdenken?!
Da gibts nix zu überdenken: Ein Teil muss mit 500 Hz ausgeführt werden,
der andere mit 100 Hz. Und einen zweiten freien Timer hab ich nciht.
> Lass uns mal ganz kurz nachdenken:> Also, deine ISR wird mit 500Hz getriggert, dein µC (welchen auch immer> du benutzt, steht ja nirgends!)
sorry, ATmega328P
> läuft schätzungsweise mit 8/16/20 MHz,> also mind. 16.000 mal schneller... und du machst dir da jetzt n Kopf um> ein paar Taktzyklen um Register "unnötigerweise" zu retten?!> Bis deine ISR das nächste mal getriggert wird, hat sich der µC doch> schon Äonen gelangweilt.
Woher willst du das wissen? Vielleicht laufen andere ISR, vielleicht ist
das Hauptprogramm zeitkritisch?
Wahrscheinlich hast du sogar recht, trotzdem mag ich es nicht wenn
unnötige Rechenzeit verbraten wird. Es ist einfach unschön (und genau
deshalb sind viele moderne .Net und C# Programme so schnarchlangsam,
weil "es ist doch egal, wir haben ja eh genug CPU power")
Michael Reinelt schrieb:> Diese komplexe Funktion wird aber nur selten (jedes fünfte mal)> aufgerufen, deshalb ist das sichern und rücksichern der Register in den> anderen vier Fällen umsonst, weil die Register gar nicht geändert> werden.
Deine ISR ist falsch konzeptioniert. Wenn es das Hauptprogramm erlaubt,
lagere den Verarbeitungs-Teil komplett aus der ISR aus und setzte dort
einfach nur ein Flag; das wird dann im Hauptprogramm geprüft und ggfs.
die Routine ausgeführt. Falls (und ich denke dass ist der Fall) du die
aufgelaufenen ADC-Daten verarbeiten willst, must Du dir natürlich
Gedanken machen, wie man verhindert, dass die alten Werte überschrieben
werden (also entweder ADC sperren bis Verarbeitung OK oder Daten
puffern)
micha schrieb:> Michael Reinelt schrieb:>> Diese komplexe Funktion wird aber nur selten (jedes fünfte mal)>> aufgerufen, deshalb ist das sichern und rücksichern der Register in den>> anderen vier Fällen umsonst, weil die Register gar nicht geändert>> werden.>> Deine ISR ist falsch konzeptioniert. Wenn es das Hauptprogramm erlaubt,> lagere den Verarbeitungs-Teil komplett aus der ISR aus und setzte dort> einfach nur ein Flag; das wird dann im Hauptprogramm geprüft und ggfs.> die Routine ausgeführt. Falls (und ich denke dass ist der Fall) du die> aufgelaufenen ADC-Daten verarbeiten willst, must Du dir natürlich> Gedanken machen, wie man verhindert, dass die alten Werte überschrieben> werden (also entweder ADC sperren bis Verarbeitung OK oder Daten> puffern)
Das geht so nicht, da in der ISR u.a. ein Inverter-Takt erzeugt wird.
Die "komplexe" Funktion ist zwar komplex im Sinne von umfangreich (und
braucht deshalb viele Register), aber eigentlich eh schnell (keine
Schleifen oder so), und etwas zeitkritisch. Das soll schon in der ISR
bleiben.
Die "ausgelagerte" Funktion mit dem Attribut "signal" versehen, und die
ISR per Hand selbst in Assembler schreiben (entweder "naked" und
Inline-ASM, oder als separates Assembler-Modul).
Michael Reinelt schrieb:> Vielleicht laufen andere ISR
Irrelevant. Für die Latenz anderer ISRs ist eh nur der Worst-Case
interessant, und der verschlechtert sich durch dein Vorgehen sogar ein
wenig.
Michael Reinelt schrieb:> trotzdem mag ich es nicht wenn> unnötige Rechenzeit verbraten wird.
Dann musst du meinen Vorschlag aber auch ablehnen, denn da wird dann ja
schließlich in der Funktion das SREG und R24 überflüssigerweise nochmal
gerettet/wiederhergestellt.
Irgendwo ist halt eine Grenze, wo der Nutzen den Mehraufwand
(insbesondere auch bei der späteren Wartung des Codes) nicht mehr
rechtfertigt. Viele würden halt sagen, dass dein "Optimieren" bei einem
500Hz Interrupt die Grenze bereits überschreitet.
Hallo,
wenn Du die Funktion über einen Pointer aufrufst, dürfte sie doch eigtl.
nicht vom Compiler geinlined werden? Bleibt die Prolog/Epilog-Situation
dann so unbefriedigend?
Vlg
Timm
Timm Reinisch schrieb:> Hallo,>> wenn Du die Funktion über einen Pointer aufrufst, dürfte sie doch eigtl.> nicht vom Compiler geinlined werden? Bleibt die Prolog/Epilog-Situation> dann so unbefriedigend?
Gute Idee, muss ich ausprobieren...
ich bin sowieso verwundert, dass das sichern und rücksichern der
register nicht grundsätzlich in der Funktion selbst passiert. Angenommen
die Funktion käme in eine Library, woher soll denn der Aufrufer dann
wissen welche Register von der Funktion verändert werden, und der
Aufrufer daher sichern müsste?
Michael Reinelt schrieb:> Gute Idee, muss ich ausprobieren...
Ob direkt oder per Pointer ändert nichts am Prolog/Epilog.
Michael Reinelt schrieb:> Angenommen> die Funktion käme in eine Library, woher soll denn der Aufrufer dann> wissen welche Register von der Funktion verändert werden, und der> Aufrufer daher sichern müsste?
Das sagt ihm die ABI.
Stefan Ernst schrieb:>> die Funktion käme in eine Library, woher soll denn der Aufrufer dann>> wissen welche Register von der Funktion verändert werden, und der>> Aufrufer daher sichern müsste?>> Das sagt ihm die ABI.
Siehe
http://gcc.gnu.org/wiki/avr-gcc#Register_Layout
Der Abschnitt
1
R18–R27, R30, R31
2
These GPRs are call clobbered. An ordinary function may use them
3
without restoring the contents. Interrupt service routines (ISRs)
4
must save and restore each register they use.
Es gibt also Register, von denen explizit gesagt wird, dass eine
Funktion sie verändern darf, ohne dass sie selbst sich darum kümmern
muss sie zu sichern. D.h. Wenn ein Aufrufer selbst keines dieser
Register benutzt, dann muss weder er, noch die Funktion sich darum
kümmern, dass diese Register gepusht oder gepopt werden. Die Funktion
kann sie einfach benutzen. Erst der erste am Callstack zurückliegende
Aufrufer, der seinerseits eines dieser Register benutzt, muss selbst
dafür sorgen, dass sein Registerinhalt den Aufruf überlebt, wenn das für
ihn wichtig ist.
Bei einer ISR ist das eben ein Sonderfall. Ein Funktionsaufruf führt
automatisch dazu, dass alles gesichert werden muss. Gut, man hätte da
noch Logik einbauen können, dass die Push/Pop Orgie im Falle eines
einzigen Funktionsaufrufs zum Call wandert, wenn es keine anderen
Hindernisse gibt, das wurde aber anscheinend nicht gemacht.
In diesem Sinne sind Tricks auf C Ebene IMHO keine gute Idee. Was ich
hier dann tatsächlich tun würde: Den Compiler Output als Vorlage nehmen
und in Assembler neu schreiben, wobei dann die Register Orgie zum Aufruf
kommt.
Danke für die Erklärung, Karl Heinz!
so gesehen ist es doch besser die Funktion inlinen zu lassen, dann
sichert er mir nur mehr die wirklich benötigten Register.
immer noch nicht optimal, aber was solls, ist es halt so.
Michael Reinelt schrieb:> Wahrscheinlich hast du sogar recht, trotzdem mag ich es nicht wenn> unnötige Rechenzeit verbraten wird. Es ist einfach unschön (und genau> deshalb sind viele moderne .Net und C# Programme so schnarchlangsam,> weil "es ist doch egal, wir haben ja eh genug CPU power")
Ich geb dir Recht das es in diesem speziellen Fall unschön ist. Trotzdem
hat diese Vergabe der Verpflichtung wer welche Register zu sichern hat
seinen Sinn. Denn an anderen Stellen bringt das etwas, wenn die
Verpflichtung zum Push/Pop von der Funktion aufwärts bis zum
eigentlichen Nutzniesser verschoben wird. Wenn der oberste Aufrufer
(main()) ebenfalls keines dieser Register benutzt oder nur temporär
benutzt und es egal ist, ob ein Call dieses Register zerstört oder
nicht, dann ergibt sich daraus, dass niemand sich groß drum kümmern
muss, diese Register zu sichern oder wieder herzustellen und man hat
einen Satz temporärer Register, die jeder nach Lust und Laune einfach so
benutzen kann.
Gut, Ich habs in Assembler auch noch so gelernt, dass jede Funktion
dafür zuständig ist, ihre eigenen Register zu sichern und wieder
herzustellen. Dann kann per Definition nicht viel passieren, aber so rum
kann man dem ganzen ebenfalls seinen Sinn nicht absprechen. Allerdings
zahlt man in der ISR drauf.
Michael Reinelt schrieb:> so gesehen ist es doch besser die Funktion inlinen zu lassen, dann> sichert er mir nur mehr die wirklich benötigten Register.
So wie es aussieht, hast du nur die Wahl zwischen Pest und Cholera.
Michael Reinelt schrieb:> Danke für die Erklärung, Karl Heinz!
Ich danke dir.
Denn bisher wusste ich auch nur, dass ein Funktionsaufruf in einer ISR
zu einer Push/Pop Orgie führt, aber nicht warum.
Jetzt hatte ich Gelegenheit, das mal zu ergründen WARUM das so ist.
:-)
Johann L. schrieb:> Du kannst die Register um den Aufruf von RSM_Worker per Hand sichern.> ".irp x 18 19 20 21 22 23 24 25 26 27 30 31\n"
Phew... noch so viel zu lernen...
btw. das Zitat von Kernighan auf deiner "über mich" seite ist cool..
@Michael Reinelt (fisa)
>Wahrscheinlich hast du sogar recht, trotzdem mag ich es nicht wenn>unnötige Rechenzeit verbraten wird. Es ist einfach unschön (und genau>deshalb sind viele moderne .Net und C# Programme so schnarchlangsam,>weil "es ist doch egal, wir haben ja eh genug CPU power")
Im Prinzip hast du Recht, praktisch aber nicht so ganz. Nenn doch mal
konkrete Zahlen. Wieviele Takte würdest du sparen? Ich sag mal 10
push/pop, macht 40 Takte, die je 2ms "sinnlos" verbraten werden. macht
bei 10 MHz CPU-Takt 2 Promille Rechenleistung. Das kann selbst ein AVR
verkraften. C-Compiler sind nicht perfekt, sie müssen ein paar
Kompromisse machen. Wenn du WIRKLICH Takte sparen MUSST, schreib eine
reine ASM-ISR.
Falk Brunner schrieb:> Wenn du WIRKLICH Takte sparen MUSST, schreib eine reine ASM-ISR.
Wird jedenfalls übersichtlicher als der C / inline asm Kauderwelsch.
Falk Brunner schrieb:> Im Prinzip hast du Recht, praktisch aber nicht so ganz. Nenn doch mal> konkrete Zahlen. Wieviele Takte würdest du sparen? Ich sag mal 10> push/pop, macht 40 Takte, die je 2ms "sinnlos" verbraten werden. macht> bei 10 MHz CPU-Takt 2 Promille Rechenleistung. Das kann selbst ein AVR> verkraften. C-Compiler sind nicht perfekt, sie müssen ein paar> Kompromisse machen. Wenn du WIRKLICH Takte sparen MUSST, schreib eine> reine ASM-ISR.
Im Prinzip hast du auch recht.
Meine Herangehensweise ist folgende: ich werf prinzipiell einen Blick
auf den Assembler-Output (obwohl ich kein Assembler schreiben könnte),
sehe etwas was mir verdächtig vorkommt (die push/pop orgie), und frage
mich "muss das sein?"
Gäbe es eine einfache Lösung (a la -fno-push-pop-baccanal), hätt ich mit
minimalem Aufwand die 40 "sinnlosen" Takte gespart.
Nachdem das aber offensichtlich so sein muss, nehm ich es eben hin, weil
es eh nicht so kritisch ist.
Außerdem hat Karl Heinz was gelernt dadurch :-)
Michael Reinelt schrieb:> Da gibts nix zu überdenken: Ein Teil muss mit 500 Hz ausgeführt werden,> der andere mit 100 Hz. Und einen zweiten freien Timer hab ich nciht.
Wer hat denn gesagt, daß Du 2 Timer brauchst?
Michael Reinelt schrieb:> Gäbe es eine einfache Lösung (a la -fno-push-pop-baccanal), hätt ich mit> minimalem Aufwand die 40 "sinnlosen" Takte gespart.>> Nachdem das aber offensichtlich so sein muss, nehm ich es eben hin, weil> es eh nicht so kritisch ist.Müssen stimmt nicht ganz, immerhin fällt der Compiler nicht vom Himmel
sondern ist Menschenwerk. Wie immer bei freien Software-Projekten gilt
daher: patches welcome! Der Aufwand zur Implementierung ist jedoch
nicht minimal...
Peter Dannegger schrieb:> Michael Reinelt schrieb:>> Da gibts nix zu überdenken: Ein Teil muss mit 500 Hz ausgeführt werden,>> der andere mit 100 Hz. Und einen zweiten freien Timer hab ich nciht.>> Wer hat denn gesagt, daß Du 2 Timer brauchst?>
1
>ISR(TIMER1_COMPA_vect)
2
>{
3
>OCR1A+=TIME1;
4
>// ..
5
>}
6
>
7
>ISR(TIMER1_COMPB_vect)
8
>{
9
>OCR1B+=TIME2;
10
>// ..
11
>}
12
>
Manno, du machst mich schon wieder ganz fertig... Wieso komm ich nie auf
sowas?
D A N K E ! ! !
Johann L. schrieb:> Michael Reinelt schrieb:>>> Nachdem das aber offensichtlich so sein muss, nehm ich es eben hin, weil>> es eh nicht so kritisch ist.>> Müssen stimmt nicht ganz, immerhin fällt der Compiler nicht vom Himmel> sondern ist Menschenwerk. Wie immer bei freien Software-Projekten gilt> daher: patches welcome! Der Aufwand zur Implementierung ist jedoch> nicht minimal...
s/so sein muss/nicht ganz einfach ist/
Außerdem hat mich Peter grad wieder davon überzeugt, doch besser Gärtner
zu werden :-)
Peter Dannegger schrieb:> Man kann auch SW-Interrupts erstellen:
Huh? EEPROM und SPM? Da kann ich jetzt nicht ganz folgen, klingt aber
spannend... hättest du da einen RTFM-Pointer? Aus dem Datenblatt
erschließt sich mir das nicht wirklich...
Lies Dir einfach die Beschreibung zu EERIE bzw. SPMIE durch.
Geht natürlich nur, wenn die Applikation nicht auf EEPROM oder Flash
schreibt oder dann die 3,4ms Verzögerung nicht stören.
Peter Dannegger schrieb:> Lies Dir einfach die Beschreibung zu EERIE bzw. SPMIE durch.
Hmmm... heisst das man benutzt das EEPROM gar nicht, sondern fragt
trickreich "liebes EEPROM, wenn du bereit bist, mach mir bitte einen
Interrupt", und da das EEPROM eh nur sinnlos Nase bohrt, löst es gleich
einen Interrupt aus?
same for SPM?
Und wie kommt man auf sowas? Wenn man nach "AVR software interrupt"
googelt, liest man nur dass AVR das nicht können, oder nur über Umweg
eines PinChange-Interrupts auf einem Ausgangspin?
Muss ich mir in jedem Fall merken!
Johann L. schrieb:> Warum ist RSM_worker denn noinline? Erzeugt das so einen großen Overhead> falls es inline eingebaut wird?
Weil ich anfangs Hoffnung hatte, dass wenn nicht inline die push/pop
orgie nur beim tatsächlichen Aufruf der RSM_worker ausgeführt wird.
Dank Karl Heinz's Erklärung weiss ich aber dass das nix bringt, ganz im
gegenteil sobald man aus der ISR eine Funktion aufruft werden noch mehr
Register per push/pop gesichert.
Zum Einen wird das inlinen eh wieder erlaubt, zum Zweiten hat sich das
problem dank Peter's Hinweis (zwei getrennte OC-ISR) ohnehin in
Wohlgefallen aufgelöst...