AVR: Wie Cycle-Counted-ISR mit Inline-ASM für portablen Code optimiert wird

Ich versuche, meine RX- und TX-Interrupts so zu optimieren, dass sie die maximale Ausführungszeit von 25 Zyklen erreichen, während Interrupts deaktiviert sind.

Bisher habe ich festgestellt, dass der Code ausreichend optimiert ist, aber das Drücken und Knallen von Registerreihen zwischen dem Laden und Entladen __SREG__überschreitet das Zeitlimit.

 272:   80 91 24 01     lds r24, 0x0124
 276:   8f 5f           subi    r24, 0xFF   ; 255
 278:   8f 71           andi    r24, 0x1F   ; 31
 27a:   90 91 c6 00     lds r25, 0x00C6
 27e:   20 91 25 01     lds r18, 0x0125
 282:   28 17           cp  r18, r24
 284:   39 f0           breq    .+14        ; 0x294 <__vector_18+0x30>
 286:   e8 2f           mov r30, r24
 288:   f0 e0           ldi r31, 0x00   ; 0
 28a:   ea 5d           subi    r30, 0xDA   ; 218
 28c:   fe 4f           sbci    r31, 0xFE   ; 254
 28e:   90 83           st  Z, r25
 290:   80 93 24 01     sts 0x0124, r24

Der einzige Weg, um am sichersten Ort platzieren zu können __SREG__(so viele Stöße wie möglich in den bewusstlosen Bereich einbeziehen) war Inline-Asm.

Hier mein aktueller Code:

ISR(RX0_INTERRUPT, ISR_NAKED)
{
    //push
    asm volatile("push r31" ::); // table pointer
    asm volatile("push r30" ::); // table pointer
    asm volatile("push r25" ::); // received character
    asm volatile("push r18" ::); // once compared to r24 -> rx0_first_byte

    asm volatile("push r24" ::); // most stuff is executed in r24
    asm volatile("in r24,__SREG__" ::); // - 
    asm volatile("push r24" ::); // but one byte more on stack

    register uint8_t tmp_rx_last_byte = (rx0_last_byte + 1) & RX0_BUFFER_MASK;
    register uint8_t tmp = UDR0_REGISTER;

    if(rx0_first_byte != tmp_rx_last_byte)
    {
        rx0_buffer[tmp_rx_last_byte] = tmp;
        rx0_last_byte = tmp_rx_last_byte;
    }

    //pop
    asm volatile("pop r24" ::);
    asm volatile("out __SREG__,r24" ::);
    asm volatile("pop r24" ::);

    asm volatile("pop r18" ::);
    asm volatile("pop r25" ::);
    asm volatile("pop r30" ::);
    asm volatile("pop r31" ::);

    reti();
}

Wie Sie sehen können, gibt es einen fest codierten Register-Push, den mein Compiler verwendet hat, übrigens funktioniert er überhaupt, aber ich bin mir nicht sicher, wie portabel er ist.

Das einzige Register, das ich mit dem Spezifizierer "=r" erhalten kann, ist r24und es passiert sogar für mask und rx0_first_byte.

Wie kann ich also dem Compiler sagen, dass er diese 5 Register pushen/poppen soll, auch wenn sie woanders platziert werden?

r19Was ist die Möglichkeit, dass der Compiler and r26anstelle von r18and verwendet r25?

Ich möchte nicht die gesamte ISR in Assembler umschreiben.

EDIT: danke für alle Vorschläge, endlich habe ich ISR in asm umgeschrieben

    ISR(RX0_INTERRUPT, ISR_NAKED)
{
    asm volatile("\n\t"                      /* 5 ISR entry */
    "push  r31 \n\t"                         /* 2 */
    "push  r30 \n\t"                         /* 2 */
    "push  r25 \n\t"                         /* 2 */
    "push  r24 \n\t"                         /* 2 */
    "push  r18 \n\t"                         /* 2 */
    "in    r18, __SREG__ \n\t"               /* 1 */
    "push  r18 \n\t"                         /* 2 */

    /* read byte from UDR register */
    "lds   r25, %M[uart_data] \n\t"          /* 2 */

    /* load globals */
    "lds   r24, (rx0_last_byte) \n\t"        /* 2 */
    "lds   r18, (rx0_first_byte) \n\t"       /* 2 */

    /* add 1 & mask */
    "subi  r24, 0xFF \n\t" //???                  /* 1 */
    "andi  r24, %M[mask] \n\t"            /* 1 */

    /* if head == tail */
    "cp    r18, r24 \n\t"                    /* 1 */
    "breq  L_%= \n\t"                        /* 1/2 */

    "mov   r30, r24 \n\t"                    /* 1 */
    "ldi   r31, 0x00 \n\t"                   /* 1 */
    "subi  r30, lo8(-(rx0_buffer))\n\t"      /* 1 */
    "sbci  r31, hi8(-(rx0_buffer))\n\t"      /* 1 */
    "st    Z, r25 \n\t"                      /* 2 */
    "sts   (rx0_last_byte), r24 \n\t"        /* 2 */

"L_%=:\t"
    "pop   r18 \n\t"                         /* 2 */
    "out   __SREG__ , r18 \n\t"              /* 1 */
    "pop   r18 \n\t"                         /* 2 */
    "pop   r24 \n\t"                         /* 2 */
    "pop   r25 \n\t"                         /* 2 */
    "pop   r30 \n\t"                         /* 2 */
    "pop   r31 \n\t"                         /* 2 */
    "reti \n\t"                              /* 5 ISR return */

    : /* output operands */

    : /* input operands */
    [uart_data] "M"    (_SFR_MEM_ADDR(UDR0_REGISTER)),
    [mask]      "M"    (RX0_BUFFER_MASK)

    /* no clobbers */
    );

}

