Wie Assembler Code in eine ISR (C, AS7, GCC, ATmega)

Sorry, ich finde solchen Inline-Assemblecode grausig! Zumindest, wenn es 
mehr, als 1-2 Befehle sind. Besser eine "richtige" ASM-Datei in das 
Projekt einbinden.

Du musst die ISR als "naked" deklarieren, sonst haut dir der Compiler 
noch Prolog und Epilog dazu.

AC schrieb:
> Was bedeutet denn "garbage at end of line"?

Genau das, was da steht: Müll am Ende der Zeile. Irgendetwas nicht 
Druckbares hat sich da eingeschlichen.

Wofür denn das umständliche Assembler-Gewurschtel?

Das hier:

1

#include <avr/io.h>

2

#include <avr/interrupt.h>

3

4

uint16_t *Audio_sample_adress;

5

uint16_t sample_counter;

6

7

ISR(ADC_vect)

8

{

9

    *Audio_sample_adress = ADC;

10

    if (--sample_counter == 0)

11

        ADCSRA &= ~_BV(ADIE);  // no more interrupts

12

}

ergibt compiliert (für einen ATmega128 - ich sehe RAMPZ bei dir):

1

.global __vector_21

2

        .type   __vector_21, @function

3

__vector_21:

4

        push r1

5

        push r0

6

        in r0,__SREG__

7

        push r0

8

        clr __zero_reg__

9

        in r0,__RAMPZ__

10

        push r0

11

        push r24

12

        push r25

13

        push r30

14

        push r31

15

/* prologue: Signal */

16

/* frame size = 0 */

17

/* stack size = 8 */

18

.L__stack_usage = 8

19

        in r24,0x4

20

        in r25,0x4+1

21

        lds r30,Audio_sample_adress

22

        lds r31,Audio_sample_adress+1

23

        std Z+1,r25

24

        st Z,r24

25

        lds r24,sample_counter

26

        lds r25,sample_counter+1

27

        sbiw r24,1

28

        sts sample_counter+1,r25

29

        sts sample_counter,r24

30

        or r24,r25

31

        brne .L1

32

        in r24,0x6

33

        andi r24,lo8(8)

34

        out 0x6,r24

35

.L1:

36

/* epilogue start */

37

        pop r31

38

        pop r30

39

        pop r25

40

        pop r24

41

        pop r0

42

        out __RAMPZ__,r0

43

        pop r0

44

        out __SREG__,r0

45

        pop r0

46

        pop r1

47

        reti

48

        .size   __vector_21, .-__vector_21

49

        .comm   sample_counter,2,1

50

        .comm   Audio_sample_adress,2,1

51

        .ident  "GCC: (GNU) 5.4.0"

Ich seh' da jetzt nicht so große Unterschiede zu deinem handgefeilten
Assemblercode, dass sich der Zirkus irgendwie lohnen würde.

René H. schrieb:
> Diesen Rückschluss wirst einem Anfänger erklären müssen.

Controller mit maximal 64 KiB Flash haben kein RAMPZ. ;)

AC schrieb:
> Das heisst, ich würde mir den ganzen Assembler Code mit deiner ISR
> sparen?

Ja.

> Das Ganze ist zeitkritisch und alle ISR müssen in Assembler programmiert
> werden

Naja, wirklich klein ist das Original nun auch nicht gerade.

Zugegeben, der Compiler generiert drei oder vier Befehle mehr, aber
man müsste dann schon die Erbsen zählen gehen um zu sehen, ob die das
Kraut fett machen dass es sich lohnt, irgendwelchen Assemblercode
dafür handzufeilen.

> (Aussage von meinem Auftraggeber des Techniker Projektes)

Wann hat der das letzte Mal einen aktuellen Compiler und dessen
generierten Code gesehen?

Also wenn es vorgeschrieben ist, weil es zu einer studentischen Übung
gehört, dann musst du die ISR natürlich in Assemblercode hacken.  Dann
nimm aber besser gleich eine separate Assemblerdatei.  Wenn's nur
wegen der Performance ist, würde ich erstmal analysieren, ob nicht das
Compilat genauso hinreichend ist.  Dass der äquivalente C-Code besser
lesbar ist, wird wohl kaum einer bestreiten wollen. :-)

