Inline-Assembler: was mache ich falsch?

[Maxim B. (max182)]

Guten Abend,
ich lerne Inline-Assembler.
Leider bekomme ich nicht ganz, was ich möchte. Könnten Sie sagen, was 
anders zu machen ist?

static inline unsigned int __attribute__((always_inline))
sum32 (unsigned long fre, unsigned long phase)
{
                unsigned int ausgabe;
    ("add %A0, %A1"    "\n\t"
    "adc %B0, %B1"    "\n\t"
    "adc %C0, %C1"    "\n\t"
    "adc %D0, %D1"
         
    : "=&d"(ausgabe)
    : "r"(fre), "r"(phase));
    return ausgabe;
}

volatile unsigned long phase[3] = {0};
volatile unsigned long freq[3] = {47244640,47244641,47244642 };

PORTA = sum32(freq[0], phase[0]);

Also, ich möchte im PORTA höchste byte von phase haben, ohne lange 
Verschieben zu machen (reine C macht viel zu viel, kopiert Register, 
setzt sie auf Null, was hier gar nicht notwendig ist)

Ich bekomme aber:

+00000063:   017C        MOVW      R14,R24        Copy register pair
+00000064:   018D        MOVW      R16,R26        Copy register pair
+00000065:   0F82        ADD       R24,R18        Add without carry
+00000066:   1F93        ADC       R25,R19        Add with carry
+00000067:   1FA4        ADC       R26,R20        Add with carry
+00000068:   1FB5        ADC       R27,R21        Add with carry
24:       PORTA = sum32(freq[0], phase[0]);
+00000069:   BB8B        OUT       0x1B,R24       Out to I/O location

Also, ich bekomme in PORTA kleinste Byte statt höchste.
Wie könnte ich das ändern, in Inline-Assembler?
Und wie wäre es möglich, Kopieren von Registerpaar am Anfang zu 
vermeiden? Das ist auch überflüssig hier.

Vielen Dank im voraus.

[Markus F. (mfro)]

irgendwie fehlt mir da ein

__asm__ __volatile__

vor der Klammer. Außerdem gibt's keine clobber list.

Frißt der Compiler das überhaupt?

[Johann L. (gjlayde)]

static inline  __attribute__((always_inline))
uint8_t sum32_byte3 (uint32_t fre, uint32_t phase)
{
    uint8_t ausgabe;
    __asm ("mov %0, %D1"
           : "=r"(ausgabe)
           : "r" (fre + phase));
    return ausgabe;
}

Die Addition brauch nicht in Inline Asm ausgeführt zu werden, und 
volatile ist es auch nicht.  Wenn ein >> 24 wirklich zu viel Overhead 
erzeugt, dann kommt man um das MOV oben schwerlich rum — es sei denn, 
man tut auch das OUT ins asm rein.

[Axel S. (a-za-z0-9)]

Ich muß unserem Hater zugute halten, daß er zumindest in einem Punkt 
recht hat: der TE weiß entweder nicht, was er will oder ist zumindest 
unfähig, es zu artikulieren. Und wenn ihm das schon Menschen gegenüber 
nicht gelingt, dann wird es ihm auch Computern gegenüber nicht gelingen. 
Ganz egal ob in C oder Assembler.

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

Axel S. schrieb:
> Und wenn ihm das schon Menschen gegenüber
> nicht gelingt, dann wird es ihm auch Computern gegenüber nicht gelingen.

Das Gegenteil davon allerdings eine ziemlich präzise Beschreibung des 
sprichwörtlichen Nerds. ;-)

[Johann L. (gjlayde)]

Also ich hab schon verstanden, was der TO will.

Und gleich ob man es verstanden hat oder nicht, ist kein Grund hier 
rumzukotzen.

[Mampf F. (mampf)]

Maxim B. schrieb:
> ich lerne Inline-Assembler.

Das ist der Fehler! Entweder Assembler und zum C-Code dazulinken oder 
nur C, aber kein Inline-Assembler-Murks ...

Hatten wir die Diskussion neulich nicht schon einmal? <4 Wochen?

Ah, genau da ist er ja ... Doch leicht älter als 4 Wochen, die Essenz 
ist die Gleiche:

Beitrag "Assembler - Fehler im Programm"

[Johann L. (gjlayde)]

Mampf F. schrieb:
> Entweder Assembler und zum C-Code dazulinken oder
> nur C, aber kein Inline-Assembler-Murks ...

Auch wenn oben Inline-Assembler nicht angebracht ist, ist deine 
Behauptung schlichtweg nicht zutreffend.  Die Folgerung wäre nämlich, 
dass Inline-Assembler grundsätzlich überflüssig ist, was nicht der Fall 
ist.

[Heinz V. (heinz_v)]

Inline Assembler in Altgriechisch?

[Maxim B. (max182)]

Mampf F. schrieb:
>> ich lerne Inline-Assembler.
>
> Das ist der Fehler! Entweder Assembler und zum C-Code dazulinken oder
> nur C, aber kein Inline-Assembler-Murks ...

Danke!
Ich komme inzwischen zu ähnlichen Gedanken.
Wahrscheinlich ist Inline-Assembler einfach zu kompliziert 
implementiert.
Das Problem habe ich mit Assembler in CodeVisionAVR gelöst. Mag sein, 
CodeVisionAVR kann C nicht so gut optimieren wie WinAVR, dafür aber viel 
einfacher mit Assembler und Flash.

