AVR + 47HC164 Code-Optimierung

Warum nimmst Du denn keinen 74HC595 ?


Peter

Hi,

> Warum nimmst Du denn keinen 74HC595 ?

Oh.. den kannte ich noch nicht. Beim naechsten mal nehme ich natuerlich 
den. Das mit dem kurzen Aufblinken ist auch nicht so schlimm, ich wollte 
nur wissen, ob man nicht vielleicht noch einbisschen optimieren 
koennte..

Gruss,
Stefan

@  Stefan Noack (stefan_n)

>den. Das mit dem kurzen Aufblinken ist auch nicht so schlimm, ich wollte
>nur wissen, ob man nicht vielleicht noch einbisschen optimieren
>koennte..

Ja, siehe unten. Der direkte Zugriff auf den Port ist volatile, das ist 
so in den Include Files definiert. Mit einer Hilfsvariable gehts noch 
etwas schneller.

1

#include <avr/io.h> 

2

3

#include "leds.h"

4

5

void set_leds(uint8_t row0, uint8_t row1, uint8_t row2) {

6

  uint8_t tmp;

7

8

  for (uint8_t bit = 0x80; bit; bit>>=1) {  

9

10

    // clear, also sets clock to low

11

    LEDS_PORT &= ~LEDS_MASK;

12

    tmp=0;

13

14

    if (row0 & bit)

15

      tmp |= _BV(LEDS_AB0);

16

17

    if (row1 & bit)

18

      tmp |= _BV(LEDS_AB1); 

19

20

    if (row2 & bit)

21

      tmp |= _BV(LEDS_AB2);   

22

23

    // clock to high

24

    LEDS_PORT |= tmp | _BV(LEDS_CLK);

25

26

  }

27

28

}

Falk Brunner schrieb:

> Ja, siehe unten. Der direkte Zugriff auf den Port ist volatile, das ist
> so in den Include Files definiert. Mit einer Hilfsvariable gehts noch
> etwas schneller.

Nicht ratsam, weil dadurch data setup time = 0, muss aber mehr sein. 
Wenn schon dann sowas wie

1

    unsigned char temp = PORTB & ~...;

2

    if (row0 & bit)

3

      temp |= _BV(...);

4

    if (row1 & bit)

5

      temp |= _BV(...);

6

    if (row2 & bit)

7

      temp |= _BV(...);

8

    PORTB = temp;

9

    PORTB |= _BV(...);

Bringt aber zumindest in 4.3.4 ziemlich wenig.

Was mehr bringen dürfte: -O3. Aber auch mehr Platz, weil vollständiges 
unrolling.

Das mit der Hilfsvariable verstehe ich ehrlich gesagt nicht.

Laut der Doku von avr-libc wird aus

1

PORTB |= _BV(...)

einfach eine set-bit instruction:

>Access to the AVR single bit set and clear instructions are provided via
>the standard C bit manipulation commands. The sbi and cbi macros are no
>longer directly supported. sbi (sfr,bit) can be replaced by sfr |= BV(bit)
>
>i.e.: sbi(PORTB, PB1); is now PORTB |= _BV(PB1);
>
>This actually is more flexible than having sbi directly, as the optimizer
>will use a hardware sbi if appropriate, or a read/or/write operation if
>not appropriate. You do not need to keep track of which registers sbi/cbi
>will operate on.
>
>Likewise, cbi (sfr,bit) is now sfr &= ~(_BV(bit));

Mit der Zwischenvariable habe ich doch dann anstatt der SBI-instructions 
erstmal variablenzuweisungen und dann noch zusaetzlich einmal eine 
OUT-instruction, oder?

Stefan Noack schrieb:

> Laut der Doku von avr-libc wird aus
> ...
> einfach eine set-bit instruction:

Das schon, aber die braucht 2 Takte auf Port und nur 1 Takt auf 
Register. Ausserdem gilt das nicht wenn es mehrere Bits sind. Da die 
Maskierung folglich als load/and/store ausgeführt werden muss kann man 
das auch explizit codieren und das Tempregister weiterverwenden.

> erstmal variablenzuweisungen

Diese Variable sitzt allerdings in einem Register.

@  Stefan Noack (stefan_n)

>Laut der Doku von avr-libc wird aus

>PORTB |= _BV(...)

>einfach eine set-bit instruction:

Ach so stimmt. Ist mir irgendwie entfallen :-0
Gilt aber in erster Näherung für den AVR und andere Prozessoren, die das 
so schnell können.
Moment, SBI dauert aber 2 Takte, ein einfaches ORI nur 1. Der Vergleich 
ist der selbe. Also ist meine Version doch schneller ;-)
Hmm, nöö, doch nicht, am Ende ist es wieder gleich, siehe Anhang.

