Forum: Compiler & IDEs GCC Compiler Optimierungen


von Toralf W. (willi)


Lesenswert?

Guten Morgen,

fange gerade an mich mit C auf dem AVR zu beschäftigen. Folgendes 
Problem habe ich mit dem Code im Anhang. Es wird die Variable 
dcc_daten[] in der ISR und in main verwendet. Sobald ich den GCC den 
Code optimieren lasse, ist der Inhalt von dcc_daten[] nur noch in der 
ISR selbst korrekt. In main lese ich nur noch "Müll" aus der Variablen. 
Was mache ich falsch?
1
#ifndef F_CPU
2
#define F_CPU 8000000
3
#endif
4
#define BAUD 19200
5
6
#include <avr/io.h>
7
#include <stdint.h>
8
//#include <util/delay.h>
9
#include <util/setbaud.h>
10
#include <avr/interrupt.h>
11
12
#define L_FIFO 32                                   // größe des fifo tx buffer
13
volatile uint8_t fifo[(L_FIFO+3)];                  // fifo (+3)
14
15
//zum DCC einlesen
16
#define DCCport PIND
17
#define DCCpin PIND4
18
19
static volatile uint8_t Lowpegel = 1;
20
static volatile uint8_t Highpegel = 1;
21
static volatile uint8_t EmpPos;
22
#define Preamble 0
23
static volatile uint8_t Bitzaehler = 0;
24
static volatile uint8_t dcc_in = 0;
25
static volatile uint8_t dcc_xor = 0;
26
static volatile uint8_t dcc_daten[6];
27
28
static volatile struct {
29
    unsigned neuerBefehl:1;                         // 1 Bit für neuerBefehl
30
    unsigned keinDCC:1;                             // 1 Bit für keinDCC
31
    unsigned bit:1;                                 // 1 Bit für bit
32
    } Flag;
33
34
//#######################################
35
//sende Byte über UART-FIFO
36
uint8_t UART_FIFO( uint8_t data )
37
//#######################################
38
{
39
    if( UCSRA & ((1<<UDRE)|(1<<TXC))){                  // werden keine Daten übertragen und ist UDR bereit für neue Daten
40
        TCCR1A |= (1<<COM1A1);                          // Träger einschalten (OCR Pin verbinden)
41
        UDR = data;
42
        return(1);                                      // i.o. daten werden gesendet
43
    }
44
    else{                                               // es werden schon Daten übertragen -> neue Daten in FIFO
45
        uint8_t fifo_anzahl = fifo[0];
46
        if (fifo_anzahl == (L_FIFO-1)){                 // ist fifo voll
47
            return(0);                                  // Fehler fifo ist voll
48
        }
49
        else{
50
            fifo_anzahl++;
51
            fifo[0] = fifo_anzahl;
52
            uint8_t fifo_wp = fifo[2];
53
            fifo[(fifo_wp+3)] = data;
54
            fifo_wp++;
55
            if(fifo_wp == L_FIFO){
56
                fifo_wp = 0;
57
            }
58
            fifo[2] = fifo_wp;
59
            return(1);
60
        }
61
    }
62
63
}
64
65
//####################################################
66
//int0 wird bei steigender Flanke ausgelöst
67
ISR(INT0_vect)
68
//####################################################
69
{
70
        TCCR1A &= ~(1<<COM1A1);                         // Träger ausschalten (OCR Pin trennen)
71
}
72
73
//####################################################
74
//int1 wird bei fallender Flanke ausgelöst
75
ISR(INT1_vect)
76
//####################################################
77
{
78
        TCCR1A |= (1<<COM1A1);                          // Träger einschalten (OCR Pin verbinden)
79
}
80
         