Sicher gibt es kein Compiler für alles. Für einige Aufgaben ist WinAVR 
besser, anderes gelingt mit CodeVisionAVR einfacher...

[Johann L. (gjlayde)]

Maxim B. schrieb:
> Mag sein, CodeVisionAVR kann C nicht so gut optimieren wie WinAVR,
> dafür aber viel einfacher mit Assembler und Flash.

Lustig.

Der von einer 9 Jahre alten GCC Version (das neueste WinAVR ist GCC v4.3 
von 2008) erzeugte Code ist nicht guit genug — so dass Assemnler in 
Betracht gezogen wird.  Statt auf eine neuere GCC-Version umzusteigen 
(aktuell v6) die besseren Code erzeugt und Features wie __flash bringt, 
wird auf einen Compiler umgesattelt, der noch schlechteren Code 
generiert...

> Wahrscheinlich ist Inline-Assembler einfach zu kompliziert
> implementiert.

GCC Inline-Assembler bietet recht feinziselierte Möglichkeiten, wie sie 
bei der Systemprogrammierung benötigt und unerlässlich sind.

Wenn die Komplexität von Inline-Assembler nicht gebrauch wird, dann geht 
natürlich auch "normaler" Assembler.

[Maxim B. (max182)]

Johann L. schrieb:

> Der von einer 9 Jahre alten GCC Version (das neueste WinAVR ist GCC v4.3
> von 2008) erzeugte Code ist nicht guit genug — so dass Assemnler in
> Betracht gezogen wird.  Statt auf eine neuere GCC-Version umzusteigen
> (aktuell v6) die besseren Code erzeugt und Features wie __flash bringt,
> wird auf einen Compiler umgesattelt, der noch schlechteren Code
> generiert...
Ich benutze AVR Studio 4.18 und WinAVR 20100110. Die neuere Version mit 
__flash bringt wirklich etwas kleineren Code. Aber unbequem, immer in 
Projektoptionen zu gehen und dort diese "-gdwarf-2" zu setzen. Das lohnt 
sich selten.

>GCC Inline-Assembler bietet recht feinziselierte Möglichkeiten, wie sie
>bei der Systemprogrammierung benötigt und unerlässlich sind.
Leider habe ich bisher dafür keine ausführliche deutsche Anleitung 
gefunden.
In jedem Fall hat man in C entweder 1 Bytes, oder 2 Bytes, oder 4 Bytes 
oder 8 Bytes-Variablen. Aber nichts mit z.B. 3 Bytes. D.h. ab und zu 
braucht man Assembler-Funktionen sowieso. Oder man muß für Vergnügen, 
ohne Assembler zu programmieren, mit teurerem Mikrocontroller und 
höherem Stromverbrauch bezahlen.

In der Anleitung für CodeVisionAVR sind einfach die Register genannt, 
die für Ausgabe benutzt werden und auch die für freie Verwendung 
freigegeben sind: r0,r1,r22,r23,r24,r25,r26,r27,r30,r31, dabei in r30, 
r31, r22 und r23 für return-Ausgabe vorgesehen sind. Einfach und 
verständlich.

CodeVisionAVR hat m.E. zwei wesentliche Nachteile:
1. das ist ein kommerzielles Programm, nicht frei wie GCC.
2. double ist wie float, nur 4 Bytes.

So denke ich, jedem das Seine. Für einige Sachen ist GCC besser, für 
anderes passt CodeVisionAVR gut.

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

Maxim B. schrieb:
> In jedem Fall hat man in C entweder 1 Bytes, oder 2 Bytes, oder 4 Bytes
> oder 8 Bytes-Variablen. Aber nichts mit z.B. 3 Bytes. D.h. ab und zu
> braucht man Assembler-Funktionen sowieso.

Oder man verwendet GCC und nimmt "__int24" bzw. "__uint24" (AVR).

[Maxim B. (max182)]

A. K. schrieb:

>
> Oder man verwendet GCC und nimmt "__int24" bzw. "__uint24" (AVR).

Kann man wirklich so machen?
Ich habe immer gedacht, typedef kann nur schon sowieso vorhandene Typen 
neu benennen, aber keine neuen von Null aus bilden.

Soll das bedeuten, daß ich einfach __uint800 schreibe und gleich 
automatisch korrekt Code für 100-Bytes-Typ bekomme?

[MaWin (Gast)]

Maxim B. schrieb:
> Ich habe immer gedacht, typedef kann nur schon sowieso vorhandene Typen
> neu benennen, aber keine neuen von Null aus bilden.

int24 ist eine GCC-AVR8-Extension.

[MaWin (Gast)]

Maxim B. schrieb:
> Soll das bedeuten, daß ich einfach __uint800 schreibe und gleich
> automatisch korrekt Code für 100-Bytes-Typ bekomme?

nein

[Maxim B. (max182)]

Ich habe gleich eine Probe gemacht:
volatile __uint24 phase = 0;
volatile __uint24 freq = 47001642;
AVR Studio sagt: Fehler.

[Carl D. (jcw2)]

Maxim B. schrieb:
> Ich habe gleich eine Probe gemacht:
> volatile __uint24 phase = 0;
> volatile __uint24 freq = 47001642;
> AVR Studio sagt: Fehler.

Und welche Compiler-Version hat dem AVR-Studio das übersetzen 
abgenommen?

Zu 4.7.2 hat der AVR-GCC __(u)int24 dazubekommen.

[Maxim B. (max182)]

Danke!
Ich habe nun mit Toolchain 3.5.0 ausprobiert: das muß ziemlich neue 
Version sein, nicht wahr?