>Mit der Zwischenvariable habe ich doch dann anstatt der SBI-instructions
>erstmal variablenzuweisungen

Die wird aber in ein CPU-Register gelegt und ist damit maximal schnell.

> und dann noch zusaetzlich einmal eine OUT-instruction, oder?

Ja, aber die dauert nur 1 Takt. Aber vorhwr muss man laden und ODERn.

MfG
Falk

>> Laut der Doku von avr-libc wird aus
>> ...
>> einfach eine set-bit instruction:
>
> Das schon, aber die braucht 2 Takte auf Port und nur 1 Takt auf
> Register.

Ah.. stimmt! (Datenblatt lesen hilft...)

> Ausserdem gilt das nicht wenn es mehrere Bits sind. Da die
> Maskierung folglich als load/and/store ausgeführt werden muss kann man
> das auch explizit codieren und das Tempregister weiterverwenden.

Stimmt.. Ich maskiere sowieso, d.h. ich habe sowieso einen 
load/and/store-zyklus, den kann ich natuerlich gleich aufweiten und 
zwischendrin direkt mit dem register arbeiten, was doppelt so schnell 
ist.

>
>> erstmal variablenzuweisungen
>
> Diese Variable sitzt allerdings in einem Register.

..und das ist schneller.. habe kapiert.

Es hat auch was gebracht. Mit der Variable ist das aufblitzen kaum noch 
zu sehen. Mit -O3 sieht man es dann nur im dunkeln und bei ganz scharfem 
hingucken.

Naja naechstes mal nehme ich gleich den 595er..

Danke!

@ Falk: wie erzeugst du die .lss-Datei?

Originalversion 24-27 Takte (datenabhängig) pro Iteration, die 
modifizierte Version 23 Takte (gcc 4.3.4). Nicht wirklich der Brüller. 
Ergibt zusammengerechnet also an die 200 Takte.

Mit -O3 sind es jedoch für alle 8 Iterationen insgesamt nur knapp 90 
Takte.

@  Stefan Noack (stefan_n)

>@ Falk: wie erzeugst du die .lss-Datei?

Einfach im AVR-Studio in den Projektoptionen den Haken setzen.

Du meinst, ein I/O-Port sei effizienter als ein Prozessorregister??

@  Sauger (Gast)

>wenn man tmp durch eines der 3 GPIOR ersetzt, ist vielleicht noch was
>rauszukitzeln.

Nö, das Setzen der Bits in tmp als CPU-Register ist schon maximal 
schnell, ori dauert 1 Takt.
Aber SBI dauert nur 2 Takte. Die drei zusätzlichen Takte durch 3x sbi 
werden in der tmp-Variante am Ende wieder kompensiert, wo der Port 
gelesen, modifiziert und wieder geschrieben wird.
Wie gewonnen, so zerronnen. ;-)

Die tmp-Variante ist dann schneller, wenn man auf variable Bits per 
Bitmasken zugreifen muss, dann geht kein sbi/cbi.

http://www.mikrocontroller.net/articles/Bitmanipulation#Siehe_auch

MfG
Falk

Falk Brunner schrieb:

> Die tmp-Variante ist dann schneller, wenn man auf variable Bits per
> Bitmasken zugreifen muss, dann geht kein sbi/cbi.

Die I/O-Bits liegen fest, was fehlt ist die passende Operation zum Test 
ob gesetzt. Auch ein TEST Befehl, d.h. ein AND ohne Resultat, wäre 
hilfreich.

In der -O3 Variante ändert insbesondere diese Aspekt, denn bei 
vollständigem unrolling liegen die Bits fest und damit wird die 
Testoperation von MOV/AND/BREQ zu SBIC.

jw schrieb:

> Was spricht gegen USI, wenn's dann ganz schnell gehen muß?

Vermutlich der Umstand, dass er 3 davon bräuchte ;-).

A. K. schrieb:
> jw schrieb:
>
>> Was spricht gegen USI, wenn's dann ganz schnell gehen muß?
>
> Vermutlich der Umstand, dass er 3 davon bräuchte ;-).

Und sobald es 4 werden wird es langsamer als die Variante mit der 
Variable.

Sauger schrieb:
> unter umständen schon. Betrachte die GPIOR's als globale Variablen die
> in einen Register liegen. Dieses dürfte für den Compiler/Optimizer ein
> gefundenes Fressen sein, wenn der Funktionsaufruf von set_leds(...) und
> was davor kommt, inline wird. Ist aber zugegebenermaßen stark vom
> Programmfluss abhängig.

Der aufruf an set_leds() ist vollkommen unkritisch (davor und danach 
wird eh 250ms gepennt). Es geht nur darum, die bits so schnell wie 
möglich in das Register reinzukrachen, damit man es nicht sieht, dass 
ich zu blöd war, ein register mit Output Latches zu verwenden.