AKTUALISIEREN:

Nach einigen Tests habe ich festgestellt, dass die Interrupts vor dem Eintritt in den ISR-Handler deaktiviert sind, nicht nach dem Entladen, __SREG__wie ich zuvor vorgeschlagen habe.

Die einzige Möglichkeit besteht darin, Register wie von ndim vorgeschlagen zu globalisieren oder den folgenden Code zu verwenden:

ISR(RX0_INTERRUPT, ISR_NAKED)
    {
        asm volatile("\n\t"                      /* 4 ISR entry */

        "push  r0 \n\t"                          /* 2 */
        "in    r0, __SREG__ \n\t"                /* 1 */

        "push  r31 \n\t"                         /* 2 */
        "push  r30 \n\t"                         /* 2 */
        "push  r25 \n\t"                         /* 2 */
        "push  r24 \n\t"                         /* 2 */
        "push  r18 \n\t"                         /* 2 */

        /* read byte from UDR register */
        "lds   r25, %M[uart_data] \n\t"          /* 2 */

#ifdef USART_UNSAFE_RX_INTERRUPT // enable interrupt after satisfying UDR register
        "sei \n\t"                               /* 1 */
#endif
        /* load globals */
        "lds   r24, (rx0_last_byte) \n\t"        /* 2 */
        "lds   r18, (rx0_first_byte) \n\t"       /* 2 */

        /* tmp_rx_last_byte = (rx0_last_byte + 1) & RX0_BUFFER_MASK */
        "subi  r24, 0xFF \n\t"                   /* 1 */
        "andi  r24, %M[mask] \n\t"               /* 1 */

        /* if(rx0_first_byte != tmp_rx_last_byte) */
        "cp    r18, r24 \n\t"                    /* 1 */
        "breq  .+14 \n\t"                        /* 1/2 */

        /* rx0_buffer[tmp_rx_last_byte] = tmp */
        "mov   r30, r24 \n\t"                    /* 1 */
        "ldi   r31, 0x00 \n\t"                   /* 1 */
        "subi  r30, lo8(-(rx0_buffer))\n\t"      /* 1 */
        "sbci  r31, hi8(-(rx0_buffer))\n\t"      /* 1 */
        "st    Z, r25 \n\t"                      /* 2 */

        /* rx0_last_byte = tmp_rx_last_byte */
        "sts   (rx0_last_byte), r24 \n\t"        /* 2 */

#ifdef USART_UNSAFE_RX_INTERRUPT
        "cli \n\t"                               /* 1 */
#endif

        "pop   r18 \n\t"                         /* 2 */
        "pop   r24 \n\t"                         /* 2 */
        "pop   r25 \n\t"                         /* 2 */
        "pop   r30 \n\t"                         /* 2 */
        "pop   r31 \n\t"                         /* 2 */

        "out   __SREG__ , r0 \n\t"               /* 1 */
        "pop   r0 \n\t"                          /* 2 */

        "reti \n\t"                              /* 4 ISR return */

        : /* output operands */

        : /* input operands */
        [uart_data] "M"    (_SFR_MEM_ADDR(UDR0_REGISTER)),
        [mask]      "M"    (RX0_BUFFER_MASK)

        /* no clobbers */
        );

    }