Ja, das hat geklappt.
Aber...
Alte Variante: WinAVR20100110 und 32bit Variablen:
mit Qs dauert 695 .
Neue Variante: Toolchain 3.5.0 und 24bit Variablen:
mit Qs dauert 750, mit Q2 ist Dauer 765.

Die Frage: wozu dann 24bit Variablen, wenn dadurch nur langsamer wird? 
Ich weiß nicht, warum. Disassembler zeigt dort deutlich mehr 
unnützlichen Code, als frühere Version...
Ist neue Compiler wirklich besser geworden?

Wirklich viel zu viel.
Ein Beispiel:

  phase[channel] += freq[channel];
  
  buf_phase = (unsigned int)(phase[channel]>>8);

343:        phase[channel] += freq[channel];
+00000B47:   01F2        MOVW      R30,R4         Copy register pair
+00000B48:   8120        LDD       R18,Z+0        Load indirect with displacement
+00000B49:   8131        LDD       R19,Z+1        Load indirect with displacement
+00000B4A:   8142        LDD       R20,Z+2        Load indirect with displacement
+00000B4B:   90600116    LDS       R6,0x0116      Load direct from data space
+00000B4D:   90700117    LDS       R7,0x0117      Load direct from data space
+00000B4F:   90800118    LDS       R8,0x0118      Load direct from data space
+00000B51:   0D26        ADD       R18,R6         Add without carry
+00000B52:   1D37        ADC       R19,R7         Add with carry
+00000B53:   1D48        ADC       R20,R8         Add with carry
+00000B54:   93200116    STS       0x0116,R18     Store direct to data space
+00000B56:   93300117    STS       0x0117,R19     Store direct to data space
+00000B58:   93400118    STS       0x0118,R20     Store direct to data space
347:        buf_phase = (unsigned int)(phase[channel]>>8);
+00000B5A:   90600116    LDS       R6,0x0116      Load direct from data space
+00000B5C:   90700117    LDS       R7,0x0117      Load direct from data space
+00000B5E:   90800118    LDS       R8,0x0118      Load direct from data space
+00000B60:   2C67        MOV       R6,R7          Copy register
+00000B61:   2C78        MOV       R7,R8          Copy register
+00000B62:   2488        CLR       R8             Clear Register

Hier sind die Zeilen +00000B5A: bis +00000B5E: überflüssig, Compiler hat 
das nicht erkannt.

So könnte ich ungefähr 1/3 der Zeit sparen (mindestens), wenn ich die 
zeitkritische Sachen mit Assembler mache und anderes auf C.

Und das ist, nachdem ich die Funktion als inline static deklariert habe 
(was die Zeit bis 590 Cycles reduzierte), früher war Code noch 
schlimmer.

[Carl D. (jcw2)]

Leider fehlt im Code die Definition der beiden Variablen. Diese sind 
nicht zufällig mit "volatile" gespickt?

[Maxim B. (max182)]

Ja, volatile. Das ist leider notwendig (da in Funktion).
Aber in Assembler würde ich nach

STS       0x0116,R18
STS       0x0117,R19
STS       0x0118,R20

für das Weitere entweder gleich r18-r20 benutzen, oder, falls diese 
Register anders gebraucht werden, schreiben:

mov r6,r18
mov r7,r19
mov r8,r20

statt

LDS       R6,0x0116
LDS       R7,0x0117
LDS       R8,0x0118

oder sogar, mit Rücksicht auf Folgendes gleich

mov r6,r19
mov r7,r20

und somit 4 Takte mit lds und clr gespart.

[Nico W. (nico_w)]

Maxim B. schrieb:
> Die neuere Version mit __flash bringt wirklich etwas kleineren Code.
> Aber unbequem, immer in Projektoptionen zu gehen und dort diese
> "-gdwarf-2" zu setzen.

Was hat den das Flag zum debuggen mit __flash zu tun?

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

Maxim B. schrieb:
> Aber in Assembler würde ich nach
> STS       0x0116,R18
> STS       0x0117,R19
> STS       0x0118,R20
>
> für das Weitere entweder gleich r18-r20 benutzen,

Das ist dem Compiler bei "volatile" verboten.

Es ist etwas unfair, dem Compiler per "volatile" zu sagen, dass er 
Zugriffe keinesfalls wegoptimieren darf, und ihm anschliessend 
vorzuwerfen, dass er Zugriffe nicht wegoptimiert.

[Carl D. (jcw2)]

Maxim B. schrieb:
> Ja, volatile. Das ist leider notwendig (da in Funktion).

"in Funktion" ??

volatile braucht man, damit man in der Hauptschleife Änderungen an einer 
Variable sehen kann, die eine ISR macht.
Ich vermute, hier wird ein DDS-Generator in einer Timer-ISR benutzt. 
Variablen, die ausschließlich die ISR benutzt, wie den Phase-Akku, oder 
Varablen, die von der ISR ausschließlich gelesen werden, wie z.B. 
Phase-Inkrement, brauchen kein volatile. Bei letzteren muß nur 
sichergestellt werden, daß sie "atomar" von der Hauptschleife 
manipuliert werden. Mit denen rechnet man in der Hauptschleife nicht 
rum, sondern setzt sie irgendwann auf den berechneten (neuen) Wert und 
sperrt für diese Zuweisung die Interrupts. Wenn man vorher noch per 
(idle) sleep einen TimerInterrupt abgewartet hat, dann geht das 
womöglich sogar Jitter-frei(/-arm).
Compiler sind heute sehr gut darin, Code zu optimieren. Aber nur 
basierend auf den Anforderungen (Source-Code). Alles, was dort nur vage 
definiert ist, führt aber dazu, daß der Compier vom Worst-Case ausgehen 
muß. "volatile" schmeissen, weil man mit Multithreading (nichts anderes 
sind ISRs) nicht zurecht kommt, ist kein Problem des Compilers.