jw schrieb:
> Naja, deshalb doch seriell, oder?
> (164(Q7) an (in)164(Q7) an (in)164, paralleler Takt)

Nein, hat jeder einen eigenen pin (bleiben eh genug übrig). Clock ist 
gemeinsam.

Egal.. Ich löte jetzt Kondensatoren an die LEDs und dann hat sichs... 
;-)

Sauger schrieb:

> unter umständen schon. Betrachte die GPIOR's als globale Variablen die
> in einen Register liegen.

In einem I/O-Register, ja.

> Dieses dürfte für den Compiler/Optimizer ein
> gefundenes Fressen sein, wenn der Funktionsaufruf von set_leds(...) und
> was davor kommt, inline wird.

Inwieweit es sinnvoll sein könnte, ein Prozessorregister durch ein 
GPIORx zu ersetzen, das kann ich absolut nicht erkennen. Mit ist kein 
einziger Befehl bekannt, der mit GPIORx kürzer und/oder schneller ginge 
als mit einem Prozessorregister.

Die GPIORs sind genau dann sehr praktisch wenn man globale(!) Flagbits 
irgendwo unterbringen will, weil man dann die CBI/SBI/SBIC/SBIS-Befehle 
an Stelle LDS/.../STS verwenden kann. Gegenüber Byte-Variablen sind die 
Lade/Speicheroperationen immerhin ein bissel schneller. Beide Fälle sind 
hier nicht gegeben.

Stefan Noack schrieb:
> jw schrieb:
>> Naja, deshalb doch seriell, oder?
>> (164(Q7) an (in)164(Q7) an (in)164, paralleler Takt)
>
> Nein, hat jeder einen eigenen pin (bleiben eh genug übrig). Clock ist
> gemeinsam.
>
> Egal.. Ich löte jetzt Kondensatoren an die LEDs und dann hat sichs...
> ;-)

Und das nächste mal Schieberegister mit getrenntem Ausgangsregister. 
Dann stellt sich das Problem erst gar nicht.

Karl heinz Buchegger schrieb:
> Stefan Noack schrieb:
>> jw schrieb:
>>> Naja, deshalb doch seriell, oder?
>>> (164(Q7) an (in)164(Q7) an (in)164, paralleler Takt)
>>
>> Nein, hat jeder einen eigenen pin (bleiben eh genug übrig). Clock ist
>> gemeinsam.
>>
>> Egal.. Ich löte jetzt Kondensatoren an die LEDs und dann hat sichs...
>> ;-)
>
> Und das nächste mal Schieberegister mit getrenntem Ausgangsregister.
> Dann stellt sich das Problem erst gar nicht.

Ich habs ja verstanden...

Ich glaube nun ist alles gesagt, was hierzu gesagt werden kann.

Wenns schnell schieben soll, bastele zuerst die 8 Portbytes zusammen und 
gebe dann aus:

1

void fast_out( uint8_t d0,

2

               uint8_t d1,

3

               uint8_t d2,

4

               uint8_t d3,

5

               uint8_t d4,

6

               uint8_t d5,

7

               uint8_t d6,

8

               uint8_t d7 )

9

{

10

    LEDS_PORT = d0;

11

    LEDS_PORT |= _BV(LEDS_CLK);

12

    LEDS_PORT = d1;

13

    LEDS_PORT |= _BV(LEDS_CLK);

14

    LEDS_PORT = d2;

15

    LEDS_PORT |= _BV(LEDS_CLK);

16

    LEDS_PORT = d3;

17

    LEDS_PORT |= _BV(LEDS_CLK);

18

    LEDS_PORT = d4;

19

    LEDS_PORT |= _BV(LEDS_CLK);

20

    LEDS_PORT = d5;

21

    LEDS_PORT |= _BV(LEDS_CLK);

22

    LEDS_PORT = d6;

23

    LEDS_PORT |= _BV(LEDS_CLK);

24

    LEDS_PORT = d7;

25

    LEDS_PORT |= _BV(LEDS_CLK);

26

}

Das Schieben dauert dann nur 24 Zyklen.


Peter

Peter Dannegger schrieb:
> Wenns schnell schieben soll, bastele zuerst die 8 Portbytes zusammen und
> gebe dann aus
>
> Das Schieben dauert dann nur 24 Zyklen.
>

Passiert das bei -O3 nicht sowieso?

EDIT:

oh.. und wenn man die mit CLK geODERten auch noch vorher ausrechnet sind
es nur noch 16 takte :D Ich denke, ich werde das mal probieren...

2nd EDIT:

Könnte man die eingabewete auch als Array übergeben, ohne dass es 
langsamer wird?

Stefan Noack schrieb:
> Passiert das bei -O3 nicht sowieso?