Ich zähle jetzt irgendwie reichlich 60 Takte für das Compilat, das
wären bei angenommenen 16 MHz CPU-Takt etwa 4 µs, sagen wir 5 µs (der
Interrupt hat ja auch noch einen Overhead).  Bei 10 kHz Aufruffrequenz
braucht diese ISR folglich 50 ms pro Sekunde oder 5 % der CPU-Zeit.

AC schrieb:

Btw:

> Habe global dafür 3 variablen:volatile uint16_t sample_counter = 0;
> volatile uint16_t audio_samples_buffer[1024];
> volatile uint16_t Audio_sample_adress = 0;

"volatile" würde ich die allerdings nicht machen, wenn die ISR in
C geschrieben wird, sonst zwingst du den Compiler zu deutlich
schlechterem Code.

Audio_sample_adress hat übrigens den falschen Typ; das sollte ein
Zeiger sein, kein Integer (so habe ich es in meinem Beispiel auch).

Ich vermute, dass "sample_counter" vom herumliegenden Programm zur
Synchronisation benutzt wird, oder?  Dann kann *) man diese Variable
natürlich "volatile" machen, dann muss in der ISR jedoch mit einer
Schattenkopie davon gearbeitet werden:

1

ISR(ADC_vect)

2

{

3

    *Audio_sample_adress = ADC;

4

    uint16_t sc = sample_counter;

5

    if (--sc == 0)

6

        ADCSRA &= ~_BV(ADIE);  // no more interrupts

7

    sample_counter = sc;

8

}

Das vermeidet die Pessimierung des Variablenzugriffs, die sonst
eintreten würde.  Denk aber dran, dass du außerhalb der ISR auf
diese Variable über die atomic-Makros (<util/atomic.h>) zugreifen
musst, ansonsten bekommst du subtile Fehler in dein Programm, wenn
der Interrupt genau in der Mitte des 16-bit-Zugriffs reinhaut.

*) Man kann auch den Speicherzugriff nur außerhalb erzwingen, aber
das geht jetzt ein bisschen zu weit.

Er hat aber noch mehr Fehler drin, Labels, die nicht definiert
sind, Zugriff auf SREG, was der Assembler so nicht kennt etc. pp.

War mir angesichts des geringen Nutzeffekts jetzt zu viel, mir das
im Detail anzusehen.

AC schrieb:

> Der Atmega läuft mit 20MHz.

Dann wird die Zeitbilanz noch 20 % besser als meine Rechnung.

> Könntest du mir deinen Code noch etwas
> erklären?

Naja, was ist denn daran so viel zu erklären?

Es ist eigentlich schlicht das Äquivalent deines Assemblercodes,
nicht weniger, aber auch nicht mehr.

> Ich muss die Werte ja in ein Array[1024] abspeichern, das müsste ich
> noch mit einer for Schleife erledigen?

Hat deine Assemblerversion doch auch nicht. ;-)

Eine for-Schleife kannst du nur innerhalb einer Funktion benutzen,
du musst hier jedoch das Weiterzählen über die einzelnen ISR-Aufrufe
hinweg machen.

Das Initialisieren von Audio_sample_adress und sample_counter muss
natürlich vor dem ersten Interrupt passieren; das ist daher weder
in meiner C-ISR noch in deinem geposteten Assemblercode enthalten.

> er ist nur der Meinung das dieser asm code zeit unkritisch ist und ich
> mit C Probleme haben werde.

Dann solltest du natürlich in der Arbeit bei der Diskussion des
Programms darauf eingehen, wie groß der vom Compiler erzeugte Code
tatsächlich ist und wie lange er braucht (also im Prinzip das, was ich
oben überschlagen habe, nur exakt nachgerechnet).

AC schrieb:
> Der Timer0 läuft im CTC im 10kHz. bzw. löst die ISR richtig aus.

Meine ISR ist übrigens für den ADC, nicht für den Timer.
Schließlich willst du ja ADC-Werte lesen, die solltest du dann lesen,
wenn der ADC auch fertig mit der Wandlung ist.

Deine gepostete ISR ist da irgendwie inkonsistent: sie gehört zum
Timer, aber wenn alle Wandlungen fertig sind, setzt sie (was ich
wieder korrekt finde) das ADIE im ADCSRA zurück.  Dennoch würde jedoch
der Timer fleißig weiter Interrupts triggern, folglich munter weiter
Audio_sample_adress erhöhen und an die entsprechende Stelle schreiben …