Und wenn der AVR wirklich zu langsam ist, für 2,50 liefert der Chinese 
einen "STM32-Arduino", der rechnet gerne mit 32Bit.

[Maxim B. (max182)]

Carl D. schrieb:
> Maxim B. schrieb:
>> Ja, volatile. Das ist leider notwendig (da in Funktion).
>
> "in Funktion" ??
>
> volatile braucht man, damit man in der Hauptschleife Änderungen an einer
> Variable sehen kann, die eine ISR macht.
> Ich vermute, hier wird ein DDS-Generator in einer Timer-ISR benutzt.
> Variablen, die ausschließlich die ISR benutzt, wie den Phase-Akku, oder
> Varablen, die von der ISR ausschließlich gelesen werden, wie z.B.
> Phase-Inkrement, brauchen kein volatile.
Danke!
Ich versuche, Programm anders zu gestalten.
Ich möchte es versuchen, zu erreichen, daß ein ATmega zwei Stimmen 
bedient. Für nur eine Stimme reicht die Zeit, für zwei noch nicht.

[Bernd K. (prof7bit)]

Carl D. schrieb:
> volatile braucht man, damit man in der Hauptschleife Änderungen an einer
> Variable sehen kann, die eine ISR macht.

Ich hab schon erlebt daß der gcc beim Optimieren eine kurze Sequenz 
inline asm einfach komplett entfernt hat, wahrscheinlich weil er 
irgendwie zum Schluss gekommen ist das ganze Konstrukt hätte keine 
Seiteneffekte und wäre somit überflüssig. Volatile hat hier geholfen dem 
Compiler das etwas eindringlicher klar zu machen.

[Maxim B. (max182)]

Na, ich kam bei 2 Stimmen auf 517 Takte, und ich fürchte, das ist die 
Grenze für GCC. Weitere Ersparnis (ohne vollständig auf Assembler 
umzusteigen, was ich auch für möglich halte), wie ich fürchte, kann nur 
auf Kosten von repetierenden Register passieren (was ich nicht möchte).

#define PHASE_MODUL 240UL
#define SESQ_REP 12 // Rep. Punkt Sesquialtera, c0
#define SCHARF_REP0 12
#define SCHARF_REP1 24
#define SCHARF_REP2 36
#define SCHARF_REP3 48
#define ZIMBEL_REP0 12
#define ZIMBEL_REP1 18
#define ZIMBEL_REP2 24
#define ZIMBEL_REP3 30
#define ZIMBEL_REP4 36
#define ZIMBEL_REP5 42
#define ZIMBEL_REP6 48
#define MCP4922_GA 5
#define MCP4922_SH 4
#define KAN 7
#define BUF 6

#define SPI_DDR DDRB
#define SPI_PORT PORTB
#define SPI_SS PB0
#define SPI_MOSI PB3
#define SPI_SCK PB5
 __uint24 phase[2] = {0};
volatile __uint24 freq[2] = {184549,207150}; // Frequenz, Probewert
volatile unsigned char amp[] = {150,201}; // Lautstaerke, Probewert
volatile unsigned char note[] = {25,33}; // Tastennummer, Probewert
volatile unsigned char status[] = {1,1}; /* Kanalstatus , Probewert 0=aus,1=klang,2=ausklang */

/* Probewerte */
volatile unsigned char ein_gedackt16 = 1; 
volatile unsigned char ein_ged_pr8 = 1;    // 0=aus, 1=ged, 2=pr
volatile unsigned char ein_fl_okt4 = 1;   // 0=aus, 1=fl,2=okt
volatile unsigned char ein_fl_okt2 = 1;   // 0=aus, 1=fl, 2=okt

volatile unsigned char ein_quinte = 1; 
volatile unsigned char ein_terzfloete = 1;
volatile unsigned char ein_sesquialtera = 1;
volatile unsigned char ein_scharf = 1;
volatile unsigned char ein_zimbel = 1;

void spi_init(void)
{
  SPI_DDR |= (1<<SPI_SS) | (1<<SPI_MOSI) | (1<<SPI_SCK);
  SPCR = (1<<SPE) | (1<<MSTR);
  SPSR = (1<<SPI2X);
}