Der Compiler ist doch kein Hellseher, er weiß nicht auf was es Dir 
ankommt.

Ein Compiler wird immer so wenig wie möglich Variablen verwenden.
Das Aufblasen von 3 auf 8 Register-Variablen mußt Du erzwingen.

Es kann durchaus sein, daß Du fast_out() noch als noinline definieren 
mußt, damit es nicht wieder zurückoptimiert wird.


Peter

verliere ich etwas, wenn ich ein array verwende?

1

void fast_out( uint8_t d[16]) __attribute__ ((noinline))

2

{

3

    LEDS_PORT = d[0];

4

    LEDS_PORT = d[1];

5

    LEDS_PORT = d[2];

6

    LEDS_PORT = d[3];

7

    LEDS_PORT = d[4];

8

    ...

9

}

da würde mir das zusammenpacken vorher einfacher fallen.

Stefan Noack schrieb:
> verliere ich etwas, wenn ich ein array verwende?

Ich denke, ja.
Schau mal ins Listing.

Die 8 Variablen müssen in Registern vorliegen, sonst gewinnst Du nichts.


Peter

So. Schneller geht es mit dem Schieben nicht (16 out-Anweisungen 
hintereinander):

1

static uint8_t get_bits(uint8_t d, uint8_t bit,

2

                 uint8_t row0, uint8_t row1, uint8_t row2)

3

{

4

  if (row0 & bit)

5

    d |= _BV(LEDS_AB0);

6

7

  if (row1 & bit)

8

    d |= _BV(LEDS_AB1); 

9

10

  if (row2 & bit)

11

    d |= _BV(LEDS_AB2);

12

13

  return d;

14

}

15

16

void set_leds(uint8_t row0, uint8_t row1, uint8_t row2) 

17

{ 

18

  uint8_t d = LEDS_PORT & ~LEDS_MASK;

19

20

  uint8_t d0 = get_bits(d, _BV(7), row0, row1, row2);

21

  uint8_t d1 = d0 | _BV(LEDS_CLK);

22

  uint8_t d2 = get_bits(d, _BV(6), row0, row1, row2);

23

  uint8_t d3 = d2 | _BV(LEDS_CLK);

24

  uint8_t d4 = get_bits(d, _BV(5), row0, row1, row2);

25

  uint8_t d5 = d4 | _BV(LEDS_CLK);

26

  uint8_t d6 = get_bits(d, _BV(4), row0, row1, row2);

27

  uint8_t d7 = d6 | _BV(LEDS_CLK);

28

  uint8_t d8 = get_bits(d, _BV(3), row0, row1, row2);

29

  uint8_t d9 = d8 | _BV(LEDS_CLK);

30

  uint8_t dA = get_bits(d, _BV(2), row0, row1, row2);

31

  uint8_t dB = dA | _BV(LEDS_CLK);

32

  uint8_t dC = get_bits(d, _BV(1), row0, row1, row2);

33

  uint8_t dD = dC | _BV(LEDS_CLK);

34

  uint8_t dE = get_bits(d, _BV(0), row0, row1, row2);

35

  uint8_t dF = dE | _BV(LEDS_CLK);

36

37

  asm("out %0, %1"  "\n\t"

38

      "out %0, %2"  "\n\t"

39

      "out %0, %3"  "\n\t"

40

      "out %0, %4"  "\n\t"

41

      "out %0, %5"  "\n\t"

42

      "out %0, %6"  "\n\t"

43

      "out %0, %7"  "\n\t"

44

      "out %0, %8"  "\n\t"

45

      "out %0, %9"  "\n\t"

46

      "out %0, %10" "\n\t"

47

      "out %0, %11" "\n\t"

48

      "out %0, %12" "\n\t"

49

      "out %0, %13" "\n\t"

50

      "out %0, %14" "\n\t"

51

      "out %0, %15" "\n\t"

52

      "out %0, %16" "\n\t"

53

      :

54

      : "I" (_SFR_IO_ADDR(LEDS_PORT)),

55

        "r" (d0), "r" (d1), "r" (d2), "r" (d3),

56

        "r" (d4), "r" (d5), "r" (d6), "r" (d7),       

57

        "r" (d8), "r" (d9), "r" (dA), "r" (dB),

58

        "r" (dC), "r" (dD), "r" (dE), "r" (dF)                

59

     );

60

}

Damit ist die Optimierung nun offiziell abgeschlossen ;-)

Du darfst natürlich keine Wunder erwarten. Das Auge ist logarithmisch.
Wenn die Schiebezeit halbiert wird, hast Du nicht den Eindruck, daß das 
Aufblitzen der ausgeschalteten LEDs auch nur halb so hell ist.

Sinnvoll ist auch, alle 100Hz eine neue Ausgabe zu machen. Dann sieht 
man das Aufblitzen nicht mehr.


Peter