Würde es zu Ihrem Gesamtprogramm passen, einfach ein Flag in der ISR zu setzen und die Verarbeitung außerhalb der ISR durchzuführen?
Dieser Interrupt liest nur ein Byte von uart RX und speichert es im Ringpuffer. Wichtig für mich ist, dass der Puffer nicht überläuft.
Ist Ihnen klar, dass Ihr Compiler beim nächsten Compilieren möglicherweise einen anderen Code mit unterschiedlichen Registern generiert?
Ich arbeite nicht an AVR, aber als allgemeine Faustregel gilt: Verlassen Sie sich niemals auf zufällige Platzierungen des Compilers, eine kleine Versionsänderung könnte das Verhalten ändern, eine kleine Codeänderung könnte es auch. Sie können auch Ihr Compiler-Handbuch sehr sorgfältig lesen, da sind echte Juwelen für kritische Dinge (Pragmas, Intrinsic) enthalten.
Ich vermute, dass ggc Variablen in verschiedene Register setzen würde, aber nach stundenlangem Googeln weiß ich immer noch nicht, wie man Pushes richtig macht.
Ich glaube nicht, dass Sie die Pushes manuell für einen vom Compiler generierten Code durchführen sollten. Sie müssen wissen, welche Register verwendet werden, was Sie im Allgemeinen nicht wissen können. Es sei denn natürlich, Sie drücken alle, die von der Aufrufkonvention gespeichert werden müssen.
Wissen Sie, dass der AVR UART einen Zwei-Byte-Puffer hat? Wenn Sie in Ihrer Interrupt-Routine bis zu zwei Bytes verarbeiten, können Sie möglicherweise Ihre 25-Zyklen-Beschränkung lockern?
Der Zwei-Byte-Puffer zerstört wahrscheinlich kein eingehendes Byte während der atomaren USB-Übertragung, nicht für den Hauptcode, der in einer langen Schleife hängen bleiben kann.
Wenn es nur 25 Zyklen lang ist, was ist falsch daran, das Ganze in Assembler-Code zu schreiben? Dann müssen Sie überhaupt nicht den Compiler-Konventionen folgen - speichern Sie einfach die von Ihnen verwendeten Register in der ISR und stellen Sie sie am Ende wieder her. Sie werden wahrscheinlich in der Lage sein, eine noch größere Gesamtoptimierung zu erreichen.
Befreien Sie sich von den Push/Pops und den registerSchlüsselwörtern und sehen Sie, ob es gelingt, effizienteren Code zu produzieren.
Standard-ISR (ohne ISR_NAKED) ist aufgrund des generierten Papierkorbcodes, der in keiner Weise geändert werden kann, nicht geeignet. Mein Code selbst ist 18 Zyklen lang, daher müssen Unterbrechungen in der Nähe des Codes deaktiviert und aktiviert werden, nicht nach dem ersten Drücken.
@jnk0le Der beste Weg ist, einen völlig separaten Asm-Code zu schreiben, der die Interrupts behandelt, und ihn mit Ihrem C-Code zu mischen. Nur so können Sie sicher sein, dass der Compiler nichts vermasselt. Als weitere Lektüre empfehle ich dringend dieses Dokument von Atmel: atmel.com/Images/doc42055.pdf

Antworten (2)

Wenn ich ein paar Pushs / ​​Pops spare, indem ich globale Registervariablen verwende und alle Anweisungen in einer asm()Anweisung zusammenfasse, würde ich so etwas wie erreichen

#define RB_WIDTH 5
#define RB_SIZE (1<<(RB_WIDTH))
#define RB_MASK ((RB_SIZE)-1)

register uint8_t rb_head      asm("r13");
register uint8_t rb_tail      asm("r14");
register uint8_t rb_sreg_save asm("r15");

volatile uint8_t rb_buf[RB_SIZE];

ISR(USART0_RX_vect, ISR_NAKED)                 /* CLOCK CYCLES */
{
  asm("\n\t"                                   /* 5 ISR entry */
      "push  r24\n\t"                          /* 2 */
      "push  r25\n\t"                          /* 2 */
      "push  r30\n\t"                          /* 2 */
      "push  r31\n\t"                          /* 2 */
      "in    %r[sreg_save], __SREG__\n\t"      /* 1 */
      "\n\t"

      /* read byte from UART */
      "lds   r25, %M[uart_data]\n\t"           /* 2 */

      /* next_tail := (cur_tail + 1) & MASK; */
      "ldi   r24, 1\n\t"                       /* 1 */
      "add   r24, %r[tail]\n\t"                /* 1 */
      "andi  r24, %a[mask]\n\t"                /* 1 */

      /* if next_tail == cur_head */
      "cp    r24, %r[head]\n\t"                /* 1 */
      "breq  L_%=\n\t"                         /* 1/2 */

      /* rb_buf[next_tail] := byte */
      "mov   r30, r24\n\t"                     /* 1 */
      "ldi   r31, 0\n\t"                       /* 1 */
      "subi  r30, lo8(-(rb_buf))\n\t"          /* 1 */
      "sbci  r31, hi8(-(rb_buf))\n\t"          /* 1 */
      "st    Z, r25\n\t"                       /* 2 */

      /* rb_tail := next_tail */
      "mov   %r[tail], r24\n\t"                /* 1 */

      "\n"
"L_%=:\t"
      "out   __SREG__, %r[sreg_save]\n\t"      /* 1 */
      "pop   r31\n\t"                          /* 2 */
      "pop   r30\n\t"                          /* 2 */
      "pop   r25\n\t"                          /* 2 */
      "pop   r24\n\t"                          /* 2 */
      "reti\n\t"                               /* 5 ISR return */
      : /* output operands */
        [tail]      "+r"   (rb_tail)    /* both input+output */
      : /* input operands */
        [uart_data] "M"    (_SFR_MEM_ADDR(UDR0)),
        [mask]      "M"    (RB_MASK),
        [head]      "r"    (rb_head),
        [sreg_save] "r"    (rb_sreg_save)
        /* no clobbers */
      );
}