inline static void channel(unsigned char channel){
unsigned char temp; // fuer MSB von Phase
unsigned char tempton; // fuer Byte von Klang aus Flash
unsigned int summ = 0; // fuer Summe von allen sig., Buf. fuer DAC
unsigned int buf_phase_16; // fuer Berechnung von Obertoenen
unsigned int buf_phase_8; // fuer Berechnung von Obertoenen
unsigned int buf_phase_4; // fuer Berechnung von Obertoenen
unsigned int buf_phase_2; // fuer Berechnung von Obertoenen
unsigned int buf_phase_qu; // fuer Berechnung von Obertoenen
unsigned int buf_phase_terz; // fuer Berechnung von Obertoenen

unsigned char buf_16;
unsigned char buf_8;
unsigned char buf_4;
unsigned char buf_2;
unsigned char buf_qu;
unsigned char buf_terz;
unsigned char notebuf = note[channel];

    /* Phase summieren */

  phase[channel] += freq[channel];
  if(phase[channel]>=PHASE_MODUL << 16){
    phase[channel] -= (PHASE_MODUL << 16);    
  }


  buf_phase_16 = (unsigned int)(phase[channel]>>8);
  buf_phase_8 = buf_phase_16 + buf_phase_16;
  buf_phase_4 = buf_phase_8 + buf_phase_8;
  buf_phase_2 = buf_phase_4 + buf_phase_4;
  buf_phase_qu = buf_phase_8 + buf_phase_4;
  buf_phase_terz = buf_phase_2 + buf_phase_8;

  buf_16 = (unsigned char)(buf_phase_16>>8);
  buf_8 = (unsigned char)(buf_phase_8>>8);
  buf_4 = (unsigned char)(buf_phase_4>>8);
  buf_2 = (unsigned char)(buf_phase_2>>8);
  buf_qu = (unsigned char)(buf_phase_qu>>8);
  buf_terz = (unsigned char)(buf_phase_terz>>8); 


/* 16' 1Harm */
if(ein_gedackt16){
  
  tempton = gedackt16[buf_16];
  summ += tempton;
}

/* 8' 2 Harm */

if(ein_ged_pr8 == 1){

  tempton = gedackt8[buf_8];
  summ += tempton;
}
if(ein_ged_pr8 == 2){

  tempton = principal8[buf_8];
  summ += tempton;
  }

/* 4' 4 Harm */

if(ein_fl_okt4 == 1){

  tempton = floete4[buf_4];
  summ += tempton;
}
if(ein_fl_okt4 == 2){

  tempton = oktave4[buf_4];
  summ += tempton;
}

/* 2 2/3' 6 Harm */

if(ein_quinte){

  tempton = qufloete[buf_qu];
  summ += tempton;
}

/* 2' 8 Harm */

if(ein_fl_okt2 == 1){

  tempton = floete2[buf_2];
  summ += tempton;
}
if(ein_fl_okt2 == 2){

  tempton = oktave2[buf_2];
  summ += tempton;
}

/* 1 3/5' 10 Harm */

if(ein_terzfloete){

  tempton = terzfloete[buf_terz];
  summ += tempton;
}

/* Sesquialtera, 1-mal rep. C-c 8' 16' */

if(ein_sesquialtera){
  if(notebuf<SESQ_REP) {
  
    tempton = sesquialtera0[buf_4];
    }
    else {
    
      tempton = sesquialtera1[buf_8];
    }
  summ += tempton;
}

/* Scharf, 4-mal rep. c0,c1,c2  4' 8' 8' 16' */
if(ein_scharf){
  
  if(notebuf<SCHARF_REP0) {
  
   // 4'
  
    tempton = scharf0[buf_4];
  }
  else if(notebuf<SCHARF_REP1) {
    
    tempton = scharf1[buf_8];
  }
  else if(notebuf<SCHARF_REP2)
  {
    
    tempton = scharf2[buf_8];
  }
  else {
  
    tempton = scharf3[buf_16];
  }
  summ += tempton;
}

/* Zimbel 8-mal rep. c0,fs0,c1,fs1,c2,fs2,c3 2',4',4',8',8',16',16',16 */
if(ein_zimbel){

  if(notebuf<ZIMBEL_REP0) {
    
    
     // 2'
    
    tempton = zimbel0[buf_2];
  }
  else if(notebuf<ZIMBEL_REP1) {
    
     // 4'
    
    tempton = zimbel1[buf_4];
    }
  else if(notebuf<ZIMBEL_REP2) {
    
     // 4'
    
    tempton = zimbel2[buf_4];
    }
  else if(notebuf<ZIMBEL_REP3) {
     // 8'
    
    tempton = zimbel3[buf_8];
    }
  else if(notebuf<ZIMBEL_REP4) {
     // 8'
    
    tempton = zimbel4[buf_8];
    }
  else if(notebuf<ZIMBEL_REP5) {
    
    tempton = zimbel5[buf_16];
    }
  else if(notebuf<ZIMBEL_REP6) {
    
    tempton = zimbel6[buf_16];
    }
  else {
    tempton = zimbel7[buf_16];
    }
  summ += tempton;

  summ *= amp[channel];
}

/* Senden summ in DAC */

  temp = (unsigned char)(summ>>8);
  temp &= 0x0f;
  temp |= (1<<MCP4922_SH)|(1<<MCP4922_GA)|(1<<BUF);
  if(channel) temp |= (1<<KAN);
  SPI_PORT &= ~(1<<SPI_SS);
  SPDR = temp;
  while(!(SPSR & (1<<SPIF)));
  temp = (unsigned char)(summ);
  SPDR = temp;
  while(!(SPSR & (1<<SPIF)));
  SPI_PORT |= (1<<SPI_SS);

}



int main(){
DDRD = 0xff;
DDRB = 0x7f;
spi_init();
SPDR = 0xff; //dummi

while(1){


if((status[0]==1)||(status[0]==2)) {
  channel(0);
}

if((status[1]==1)||(status[1]==2)) {
  channel(1);
}

}
return 0;
}

Also, unmöglich, zwei Stimmen mit einem ATmega zu schaffen.

[Carl D. (jcw2)]

