GCC bei vielen Bitshiftingfunktionen sehr suboptimal?

[Matthias Larisch (Gast)]

Hi!

Ich programmiere erst seit gestern mit WinAVR in neuster Version, vorher 
habe ich so einige Projekte in Assembler realisiert, aber ich wollte mir 
das Leben auch mal einfacher & übersichtlicher gestalten ;-) Deshalb 
mache ich gerade ein Spaßprojekt, bei dem ein BlinkenLEDs Display von 
einem Mega 8 angesteuert wird.

Nun ist mir aufgefallen, dass der Compiler bei Operationen, wo ich 
diverse Bits in anderer Reihenfolge in andere Variablen bringe (kurz: 
Bitshifting betreibe) keine sonderlich gute Arbeit leistet. Mit Inline 
ASM hab ich einige Stellen innerhalb kürzester Zeit auf 35% Zyklenbedarf 
bei ~20% Flash ersparnis reduziert... Nun eine Zeile, welche ich euch 
einmal zeigen möchte:

228:          LCD_XTRA |= (lcd_framebuffer[3*(7-i)+2] & 64) >> (6 - LCD_D8);
+00000172:   B328        IN      R18,0x18         
+00000173:   8182        LDD     R24,Z+2          
+00000174:   E090        LDI     R25,0x00         
+00000175:   7480        ANDI    R24,0x40         
+00000176:   7090        ANDI    R25,0x00         
+00000177:   E036        LDI     R19,0x06         
+00000178:   9595        ASR     R25              
+00000179:   9587        ROR     R24              
+0000017A:   953A        DEC     R19              
+0000017B:   F7E1        BRNE    PC-0x03          
+0000017C:   2B28        OR      R18,R24          
+0000017D:   BB28        OUT     0x18,R18

Das steht in einer Schleife, i ist der Schleifenzähler. LCD_D8 ist ein 
#define LCD_D8 0.

Merkt der Compiler an der Stelle nicht, dass ich lediglich 1 Bit von 
einer variablen in ne andere bringen muss? Gibt es eine sinnvollere 
Schreibweise, wie ich ihm das mitteilen kann? Ich würde in ASM einfach 
folgendes machen:

ldd r24, z+2
bld r24, 6
brtc weiter
sbi 0x18, 0
weiter:

das sind grad mal 4 Instruktionen, welche exakt das gleiche machen. 
Warum kann mein gcc nicht auch wenigstens Ansatzweise solchen Code 
generieren?

Ich hoffe ja immernoch, dass der Fehler bei mir zu suchen ist... Denn 
mir vergeht die Lust an C auf AVR, wenn mir jedes mal, wenn ich mir das 
List file angucke, 1000 Verbesserungsmöglichkeiten auffallen.

Mir ist klar, dass der Compiler nicht perfekt arbeitet, aber ein 
weeeeenig kann ich ja wohl von ihm erwarten? Teilweise klappts ganz gut 
mit der Optimierung, aber eben an einigen Ecken und Kanten nicht.

Ich benutze übrigens -Os als Optimierungsstufe.

Vielen Dank,

Matthias

[eProfi (Gast)]

Ich glaube, das merkt er nicht. Das musst Du ihm schon vorkauen:

LCD_XTRA |= (lcd_framebuffer[3*(7-i)+2] & 64) >> (6 - LCD_D8);
-->
if (lcd_framebuffer[3*(7-i)+2] & 64){LCD_XTRA |= 1;}

Und ich wette, lcd_framebuffer[] ist 16 bit breit, also int.
Bei 8 Bit (char) würde der Code schon etwas kürzer sein.

[Matthias Larisch (Gast)]

Hi!

Danke erstmal, das mit dem if... oehm, irgendwie logisch, so könnts 
gehen :-)

Und nein, ich vergaß zu schreiben, lcd_framebuffer ist ein uint8_t 
Array. Werde morgen früh mal auf die If-Abfrage umstellen und mal 
schauen, was bei raus kommt, jetzt gehts erstmal ins Bett.

Vielen Dank!

Matthias

[holger (Gast)]

>Gibt es eine sinnvollere
>Schreibweise, wie ich ihm das mitteilen kann?

Ja, sicher. Erstmal schreibst du nicht alle Berechnungen
in eine Zeile. Und welchen Variablentyp hat i ? int i ?
Ohne etwas mehr Code kann man dazu kaum was sagen.

[Matthias Larisch (Gast)]

Guten Morgen!

Okay, hier dann mal alle relevanten Code-Abschnitte, dachte, aus oben 
genanntem kann man das bereits rausziehen:

#define LCD_DATA PORTD
#define LCD_XTRA PORTB
#define LCD_D8 PB0
#define LCD_CLK PB3

#define LCD_CLK_HIGH LCD_XTRA |= (1 << LCD_CLK);
#define LCD_CLK_LOW LCD_XTRA &= ~(1 << LCD_CLK);
uint8_t lcd_framebuffer[24];

[...]