81
//####################################################
82
//TXC Interrupt wird ausgelöst bei TX fertig
83
//kommt erst bei TX fertig, nicht schon bei UDR 
84
//bereit für neue Daten 
85
ISR(USART_TX_vect)
86
//####################################################
87
{
88
    uint8_t fifo_anzahl = fifo[0];
89
    if (fifo_anzahl == 0){                              // fifo ist leer
90
        TCCR1A &= ~(1<<COM1A1);                         // Träger ausschalten (OCR Pin trennen)
91
    }
92
    else{
93
        fifo_anzahl--;
94
        fifo[0] = fifo_anzahl;
95
        uint8_t fifo_rp = fifo[1];
96
        uint8_t data = fifo[(fifo_rp+3)];
97
        UDR = data;
98
        fifo_rp++;
99
        if(fifo_rp == L_FIFO){
100
            fifo_rp = 0;
101
        }
102
        fifo[1] = fifo_rp;
103
    }
104
}
105
106
//####################################################
107
//Timer 0 CTC CompA
108
//wird alle 10us ausgelöst liest bit vom DCCpin ein
109
//
110
ISR(TIMER0_COMPA_vect)
111
//####################################################
112
{
113
    if ( DCCport & (1<<DCCpin) ) {                      // ist DCCpin High
114
        Highpegel++;                                    // inc Highpegel
115
        if (Highpegel == 0){                            // wenn Überlauf
116
            Flag.keinDCC = 1;                           // kein DCC setzen
117
            Highpegel = 1;
118
            Lowpegel = 12;
119
            return;                                     // und fertig
120
        }
121
    }
122
    else{
123
        Lowpegel++;                                     // wenn DCCpin Low inc Lowpegel
124
        if (Lowpegel == 0){
125
            Flag.keinDCC = 1;                           // bei Überlauf wieder fertig
126
            Highpegel = 1;
127
            Lowpegel = 12;
128
            return;
129
        }
130
    }
131
    if (Lowpegel == 4) Highpegel = 1;                   // beim 3.mal (3+1) Lowpegel Highpegelreset machen 
132
    if (Highpegel != 4) return;                         // wenn nicht 3x (3+1) Highpegel war fertig
133
    if (Lowpegel < 8) Flag.bit = 1;                     // wenn Lowpegel kleiner 8 neues bit high
134
    else Flag.bit = 0;                                  // sonst neues bit low
135
    Lowpegel = 1;                                       // Reset Lowpegel
136
    Highpegel++;                                        // 1x erhöhen, damit nicht noch ein Bit erkannt wird
137
138
    if (EmpPos == Preamble){                            // Preamble                                
139
        if (Flag.bit == 1) Bitzaehler++;                // empf.Bit high -> erhöhe Bitzähler
140
        else {                                          // empf.Bit low
141
            if (Flag.neuerBefehl == 1) {                // wenn letzter Befehl noch nicht bearbeitet neuen verwerfen
142
                Bitzaehler = 0;
143
                return;
144
            }
145
            if (Bitzaehler >= 10) EmpPos++;             // und mindestens 10 high empfangen -> schalte auf Byte A lesen um
146
            Bitzaehler = 0;                             // und lösche Bitzähler wieder
147
            dcc_xor = 0;          
148
        }
149
        return;
150
    }
151
    else {                                              // DCC Daten einlesen
152
        if (Bitzaehler < 8){                            // von 0-7 bits empfangen
153
            dcc_in += dcc_in;                           // neues bit einschieben
154
            if( Flag.bit == 1 ) dcc_in++;
155
            Bitzaehler++;                               // bitzähler auf nächstes bit
156
        }
157
        else {                                          // wenn 8 bits empfangen
158
            dcc_xor ^= dcc_in;                          // xor zur Fehlererkennung
159
            dcc_daten[(EmpPos-1)] = dcc_in;
160
            
161
            if (Flag.bit == 0){                         // kam 0 -> Trennbit
162
                EmpPos++;                               // auf nächstes Byte umschalten
163
                Bitzaehler = 0;                         // und bitzähler reset
164
            }
165
            else {                                      // kam 1 -> Endbit
166
                if ((EmpPos >= 2) && (dcc_xor == 0)){   // nachsehen ob mindest 3 Byte empfangen wurden und xor = 0 ist 
167
                    Flag.neuerBefehl = 1;               // wenn ja flag für neue Daten setzten
168
                    Flag.keinDCC = 0;                   // flag kein dcc löschen
169
                }
170
                EmpPos = Preamble;                      // DCC Schleife auf start
171
                Bitzaehler = 0;
172
            }
173
        }
174
    } 
175
    return;
176
}
177
                         