Bernd K. schrieb:
> Carl D. schrieb:
>> volatile braucht man, damit man in der Hauptschleife Änderungen an einer
>> Variable sehen kann, die eine ISR macht.
>
> Ich hab schon erlebt daß der gcc beim Optimieren eine kurze Sequenz
> inline asm einfach komplett entfernt hat, wahrscheinlich weil er
> irgendwie zum Schluss gekommen ist das ganze Konstrukt hätte keine
> Seiteneffekte und wäre somit überflüssig. Volatile hat hier geholfen dem
> Compiler das etwas eindringlicher klar zu machen.

"volatile asm" ist etwas anderes als eine volatile Variable.
Bei ersterem sagt man dem Compiler "Finger weg von meinem ASM Code",
bei letzterem "diese Variable kann ihren Inhalt jederzeit ohne 
erkennbaren Grund ändern".
Z.B. ein Hardwareregister oder eben eine (globale) Variable, die in 
einer ISR geändert wird. Beide Fälle erlauben kein Zwischenspeichern des 
Wertes in einem Register, denn der aktuelle Programmfluß läst keinen 
Rückschluss auf den aktuellen Wert zu und damit ob der "gepufferte Wert' 
noch aktuell ist.


@Maxim B.:
Ich sehe da nirgends eine Timer-Isr, d.h. das ganze läuft 
"singlethreaded" und damit ist volatile für die Variablen überflüssig.
Allerdings frage ich mich, wie dann der Takt für die DDS zustande kommt. 
Der geht nämlich direkt in die Tonhöhe ein und unterschiedlich lang 
laufende Code-Paths (es gibt ja einige if's) sorgen für "jaulen".

[Johann L. (gjlayde)]

Maxim B. schrieb:
> volatile __uint24 freq[2] = {184549,207150};
> volatile unsigned char amp[] = {150,201};
> volatile unsigned char note[] = {25,33};
> volatile unsigned char status[] = {1,1};
> volatile unsigned char ein_gedackt16 = 1;
> volatile unsigned char ein_ged_pr8 = 1;
> volatile unsigned char ein_fl_okt4 = 1;
> volatile unsigned char ein_fl_okt2 = 1;
>
> volatile unsigned char ein_quinte = 1;
> volatile unsigned char ein_terzfloete = 1;
> volatile unsigned char ein_sesquialtera = 1;
> volatile unsigned char ein_scharf = 1;
> volatile unsigned char ein_zimbel = 1;

Kein einziges dieser "volatile" ist notwendig da die Anwendung weder 
Interrupts noch ISRs hat.

Wenn du grundlos volatile mit der Grießkanne verteilst, dann hat ein 
optimierender Compiler keine Chance, auch kein CodeVisionAVR — es sei 
der ist nicht volatile-korrekt.

Selbtst mit #include <avr/io.h> lässt sich der Code nich übersetzen:

foo.c: In function 'channel':
foo.c:96:13: error: 'gedackt16' undeclared (first use in this function)
foo.c:96:13: note: each undeclared identifier is reported only once for each function it appears in
foo.c:104:13: error: 'gedackt8' undeclared (first use in this function)
foo.c:109:13: error: 'principal8' undeclared (first use in this function)
foo.c:117:13: error: 'floete4' undeclared (first use in this function)
foo.c:122:13: error: 'oktave4' undeclared (first use in this function)
foo.c:130:13: error: 'qufloete' undeclared (first use in this function)
foo.c:138:13: error: 'floete2' undeclared (first use in this function)
foo.c:143:13: error: 'oktave2' undeclared (first use in this function)
foo.c:151:13: error: 'terzfloete' undeclared (first use in this function)
foo.c:160:15: error: 'sesquialtera0' undeclared (first use in this function)
foo.c:164:17: error: 'sesquialtera1' undeclared (first use in this function)
foo.c:176:15: error: 'scharf0' undeclared (first use in this function)
foo.c:180:15: error: 'scharf1' undeclared (first use in this function)
foo.c:185:15: error: 'scharf2' undeclared (first use in this function)
foo.c:189:15: error: 'scharf3' undeclared (first use in this function)
foo.c:202:15: error: 'zimbel0' undeclared (first use in this function)
foo.c:208:15: error: 'zimbel1' undeclared (first use in this function)
foo.c:214:15: error: 'zimbel2' undeclared (first use in this function)
foo.c:219:15: error: 'zimbel3' undeclared (first use in this function)
foo.c:224:15: error: 'zimbel4' undeclared (first use in this function)
foo.c:228:15: error: 'zimbel5' undeclared (first use in this function)
foo.c:232:15: error: 'zimbel6' undeclared (first use in this function)
foo.c:235:15: error: 'zimbel7' undeclared (first use in this function)
foo.c:67:15: warning: variable 'buf_terz' set but not used [-Wunused-but-set-variable]
foo.c:66:15: warning: variable 'buf_qu' set but not used [-Wunused-but-set-variable]
foo.c:65:15: warning: variable 'buf_2' set but not used [-Wunused-but-set-variable]
foo.c:64:15: warning: variable 'buf_4' set but not used [-Wunused-but-set-variable]
foo.c:63:15: warning: variable 'buf_8' set but not used [-Wunused-but-set-variable]
foo.c:62:15: warning: variable 'buf_16' set but not used [-Wunused-but-set-variable]

[Johann L. (gjlayde)]

Ich bin den Code mal durchgegangen und hab die fehlenden Deklarationen 
geraten; vermutlich Tabellen für Hüllkurven im Falsh.  Anmerkungen unten
 

#include <avr/io.h>

#define XX extern const __flash uint8_t

XX gedackt16[];
XX gedackt8[];
XX principal8[];
XX floete4[];
XX oktave4[];
XX qufloete[];
XX floete2[];
XX oktave2[];
XX terzfloete[];
XX sesquialtera0[];
XX sesquialtera1[];
XX scharf0[];
XX scharf1[];
XX scharf2[];
XX scharf3[];
XX zimbel0[];
XX zimbel1[];
XX zimbel2[];
XX zimbel3[];
XX zimbel4[];
XX zimbel5[];
XX zimbel6[];
XX zimbel7[];

#define PHASE_MODUL 240UL
#define SESQ_REP 12 // Rep. Punkt Sesquialtera, c0
#define SCHARF_REP0 12
#define SCHARF_REP1 24
#define SCHARF_REP2 36
#define SCHARF_REP3 48
#define ZIMBEL_REP0 12
#define ZIMBEL_REP1 18
#define ZIMBEL_REP2 24
#define ZIMBEL_REP3 30
#define ZIMBEL_REP4 36
#define ZIMBEL_REP5 42
#define ZIMBEL_REP6 48
#define MCP4922_GA 5
#define MCP4922_SH 4
#define KAN 7
#define BUF 6

#define SPI_DDR DDRB
#define SPI_PORT PORTB
#define SPI_SS PB0
#define SPI_MOSI PB3
#define SPI_SCK PB5

//__uint24 phase[2] = {0};
//volatile __uint24 freq[2] = {184549,207150}; // Frequenz, Probewert
//volatile unsigned char amp[] = {150,201}; // Lautstaerke, Probewert
//volatile unsigned char note[] = {25,33}; // Tastennummer, Probewert
//volatile unsigned char status[] = {1,1}; /* Kanalstatus , Probewert 0=aus,1=klang,2=ausklang */

typedef struct
{
    // Phase, Frequenz
    __uint24 phase, freq;

    // Lautstaerke, Tastennummer, Kanalstatus
    uint8_t amp, note, status;
} channel_t;

channel_t channel[2] =
{
    { 0, 184549, 150, 25, 1 },
    { 0, 207150, 201, 33, 1 }
};

#define volatile

/* Probewerte */
volatile uint8_t ein_gedackt16 = 1; 
volatile uint8_t ein_ged_pr8 = 1;    // 0=aus, 1=ged, 2=pr
volatile uint8_t ein_fl_okt4 = 1;   // 0=aus, 1=fl,2=okt
volatile uint8_t ein_fl_okt2 = 1;   // 0=aus, 1=fl, 2=okt

volatile uint8_t ein_quinte = 1; 
volatile uint8_t ein_terzfloete = 1;
volatile uint8_t ein_sesquialtera = 1;
volatile uint8_t ein_scharf = 1;
volatile uint8_t ein_zimbel = 1;

#undef volatile

static void spi_init (void)
{
    SPI_DDR |= (1 << SPI_SS) | (1 << SPI_MOSI) | (1 << SPI_SCK);
    SPCR = (1 << SPE) | (1 << MSTR);
    SPSR = (1 << SPI2X);
}

static inline __attribute__((always_inline))
void do_channel (channel_t *ch)
{
    asm volatile ("" ::: "memory");

    if (ch->status == 0)
        return;

    uint16_t summ = 0; // fuer Summe von allen sig., Buf. fuer DAC

    /* Phase summieren */

    ch->phase += ch->freq;
    if (ch->phase >= PHASE_MODUL << 16)
    {
        ch->phase -= PHASE_MODUL << 16;
    }

    // fuer Berechnung von Obertoenen
    uint16_t buf_phase_16 = (unsigned int) (ch->phase >> 8);
    uint16_t buf_phase_8 = buf_phase_16 + buf_phase_16;
    uint16_t buf_phase_4 = buf_phase_8 + buf_phase_8;
    uint16_t buf_phase_2 = buf_phase_4 + buf_phase_4;
    uint16_t buf_phase_qu = buf_phase_8 + buf_phase_4;
    uint16_t buf_phase_terz = buf_phase_2 + buf_phase_8;

    uint8_t notebuf = ch->note;
    uint8_t buf_16 = (uint8_t) (buf_phase_16 >> 8);
    uint8_t buf_8 = (uint8_t) (buf_phase_8 >> 8);
    uint8_t buf_4 = (uint8_t) (buf_phase_4 >> 8);
    uint8_t buf_2 = (uint8_t) (buf_phase_2 >> 8);
    uint8_t buf_qu = (uint8_t) (buf_phase_qu >> 8);
    uint8_t buf_terz = (uint8_t) (buf_phase_terz >> 8); 


    /* 16' 1Harm */
    if (ein_gedackt16)
        summ += gedackt16[buf_16];

    /* 8' 2 Harm */

    if (ein_ged_pr8 == 1)
        summ += gedackt8[buf_8];
    
    if (ein_ged_pr8 == 2)
        summ += principal8[buf_8];

    /* 4' 4 Harm */

    if (ein_fl_okt4 == 1)
        summ += floete4[buf_4];

    if (ein_fl_okt4 == 2)
        summ += oktave4[buf_4];

    /* 2 2/3' 6 Harm */

    if (ein_quinte)
        summ += qufloete[buf_qu];

    /* 2' 8 Harm */

    if (ein_fl_okt2 == 1)
        summ += floete2[buf_2];

    if (ein_fl_okt2 == 2)
        summ += oktave2[buf_2];

    /* 1 3/5' 10 Harm */

    if (ein_terzfloete)
        summ += terzfloete[buf_terz];

    /* Sesquialtera, 1-mal rep. C-c 8' 16' */

    if (ein_sesquialtera)
    {
        if (notebuf < SESQ_REP)
            summ += sesquialtera0[buf_4];
        else
            summ += sesquialtera1[buf_8];
    }

    /* Scharf, 4-mal rep. c0,c1,c2  4' 8' 8' 16' */
    if (ein_scharf)
    {
        if (notebuf < SCHARF_REP0)
        {
            // 4'
            summ += scharf0[buf_4];
        }
        else if (notebuf < SCHARF_REP1)
        {
            summ += scharf1[buf_8];
        }
        else if (notebuf < SCHARF_REP2)
        {
            summ += scharf2[buf_8];
        }
        else
        {
            summ += scharf3[buf_16];
        }
    }

    /* Zimbel 8-mal rep. c0,fs0,c1,fs1,c2,fs2,c3 2',4',4',8',8',16',16',16 */
    if (ein_zimbel)
    {
        if (notebuf < ZIMBEL_REP0)
        {
            // 2'
            summ += zimbel0[buf_2];
        }
        else if (notebuf < ZIMBEL_REP1)
        {
            // 4'
            summ += zimbel1[buf_4];
        }
        else if (notebuf < ZIMBEL_REP2)
        {
            // 4'
            summ += zimbel2[buf_4];
        }
        else if (notebuf < ZIMBEL_REP3)
        {
            // 8'
            summ += zimbel3[buf_8];
        }
        else if (notebuf < ZIMBEL_REP4)
        {
            // 8'
            summ += zimbel4[buf_8];
        }
        else if (notebuf < ZIMBEL_REP5)
        {
            summ += zimbel5[buf_16];
        }
        else if (notebuf < ZIMBEL_REP6)
        {
            summ += zimbel6[buf_16];
        }
        else
        {
            summ += zimbel7[buf_16];
        }

        summ *= ch->amp;
    }

    /* Senden summ in DAC */

    uint8_t spdr = (uint8_t) (summ >> 8);
    spdr &= 0x0f;
    spdr |= (1 << MCP4922_SH) | (1 << MCP4922_GA) | (1 << BUF);
    if (ch == &channel[1])
        spdr |= (1 << KAN);
    SPI_PORT &= ~(1 << SPI_SS);
    SPDR = spdr;
    while (!(SPSR & (1 << SPIF)))
        ;

    spdr = (uint8_t) summ;
    SPDR = spdr;
    while (!(SPSR & (1 << SPIF)))
        ;
    SPI_PORT |= (1 << SPI_SS);
}

__attribute__((OS_main))
int main (void)
{
    DDRD = 0xff;
    DDRB = 0x7f;
    spi_init();
    SPDR = 0xff; //dummi

    while (1)
    {
        do_channel (&channel[0]);
        do_channel (&channel[1]);
    }
    return 0;
}

 
Mit avr-gcc-6 -O2 übersetzt sieht der erzeugte Code ganz vernünftig aus. 
Hie und da ließe sich ein Befehl sparen, aber auch mit 
(Inline-)Assembler wird das nur marginal besser und nicht den erhofften 
Fortschritt bringen.

Die ein_xx Variablen werden im Programm nicht verändert, dher kann ein 
COmpiler deren Werte vor der while-Schleife laden und darf annehmen, 
dass sie sich in die Schleife nicht mehr ändern.  Vermultich willst du 
irgendwann die Werte im Betrieb ändern, z.B. per Tastatur.  Daher hab 
ich ein memory-Clobber an den Anfang von do_channel gesetzt; ein 
volatile würde hier etwas schlechteren Code machen, da manche Werte wie 
ein_fl_okt2 nur einmal pro Durchlauf gelesen werden müssen.  Das 
memory-Clobber geht wohl auch 1x in der while-Schleife:
 

    while (1)
    {
        asm volatile ("" ::: "memory");
        do_channel (&channel[0]);
        do_channel (&channel[1]);
    }

 
Da ziemlich viel gelesen, entschieden und "gerechnet" wird, lässt sich 
nicht mehr viel abkürzen am Code, und dass CodeVisionAVR krass besser 
sein kann bleibt zu bewerten.  Die einzige Stelle, wo evtl. merklich 
Zeit eingespart werden kann, ist die Ausgabe nach SPI:  Dies geschieht 
nach dem Schema
 

   wert-berechnen
   wert-an-SPI-ausgeben
   warten-auf-SPI

 
Mit folgendem Schema kann mehr parallelisiert werden, weil während des 
Wartens bereits neue Werte berechnet werden können:
 

   wert-berechnen + SPI-nudelt-vor-sich-hin
   warten-auf-SPI
   wert-an-SPI-ausgeben

 
Die Anwendung von amp scheint an der falschen Stelle zu erfogen, sollte 
wohl aus dem if raus?

[Johann L. (gjlayde)]

Mit einem kleinen Tweak wird der Code etwas besser

https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/config/avr/avr.md?r1=245206&r2=245205&pathrev=245206

Auf den gesamten Code spart das 2%.  Das Pattern passt 16x, und 
insgesamt belegt der o.g. Code ca 1.5KiB.

Wirklich viel ist an dem Code auch nicht mehr zu optimieren, das meiste 
lässt sich wie gesagt durch Umstellen des Codes erreichen, so dass auch 
während Warten auf SPI Code ausgeführt wird anstatt die Zeit zu 
verplempern.