Dies dauert immer noch 42 Zyklen.

Eine kleine Reorganisation des Ringpuffercodes könnte die Codemenge in der ISR, die in den Ringpuffer schreibt, sogar noch einfacher reduzieren (auf Kosten einer komplexeren Funktion, die aus dem Puffer liest).

Ich habe ein vollständiges Beispiel mit Build-System und Support-Strukturen unter https://github.com/ndim/avr-uart-example/ erstellt.

Sieht so aus, als hätte ich alle Inline-ASM-Anleitungen, insbesondere Operanden, völlig missverstanden. Ich werde versuchen, es zu schreiben, ohne globale Variablen in Registern auszurichten;)
Es sieht so aus, als würde gcc nicht initialisierte Variablen als r24.
Wie ich sehen kann, ist es am besten, den gesamten Code in Assembler zu schreiben.

Ein paar hilfreiche Dinge, die ich fand, als ich kurz und schnell eine AVR-ISR für https://github.com/ndim/freemcan/tree/master/firmware erstellte , waren:

  • Lassen Sie Ihr Build-System bei jedem Neuaufbau Assembler-Dumps Ihres generierten Codes generieren und beobachten Sie die Änderungen im generierten Code jedes Mal, wenn Sie die Quelle ändern. Es hilft wirklich zu sehen, was wirklich passiert. (Ich benutze avr-objdump -h -S firmware.elf > firmware.lss.)

  • Wenn Sie eine wirklich schnelle ISR benötigen, können Sie einige Zyklen für das Pushen/Knallen von Registern einsparen, indem Sie angeben, den avr-gccgesamten C-Code zu kompilieren, ohne einige Register zu verwenden (z. B. -ffixed-r13), und diese Register dann als globale Variablen in der ISR ohne Pushen/Knallen zu verwenden. Dies erspart Ihnen auch die zusätzlichen Zyklen für den Speicherzugriff. Die Head- und Tail-Zeiger für den Ringpuffer sind in Ihrem Fall Kandidaten.

  • Ich kann mich nicht auf Anhieb erinnern, ob die avr-gccgenerierte ISR immer alle Register drückt / knallt oder nur die, die sie tatsächlich verwendet. Wenn es mehr als unbedingt nötig drückt / knallt, müssen Sie die ISR möglicherweise doch in Assembly schreiben.

  • Sie können dann immer noch die generierten Anweisungen in Assemblersprache nehmen, sie in eine .SAssembler-Quelldatei einfügen und diese noch weiter von Hand optimieren.

In meinem Anwendungsfall stellte sich jedoch heraus, dass die ISR doch nicht so zeitkritisch war.

Übrigens würde ich die Inline-asm-Parameter verwenden, um gcc die Register auswählen zu lassen, anstatt sie fest zu codieren. Siehe http://www.nongnu.org/avr-libc/user-manual/inline_asm.html

GCC fügt Code zum Pushen r0und r1(+ Nullen) hinzu und deaktiviert Interrupts nach dem ersten Push. In der Zwischenzeit möchte ich zuerst Pushes ausführen und dann Interrupts deaktivieren, um in 25 Zyklen zu passen, während Interrupts deaktiviert sind. Ich dachte darüber nach, ISR in Assembler umzuschreiben, aber ich brauche viele avr-asm-Grundlagen. Inline-Asm ist noch schlimmer als normal - wie man zu globalen Variablen kommt und wie man verwendete Register durch Inline-Asm-Parameter übergibt (ich kann es bekommen r24und dieses Handbuch ist nicht hilfreich genug)
Helfen die Beispiele in github.com/ndim/freemcan/blob/master/firmware/table-element.h dabei, wie man von Inline-ASM zu Variablen gelangt?
Nun, wenn Sie eine andere ISR-Präambel und einen anderen Bereinigungscode als den von avr-gcc generierten haben möchten, müssen Sie entweder avr-gcc ändern oder (viel einfacher) selbst Assembler schreiben. Verwenden Sie den von avr-gcc generierten Assembler-Code als Grundlage und gehen Sie von dort aus weiter.