178
//##################################################################################################
179
//Hauptprogramm
180
int main (void) 
181
//##################################################################################################
182
{
183
//PORTS Initalisierung
184
    DDRA = 0b00000000;                          // 1=Ausgang/0=Eingang
185
    DDRB = 0b00001000;
186
    DDRD = 0b00000010;
187
188
    PORTA = 0;                                  // alle Ausgänge auf Low
189
    PORTB = 0;
190
    PORTD = 0;
191
192
//UART Initalisierung
193
    UBRRH = UBRRH_VALUE;                        // UBR_Wert aus setbaud.h 
194
    UBRRL = UBRRL_VALUE;
195
    #if USE_2X                                  // U2X-Modus erforderlich
196
        UCSRA |= (1 << U2X);
197
    #else                                       // U2X-Modus nicht erforderlich
198
        UCSRA &= ~(1 << U2X);
199
    #endif
200
    UCSRB = (1<<TXCIE)|(1<<TXEN);               // UART TX + TX fertig int einschalten
201
    UCSRC = (1<<UCSZ1)|(1<<UCSZ0);              // Asynchron 8N1
202
    
203
//init_fifo
204
    fifo[0] = 0;
205
    fifo[1] = 0;
206
    fifo[2] = 0;
207
208
//Timer1 init erzeugt den IR Träger PWM PhaseCorrect mit clk/1 top = ICR1
209
    TCCR1A = (1<<WGM11);                        // Clear OC1A on Compare Match (Set output to low level)
210
    TCCR1B = (1<<WGM13)|(1<<CS10);              // TCCR1A (1<<COM1A1) wird später gesetzt
211
    TCCR1C = 0;
212
213
    ICR1H = 0;                                  // Timer max auf 8,8 (8MHz/455KHz)/2
214
    ICR1L = 9;                                  // 455KHz = 2,2µs  8MHZ = 0,125µs *2 PhaseCorrect = 0,25µs/Timerschritt -> 8,8
215
216
    OCR1AH = 0;                                 // Timer Pulsbreite auf (0,2-1,1 us) kurzer Puls mit max Strom
217
    OCR1AL = 1;                                 // 1* 0,25
218
219
//Ext Int init
220
    MCUCR = (1<<ISC11)|(1<<ISC01)|(1<<ISC00);   // steigende Flanke INT0 + fallende Flanke INT1 erzeugt Interrupt
221
    GIMSK = (1<<INT1)|(1<<INT0);                // INT0+INT1 Interrupt enable
222
223
//Timer0 init alle 10us ein CTC interrupt
224
    TCCR0A = (1<<WGM01);                        // CTC OCR0A = Top
225
    TCCR0B = (1<<CS00);                         // clk/(No prescaling)
226
    TCNT0 = 0;
227
    OCR0A = 80;
228
    OCR0B = 0;
229
    TIMSK |= (1<<OCIE0A);                       // Timer/Counter0 Output Compare Match A Interrupt Enable
230
    TIFR |= (1<<OCF0A);                         // Flag löschen
231
                                       
232
    sei();
233
234
    UART_FIFO('S');
235
    UART_FIFO('t');
236
    UART_FIFO('a');
237
    UART_FIFO('r');
238
    UART_FIFO('t');
239
240
241
242
    while(1) {                                  // Hauptschleife
243
        
244
        if (Flag.neuerBefehl == 1){
245
            UART_FIFO(dcc_daten[0]);
246
            UART_FIFO(dcc_daten[1]);
247
            UART_FIFO(dcc_daten[2]);
248
            UART_FIFO(dcc_daten[3]);
249
            UART_FIFO(dcc_daten[4]);
250
            UART_FIFO(dcc_daten[5]);
251
            
252
            UART_FIFO(13);
253
            
254
            Flag.neuerBefehl = 0;
255
        }
256
    }
257
}

von mete (Gast)


Lesenswert?

Ohne einen konkreten Verdacht bezüglich dcc_daten[] zu haben, hier etwas 
anderes:

Ist dir bewusst dass der Zugriff auf die einzelnen Bits von Flag nicht 
atomar ist?

Die komplette struct Flag wird vom Compiler in eine einzelne (vermutlich 
sizeof (char) große?) Variable gepackt.

Wenn du sachen wie
1
    Flag.neuerBefehl = 0;

Schreibst, dann macht der Compiler folgendes:

1. Er liest die Variable mit allen Bits
2. Maskiert das Bit neuerBefehl
3. Schreibt diesen Wert zurück in die Flag Variable

Wenn zwischen diesen 3 Schritten die ISR aufgerufen wird, und dabei Flag 
verändert, dann überschreibt main() diese änderung der Flag Variable 
sobald die ISR beendet wird.

Vielleicht ist dein dcc_daten[] Problem ja nur ein Symptom dessen.

von Toralf W. (willi)


Lesenswert?

Danke. Wie gesagt ist meine erste C Übung. Das ganze Projekt läuft schon 
mit 760 Byte in ASM und ich dachte mir, das ist genau die richtige Größe 
um das mal zur Übung in C umzusetzen. Mit der Bit Variablen hast Du 
natürlich recht, das könnte schief gehen muss ich überdenken (cli/sei 
einfügen). Wenn die ISR da zwischen funkt, passiert aber auch nichts, da 
es in der ISR nur wieder gesetzt wird, wenn es vorher erfolgreich 
gelöscht wurde. Ansonsten werden die neuen Daten einfach verworfen.
dcc_daten[] wird in der ISR auch nur geändert, wenn das Flag.neuerBefehl 
erfolgreich auf Null gesetzt wurde.
Das sollte auch nicht mein Fehler sein. Ich denke eher, ich habe die 
Variablen nicht richtig C -konform deklariert, so das der Compiler mir 
da etwas wegoptimiert. Wie gesagt ohne Optimierung geht das so (mit 1300 
Byte) mit Optimierung (egal welche) werden es dann ca. 800 Byte aber der 
Inhalt von dcc_daten[] ist nur noch in der ISR richtig.

von Karl H. (kbuchegg)


Lesenswert?

Toralf Wilhelm schrieb:

> um das mal zur Übung in C umzusetzen. Mit der Bit Variablen hast Du
> natürlich recht, das könnte schief gehen muss ich überdenken (cli/sei
> einfügen). Wenn die ISR da zwischen funkt, passiert aber auch nichts, da
> es in der ISR nur wieder gesetzt wird,

Ähm. Es geht NICHT um das

  Flag.NeuerBefehl

es geht um die anderen Flags in dieser Struktur!


> Das sollte auch nicht mein Fehler sein. Ich denke eher, ich habe die
> Variablen nicht richtig C -konform deklariert,

Beim Drüberschauen ist mir nichts aufgefallen.
Wie hast du festgestellt, dass die Werte in der ISR noch in Ordnung 
sind?

von Karl H. (kbuchegg)


Lesenswert?

Deine FIFO Routinen sind IMHO fehlerhaft. Die Manipulation von 
fifo_anzahl ist nicht interruptfest. Am besten wirst du diese Anzahl 
überhaupt los. Du brauchst sie nicht. Durch den Write und den Read 
Pointer kannst du immer eine Aussage darüber treffen ob der FIFO voll 
ist bzw. die Anzahl errechnen, wenn diese gebraucht werden würde.
(und über die Technik, im Array 2 Einträge auf die harte Tour zu 
zweckentfremden, breiten wir jetzt erst mal den Mantel des Schweigens)


In Summe denke ich, dass du zuviel Code auf einmal geschrieben hast. Da 
jetzt den entscheidenden Fehler zu finden, wird ziemlich schwierig. 
Zumindest mir ist da jetzt nichts offensichtliches aufgefallen.

von Karl H. (kbuchegg)


Lesenswert?

Stimmen die Komentare?
1
//Timer 0 CTC CompA
2
//wird alle 10us ausgelöst liest bit vom DCCpin ein
3
//
4
ISR(TIMER0_COMPA_vect)

10µs ist schon ganz schön sportlich. Bei 8Mhz sind das 80 Takte von 
einer ISR zur nächsten.