Wenn du den Timer als Trigger für den ADC benutzen willst, dann solltest
du dir im ADC das "auto trigger"-Feature ansehen und dort den Timer
als Triggerquelle nehmen, aber die Ergebnisse in der ADC-ISR lesen.

Mach's in Assembler. avr-gcc -x assembler-with-cpp ...

1

#define __SFR_OFFSET 0

2

#include <avr/io.h>

3

4

.text

5

6

.type   TIMER0_COMPA_vect, @function

7

.global TIMER0_COMPA_vect

8

9

TIMER0_COMPA_vect:

10

11

    push R27

12

    push R26

13

    push R21

14

    push R20

15

    in   R20, SREG

16

    push R20

17

    in   R20, RAMPZ

18

    push R20

19

20

    LDS R20, ADCL

21

    LDS R21, ADCH

22

23

    LDS R26, Audio_sample_adress

24

    LDS R27, Audio_sample_adress+1

25

    ST  X+, R20

26

    ST  X+, R21

27

    STS Audio_sample_adress, R26

28

    STS Audio_sample_adress+1, R27

29

30

    LDS R26, sample_counter

31

    LDS R27, sample_counter+1

32

    SBIW R26, 1

33

    STS sample_counter, R26

34

    STS sample_counter+1, R27

35

36

    BRNE jump_out

37

38

    LDS  R26, ADCSRA

39

    ANDI R26, 0b11110111

40

    STS  ADCSRA, R26

41

42

jump_out:

43

    POP R20

44

    OUT RAMPZ, R20

45

    POP R20

46

    OUT SREG, R20

47

    POP R20

48

    POP R21

49

    POP R26

50

    POP R27

51

52

    RETI

53

54

.size TIMER0_COMPA_vect, . - TIMER0_COMPA_vect

Kommentare sind Hausaufgabe.

...und PUSH / POP von RAMPZ sind überflüssig.

Blubber schrieb:
> AC schrieb:
>> Das Prorogramm soll mit Timer0 im CTC Modus mit einer Samplerate von
>> 10kHz werte ins Array schreiben.
>
> Dafür braucht man keine assembler isr :)

In BASCOM vermutlich schon :-P

AC schrieb:
> aber die ISR an sich macht mir kopfzerbrechen

Das verstehe ich nun wirklich nicht.  Was ist an drei Zeile C-Code,
die nur das Äquivalent deines länglichen Assemblerpamphlets sind,
nun so schwer zu verstehen?

\r\n schrieb:
> AC schrieb:
>> "push R27"  "\n\t"                 //X-Register High-Byte
>
> Ich würde erstmal hinterfragen wollen, was die albernen "\n\t" am Ende
> jeder Zeile sollen.

Das ist ein Zeilenumbruch.

Assemblerbefehle mussen duch einen solchen oder ein $ getrennt werden.

Jörg W. schrieb:
> Ich seh' da jetzt nicht so große Unterschiede zu deinem handgefeilten
> Assemblercode, dass sich der Zirkus irgendwie lohnen würde.

Der Code scheitert daran, folgendes Konstrukt nachzubilden

> "STS sample_counter,R26"   "\n\t" //Store back decremented Sample-Counter
> "LDS sample_counter+1,R27" "\n\t

SCNR

Johann L. schrieb:
> Der Code scheitert daran, folgendes Konstrukt nachzubilden

:-)))

AC schrieb:
> Johann L. schrieb:
>> Mach's in Assembler. avr-gcc -x assembler-with-cpp ...
>> #define __SFR_OFFSET 0
>> #include <avr/io.h>
>> ...
>
> Das habe ich auch mit einer externen .S Datei versucht, was aber auch
> nicht funktioniert

Ich habe lediglich deinen Code nach GNU-Assembler übertragen.  Lässt 
sich Problemlos für einen ATmega168 assemblieren.

Die Bugs, die aufs Original-Posting zurückgehen, musst schon selbst 
beheben...

Mir dünkt, dass der TE erstmal lernen sollte zu verstehen, was in der
ISR überhaupt passiert, statt die ganze Zeit nur irgendwelchen fremden
Code trial&error-mäßig durchzuprobieren.