avr-gcc 4.3.3: ISR und Inhalt von R1 bei Eintritt in ISR

[Werner B. (werner-b)]

Beim Durchsehen des erzeugten Listing aus dem Beispielcode für den 
AtXmega128A1
(Appnote AVR1300) bin ich auf die folgende Codesequenz gestossen.

ISR(ADCA_CH3_vect)
{
 244:  1f 92         push  r1
 246:  0f 92         push  r0
 248:  0f b6         in  r0, 0x3f  ; 63
 24a:  0f 92         push  r0
 24c:  08 b6         in  r0, 0x38  ; 56
 24e:  0f 92         push  r0
 250:  18 be         out  0x38, r1  ; 56
 252:  09 b6         in  r0, 0x39  ; 57
 254:  0f 92         push  r0
 256:  19 be         out  0x39, r1  ; 57
 258:  0b b6         in  r0, 0x3b  ; 59
 25a:  0f 92         push  r0
 25c:  1b be         out  0x3b, r1  ; 59
 25e:  11 24         eor  r1, r1
 260:  0f 93         push  r16
  ...

R1 kann vom Vordergrundprogramm jeden beliebigen, zufälligen Wert 
erhalten, Was landet dann in den IO Register 0x38, 0x39 und 0x3B ?

Das verwendete Makefile als Anhang.

[Stefan B. (stefan)]

r1 wird möglicherweise beim Xmega genauso behandelt wie bei den 
normalen AVRs

"Falls das Register r1 verändert wird – was z.B. geschieht, wenn man 
Multiplikationsbefehle verwendet – dann muss am Ende des Templates das 
Register wieder auf 0 gesetzt werden, denn bei avr-gcc enthält dieses 
Register immer den Wert 0. Das Register in die Clobber-Liste aufzunehen 
bleibt wirkungslos. Hat man es zerstört, dann schreibt man ans Ende des 
Templates ein clr _zero_reg_ und stellt es dadurch wieder her."
http://www.rn-wissen.de/index.php/Inline-Assembler_in_avr-gcc

Das würde in deinem Listing allerdings bedeuten, dass man eigenen 
Inline-Assembler mit Nutzung von r1 atomar machen muss, d.h. nicht durch 
Interrupts unterbrechbar.

[Peter D. (peda)]

Werner B. schrieb:
> R1 kann vom Vordergrundprogramm jeden beliebigen, zufälligen Wert
> erhalten, Was landet dann in den IO Register 0x38, 0x39 und 0x3B ?

Mumpitz, sieht also nach nem Compilerbug aus.
Welche GCC-Version its das denn?

R1 ist zwar fast immer 0, aber wenn der Interrupt nach ner 
Multiplikation mit 16Bit Ergebnis zuschlägt, krachts.


Peter

[Jörg W. (dl8dtl) (Moderator)]

Ich glaube, das ist ein Bug.  Hier mal der Code, wie er vom
Codegenerator selbst kommt:

.global __vector_74
        .type   __vector_74, @function
__vector_74:
        push __zero_reg__
        push r0
        in r0,__SREG__
        push r0
        in r0,56-0
        push r0
        out 56-0,__zero_reg__
        clr __zero_reg__
/* prologue: Signal */
/* frame size = 0 */
/* epilogue start */
        pop r0
        out 56-0,r0
        pop r0
        out __SREG__,r0
        pop r0
        pop __zero_reg__
        reti

Die ISR kann sich nicht darauf verlassen, dass _zero_reg_ in der
Tat eine 0 enthält, insofern löscht sie das Register ordentlich,
bevor der eigentliche Code der ISR kommen würde -- aber zuvor
benutzt sie es bereits, um RAMPD zu löschen. :-/

Bitte schreib einen GCC-Bugreport dafür.

(Nicht-Xmegas sind davon nicht betroffen, da sie kein RAMPD-Register
besitzen.)

[Jörg W. (dl8dtl) (Moderator)]

Jörg Wunsch schrieb:

> Bitte schreib einen GCC-Bugreport dafür.

Bitte setz eric.weddington -at- atmel.com gleich mit auf die Cc-Liste
des Bugreports.

[Peter D. (peda)]

Wäre eigentlich ein schöner Zeitpunkt, endlich mal die Spezialregister 
R0,R1 von ihrer Doppelfunktion zu entbinden und R2,R3 als SREG-Kopie und 
Zero zu nehmen.
Würde pro Interrupt 7 Befehle und 13 Zyklen sparen.


Peter

[(prx) A. K. (prx)]

Änderungen am ABI sind eine Höllenmaschine. Jeder Assembler-Code muss 
dann mindestens kontrolliert und ggf. umgeschrieben werden, egal ob 
innerhalb GCC oder separater Code.

Ein Register wie R2 global als Platz für die SREG-Kopie von 
Interrupt-Handlern zu reservieren würde in sehr subtiler und schwer 
identifizierbarer Form jene bestehenden Programme abschiessen, die 
verschachtelte Interrupts verwenden. Was bei AVRs zwar nicht die Regel 
ist, aber zulässig.

[Dback (Gast)]

Ein klarer Fall von, "Das hab ich schon immer so gemacht, so bleibt es 
auch ? - Was neues kommt nicht in Frage" :-))

Tja...muss man halt durch..
Gibts eigentlich das A20-Gate noch ?

[(prx) A. K. (prx)]

Dback schrieb:

> Ein klarer Fall von, "Das hab ich schon immer so gemacht, so bleibt es
> auch ? - Was neues kommt nicht in Frage" :-))

Solche Registerkonventionen sind eine Grundsatzentscheidung, die in der 
Anfangsphase einer GCC Portierung getroffen wird. Sie im voll 
entwickelten produktiven Produkt zu renovieren braucht mehr Anlass als 
ein paar gewonnene Takte im Interrupt-Handler.

[Dback (Gast)]

Ist ja auch ok so :-) Die Komptibilität wiegt schwerer.
Das A20-Gate ist übrigens gestern 28 Jahre alt geworden :-)