for (uint8_t i = 0; i < 8; i++)
  {
    uint8_t tempo, temp;
    /* An dieser Stelle wird noch temp und tempo berechnet, lcd_framebuffer enthält eigentlich die
Pixeldaten in einem anderen Format, welches hier umgewandelt wird. Das dürfte aber irrelevant sein.
In der folgenden Zeile wird dann ein Teil, welcher erst nach den ersten 8 Durchläufen gebraucht wird,
in einen Teil des Buffers geschrieben, der schon konvertiert wurde (sozusagen Doppelbenutzung zum
Speichersparen). Aber auch das dürfte nicht relevant sein, funktionieren tut es*/
    lcd_framebuffer[3*(7-i)] = temp; 
    LCD_DATA = tempo;
    LCD_XTRA &= ~( 1 << LCD_D8);
    LCD_XTRA |= (lcd_framebuffer[3*(7-i)+2] & 64) >> (6 - LCD_D8);
    LCD_CLK_HIGH;
    LCD_CLK_LOW;
  }

for (uint8_t i = 0; i < 8; i++)
  {
    LCD_DATA = lcd_framebuffer[3*(7-i)];
    LCD_XTRA &= ~( 1 << LCD_D8);
    LCD_XTRA |= (lcd_framebuffer[3*(7-i)+2] & 128) >> (7 - LCD_D8);
    LCD_CLK_HIGH;
    LCD_CLK_LOW;
  }

In der 2. For-Schleife implementiert er das etwas besser:

237:          LCD_XTRA |= (lcd_framebuffer[3*(7-i)+2] & 128) >> (7 - LCD_D8);
+00000189:   B398        IN      R25,0x18         
+0000018A:   01ED        MOVW    R28,R26          
+0000018B:   818A        LDD     R24,Y+2          
+0000018C:   1F88        ROL     R24              
+0000018D:   2788        CLR     R24              
+0000018E:   1F88        ROL     R24              
+0000018F:   2B89        OR      R24,R25          
+00000190:   BB88        OUT     0x18,R24

Wobei der move von R26:R25 in R28:R27 keinen Sinn ergibt, denn R28:R27 
wird überhaupt nicht mehr verwendet, bevor es vor Ende der Funktion vom 
Stack gepopt wird :-) Er macht hier die Bitübertragung explizit mit 
rollen durchs carry. Warum nicht den kack sparen und:

LDD R24,Y+2
SBRS R24,7
rjmp nicht_setzen
sbi LCD_XTRA, LCD_D8
nicht_setzen:

eben genau wie an der anderen Stelle (der ASM Code der anderen Funktion 
steht oben)

Also, wäre nett, wenn ihr mir schreiben könntet, wie ich das anstellen 
soll :-) Ich probiere jetzt, wie gestern schon gesagt, erstmal die 
If-Anweisung aus.

Matthias

[Matthias Larisch (Gast)]

Hm jetzt muss ich mir schon selbst antworten, kann man als registrierter 
Benutzer seine Beiträge editieren?

Also: If ist die Lösung. Sogar noch schlauer als ich, bin wohl grade 
nicht so ganz drin im ASM:

229:          if(lcd_framebuffer[3*(7-i)+2] & 64) LCD_XTRA |= (1 << LCD_D8);
+00000172:   8182        LDD     R24,Z+2          
+00000173:   FD86        SBRC    R24,6            
+00000174:   9AC0        SBI     0x18,0

Wunderbar, so solls sein ^^

Schade nur, dass er das nicht automatisch aus der anderen Zeile erkennt, 
denn durch das & ist ja klar, dass ich mich nur auf exakt 1 bit beziehe, 
durch das oder ist klar, dass es nur gesetzt werden kann und der shift 
zeigt an, um welches ziel-bit es sich handelt..

Immerhin zeigt das gut, dass man auch als C-Programmierer immer etwas 
mitdenken sollte :-)

Vielen Dank & noch einen schönen Sonntag,

Matthias

[Philipp B. (philipp_burch)]

Matthias Larisch wrote:
> Hm jetzt muss ich mir schon selbst antworten, kann man als registrierter
> Benutzer seine Beiträge editieren?

Kann man.

> Immerhin zeigt das gut, dass man auch als C-Programmierer immer etwas
> mitdenken sollte :-)

Allerdings. Aber mich wundert teilweise schon, wie intelligent der GCC 
doch ist. Der ersetzt mir doch tatsächlich eine Subtraktionsschleife 
durch einen Aufruf von _divmodqi4...

[Matthias L. (matze88)]

So, hab mich angemeldet.

Ja, ich war auch überrascht, wie er bei oben geschriebenen Code die 
for-schleife implementiert hat:
X Pointer wird auf höchste "base-adresse" gesetzt, also auf 
lcd_framebuffer[3*(7-i)] und dann für jeden loop um 3 verringert, dann 
einfach gegen den niedrigsten wert getestet, falls drunter -> loop zu 
ende... Also komplett ohne eigene Loopvariable sozusagen :-) Das gefällt 
mir wirklich sehr gut!

Naja, all das zeigt auch, das man sich den entstehenden ASM Code doch 
mal anschauen sollte. Wie eine kleine Veränderung so viel bewirken kann 
(das ersetzen der hier diskutierten Zeile durch gleichwertiges mit dem 
If verringert die Zyklenzahl für den kompletten Funktionsaufruf von 1126 
auf 822 - also benötigt nur 73% der Zeit!

Matthias