von Stefan E. (sternst)


Lesenswert?

1
if( UCSRA & ((1<<UDRE)|(1<<TXC))){  // werden keine Daten übertragen und ist UDR bereit für neue Daten
Hier passt der Kommentar nicht zum Code. Die Bedingung ist auch wahr, 
wenn nur eines der Bits (UDRE oder TXC) 1 ist. Das Testen des TXC-Flags 
macht eh keinen Sinn. Du benutzt den TXC-Interrupt, der das Flag 
zurücksetzt, also wird es an dieser Stelle praktisch nie 1 sein.

von Toralf W. (willi)


Lesenswert?

Karl Heinz Buchegger schrieb:

> Ähm. Es geht NICHT um das
>
>   Flag.NeuerBefehl
>
> es geht um die anderen Flags in dieser Struktur!

ok, das ist wohl war, ich werde das ändern.

> Deine FIFO Routinen sind IMHO fehlerhaft. Die Manipulation von
> fifo_anzahl ist nicht interruptfest. Am besten wirst du diese Anzahl
> überhaupt los. Du brauchst sie nicht. Durch den Write und den Read
> Pointer kannst du immer eine Aussage darüber treffen ob der FIFO voll
> ist bzw. die Anzahl errechnen, wenn diese gebraucht werden würde.

ich wollte mir die eine Rechenoperation sparen, werde ich ändern. Soll 
ich die beiden Zeiger besser extra anlegen?
fifo_anzahl könnte auch mein Problem sein, weil den Test das die Daten 
in der ISR stimmen, habe ich natürlich gemacht, in dem ich mir in der 
ISR die Daten in den fifo geschrieben habe und da kann er ja nicht mehr 
unterbrochen werden.

alle 10us/80 Tackte eine ISR ist richtig, ich weiß, das das sportlich 
ist, aber es ist nur jede 12. oder 24. ISR länger (je nach empfangenden 
Daten) die anderen sind nur ein Paar Takte lang.

Stefan Ernst schrieb :

> Hier passt der Kommentar nicht zum Code. Die Bedingung ist auch wahr,
> wenn nur eines der Bits (UDRE oder TXC) 1 ist. Das Testen des TXC-Flags
> macht eh keinen Sinn. Du benutzt den TXC-Interrupt, der das Flag
> zurücksetzt, also wird es an dieser Stelle praktisch nie 1 sein.

Das stimmt, muss ich noch anpassen, das stammt noch aus dem FIFO Test 
ohne ISR. Wobei das oder sowie so falsch war, sollte schon & sein.

Ich werde das alles mal ändern.
Dank erst einmal
Willi

von Karl H. (kbuchegg)


Lesenswert?

Toralf Wilhelm schrieb:


> fifo_anzahl könnte auch mein Problem sein, weil den Test das die Daten
> in der ISR stimmen, habe ich natürlich gemacht, in dem ich mir in der
> ISR die Daten in den fifo geschrieben habe und da kann er ja nicht mehr
> unterbrochen werden.

Kannst du ja auf die Schnelle feststellen.
Kapsle den Inhalt der UART_FIFO in cli() und sei(). Dann hast du schon 
mal einen ersten Anhaltspunkt, ob die FIFO da was damit zu tun hat.

von Stefan E. (sternst)


Lesenswert?

Toralf Wilhelm schrieb:
> Wobei das oder sowie so falsch war, sollte schon & sein.

Ne, wäre auch falsch. Wenn du das | durch & ersetzt, dann ist die 
Bedingung immer false.

von Toralf W. (willi)


Lesenswert?

Karl Heinz Buchegger schrieb:
> Toralf Wilhelm schrieb:
>
>
>> fifo_anzahl könnte auch mein Problem sein, weil den Test das die Daten
>> in der ISR stimmen, habe ich natürlich gemacht, in dem ich mir in der
>> ISR die Daten in den fifo geschrieben habe und da kann er ja nicht mehr
>> unterbrochen werden.
>
> Kannst du ja auf die Schnelle feststellen.
> Kapsle den Inhalt der UART_FIFO in cli() und sei(). Dann hast du schon
> mal einen ersten Anhaltspunkt, ob die FIFO da was damit zu tun hat.

mache ich, komme aber erst heute Abend dazu.

Stefan Ernst schrieb:

> Ne, wäre auch falsch. Wenn du das | durch & ersetzt, dann ist die
> Bedingung immer false.

ja hast Du recht, habe ich mich doof ausgedrückt, das stammt noch aus 
der Testphase, ohne TXC ISR und da war es wichtig das nicht nur UDR frei 
war sondern auch TX fertig war, weil nur dann durfte der 455KHz Träger 
für die IR Übertragung abgeschaltet werden. Jetzt nutze ich ja die TXC 
ISR und das bedeutet das UDR dann eh schon für neue Daten bereit ist und 
wenn keine neuen Daten mehr im FiFo sind auch der Träger abgeschaltet 
werden kann (weil TXC). Ohne ISR hatte ich zum senden nur auf UDR frei 
getestet und da musste ich noch warten bis TX fertig ist, bevor ich den 
Träger abschalten konnte.

von Toralf W. (willi)


Lesenswert?

Karl Heinz Buchegger schrieb:

> (und über die Technik, im Array 2 Einträge auf die harte Tour zu
> zweckentfremden, breiten wir jetzt erst mal den Mantel des Schweigens)

noch einmal dazu eine Frage, ich habe den FIFO mehr oder weniger aus 
meinem ASM Code portiert. Da ist es einfacher bzw. schneller die Anzahl 
in einer dritten Variable abzulegen. Die ist schon überflüssig, man 
erhält die auch aus den beiden lese/schreib Pointer. Sollte ich diese 
besser so anlegen:
1
volatile uint8_t fifo[(FIFO_Groeße)];
2
static volatile uint8_t fifo_write_Pointer;
3
static volatile uint8_t fifo_read_Pointer;

von Karl H. (kbuchegg)


Lesenswert?

Toralf Wilhelm schrieb:
> Karl Heinz Buchegger schrieb:
>
>> (und über die Technik, im Array 2 Einträge auf die harte Tour zu
>> zweckentfremden, breiten wir jetzt erst mal den Mantel des Schweigens)
>
> noch einmal dazu eine Frage, ich habe den FIFO mehr oder weniger aus
> meinem ASM Code portiert. Da ist es einfacher bzw. schneller die Anzahl
> in einer dritten Variable abzulegen. Die ist schon überflüssig, man
> erhält die auch aus den beiden lese/schreib Pointer. Sollte ich diese
> besser so anlegen:

Du sollst in deinem C Buch über Strukturen nachlesen.
Das ist dein 'Werkzeug', wie du dir eine Datenstruktur machen kannst, 
die heterogen ist und in der du die Dinge zusammenfasst, die 
zusammengehören.

1
struct FIFO
2
{
3
  uint8_t Anzahl;
4
  uint8_t ReadPtr;
5
  uint8_t WritePtr;
6
  uint8_t Data[L_FIFO];
7
};
8
9
volatile struct FIFO myFifo;

von Toralf W. (willi)


Lesenswert?

Danke

von Peter (Gast)


Lesenswert?

Versuche mal ob es sich anders verhält, wenn Du alle Variabeln nur 
"volatile" ohne "static" deklarierst.

p.s. Du solltest nur die Variabeln als Volatile deklarieren, welche Du 
in der ISR und auch ausserhalb der ISR verwendest.

von Toralf W. (willi)


Lesenswert?

Danke noch einmal an alle, die mir hier geholfen haben. Abgesehen von 
meinen C Schwächen, lag das Problem einfach mal an der 10µs ISR. Ohne 
Optimierung bzw. mit einer zusätzlichen Verzögerung in der ISR passte 
meine Signalabtastung. Ich habe mich um 10µs bei der Signallänge 
verhauen und das viel mit der nicht mehr alle 10µs ausgeführten ISR 
nicht auf. Ich habe das alles inzwischen überarbeitet und es läuft 
genauso wie die ASM Version, allerdings mit 264 Byte mehr Code. Damit 
kann ich gut leben.

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.