[Peter D. (peda)]

A. K. schrieb:
> Änderungen am ABI sind eine Höllenmaschine.

Mit dem R1 gabs ja schon öfters Ärger und es wird auch nicht das letzte 
mal sein.
Man könnte das Zero-Register auch ganz weglassen, der Nutzen ist nur 
sehr gering.


> Ein Register wie R2 global als Platz für die SREG-Kopie von
> Interrupt-Handlern zu reservieren würde in sehr subtiler und schwer
> identifizierbarer Form jene bestehenden Programme abschiessen

Dafür gibts doch ISR_BLOCK, ISR_NOBLOCK, damit könnte man dann das 
zusätzliche Push/Pop steuern.


Peter

[Werner B. (werner-b)]

Das Problem besteht offenbar nur beim U*nix Compiler von 
Bingo600@avrfreaks.
Ich habe das noch einmal mit WinAVR 20100110 Übersetzt. Da tritt der 
Fehler nicht auf.

 244:  1f 92         push  r1
 246:  0f 92         push  r0
 248:  0f b6         in  r0, 0x3f  ; 63
 24a:  0f 92         push  r0
 24c:  08 b6         in  r0, 0x38  ; 56
 24e:  0f 92         push  r0
 250:  09 b6         in  r0, 0x39  ; 57
 252:  0f 92         push  r0
 254:  0b b6         in  r0, 0x3b  ; 59
 256:  0f 92         push  r0
 258:  11 24         eor  r1, r1
 25a:  18 be         out  0x38, r1  ; 56
 25c:  19 be         out  0x39, r1  ; 57
 25e:  1b be         out  0x3b, r1  ; 59
...

Ich werde den Bug-Report also nur an Bingo600 schicken ;-)

[Jörg W. (dl8dtl) (Moderator)]

Werner B. schrieb:
> Das Problem besteht offenbar nur beim U*nix Compiler von
> Bingo600@avrfreaks.

Das würde bedeuten, dass mir wohl ein Patch fehlt.  Schade auch,
leider hat Eric seine WinAVR-Patches noch nicht ins CVS bei
sourceforge.net gestellt.  Damit kann ich nicht vergleichen, was
da fehlt.

[Jörg W. (dl8dtl) (Moderator)]

Jörg Wunsch schrieb:
> Das würde bedeuten, dass mir wohl ein Patch fehlt.

Ich habe die Patches jetzt da.  Werde ich mir demnächst mal angucken,
was mir da durch die Lappen gegangen ist.

[Werner B. (werner-b)]

Ich hab's gefunden (nicht den Patch, aber was der tun muss).

In gcc/config/avr/avr.c die Zeilen 810-816 ...

      /* Clear zero reg.  */
      insn = emit_move_insn (zero_reg_rtx, const0_rtx);
      RTX_FRAME_RELATED_P (insn) = 1;

      /* Prevent any attempt to delete the setting of ZERO_REG!  */
      emit_insn (gen_rtx_USE (VOIDmode, zero_reg_rtx));

.. nach Zeile 741 (vor /* Push RAMPD, RAMPX, RAMPY. */) verschieben.