Forum: Compiler & IDEs eigene memcpy Funktion mit 2 Zielen


von Tobias W. (tobiasw)


Lesenswert?

Servus,

ich Sitze nun schon eine Weile daran mir eine memcpy-Funktion zu bauen, 
welche Daten aus einer Quelle an 2 Ziele schreibt.

Genauer geht es darum, dass ich 3 SPI-Puffer habe die relativ häufig das 
selbe Datenpaket aufnehmen sollen.
die 3 Puffer sind dabei als
1
 uint8_t spi_buf[3][6];
angelegt. Nun möchte ich, wie erwähnt recht häufig, gleiche Daten in die 
Puffer legen. Ich stelle nun also meine Daten im Puffer 1 zusammen und 
kopiere den Puffer dann in die anderen. Da nun memcpy aber offenbar 
grundsätzlich geinlined wird und mir nun doch langsam der Speicher 
knapper wird dachte ich an eine Funktion, die etwas flotter und 
kompakter ist fie folgende:
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  __asm__ __volatile__(
4
    "ldi r24,  6        \n\t"
5
    "ldi r30,lo8(data)      \n\t"
6
    "ldi r31,hi8(data)      \n\t"
7
    "0:              \n\t"
8
    "ld __tmp_reg__,Z+      \n\t"
9
    "std Z+5,__tmp_reg__    \n\t"
10
    "std Z+11,__tmp_reg__    \n\t"
11
    "dec r24          \n\t"
12
    "brne 0b          \n\t"
13
    :
14
    : "e" (buf)
15
    : "r24", "r30", "r31"    
16
  );
17
}
18
19
20
[...]
21
// aufruf mit
22
duplicate_buffer( spi_buf[0] );
da ich nun aber wirklich noch ein kleiner Anfänger bin, frage ich lieber 
mal die Profis ob das so passt, oder ob dabei irgendwas falsch ist. 
Sorgen mache ich mir vor allem um die clobberliste, da hier die Angaben 
im Internet über das, was da stehen sollte doch eher verwirrend oder 
widersprüchlich ist. Und das ganze richtig ausgibig Testen kann ich als 
Anfänger wirklich nicht, da ich nicht weiß welche Spezialfälle man am 
besten zu Testen ansetzt...

Vielleicht gibt es ja auch eine alternative, sicherere Methode, dieses 
Optimierungsproblem zu lösen. Muss ja gar nicht inline-Assembler sein, 
aber wie ich dem Compiler beibringe das ganze halbwegs effizient aus C 
zu übersetzten ist mir gerade nicht klar.
Vielleicht hat ja sogar schon mal jemand eine solche Funktion 
geschrieben und kann/will mir damit etwas helfen.

Wäre wirklich sehr nett.

Vielen Dank
Gruß Tobi

von Karl H. (kbuchegg)


Lesenswert?

Tobias Weilenbach schrieb:

> Vielleicht gibt es ja auch eine alternative, sicherere Methode, dieses
> Optimierungsproblem zu lösen. Muss ja gar nicht inline-Assembler sein,
> aber wie ich dem Compiler beibringe das ganze halbwegs effizient aus C
> zu übersetzten ist mir gerade nicht klar.

Hast du schon mal das naheliegende
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  for( uint8_t i = 0; i < 6; i++ )
4
    buf[i+11] = buf[i+5] = buf[i];
5
}

durch den Compiler gejagt und im Listing File nachgesehen, was dabei 
rauskommt?

von Roland P. (pram)


Lesenswert?

Ich möchte fast behaupten, dass der Compiler aus
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  int i;
4
  for (i = 0;  i < 6; i++) {
5
     buf[i+12] = buf[i+6] = buf[i]
6
  }
7
}
bei eingeschalteter Optimierung bereits sehr effizienten Code macht.
Du kannst ja mal in der .lss Datei schauen was daraus wird.

Gruß
Roland

/edit: Karl-Heinz war schneller, aber ich glaube er hat sich verzählt, 
bzw die +5 falsch aus dem Assembler-Code übernommen

von Peter II (Gast)


Lesenswert?

es könnte aber durchaus sein das

buf[i+12] = buf[i+6] = buf[i]

recht langsam ist, weil er mit zu vielen Zeiger arbeiten muss. Ich kann 
mir sogar vorstellen das 2 schleifen schneller sind, weil damit mehr im 
Register gemacht werden kann.

von Oliver (Gast)


Lesenswert?

Peter II schrieb:
> es könnte aber durchaus sein
> Ich kann mir sogar vorstellen...

das der Mond aus grünem Käse ist. Sowas findet man nicht da durch raus, 
daß man wilde Spekulationen anstellt, sondern man fliegt hin, und sieht 
nach.

Also jag den Code durch den Compiler, und schau nach, was rauskommt.

Oliver

von Peter D. (peda)


Lesenswert?

Tobias Weilenbach schrieb:
> Genauer geht es darum, dass ich 3 SPI-Puffer habe die relativ häufig das
> selbe Datenpaket aufnehmen sollen.

Wozu dann der Umweg über 3 Puffer?
Übergibt den 3 SPI einfach den Zeiger auf den gleichen Puffer.
Nix kopieren und obendrein RAM gespart.


Peter

von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Tobias Weilenbach schrieb:
> Da nun memcpy aber offenbar
> grundsätzlich geinlined wird und mir nun doch langsam der Speicher
> knapper wird dachte ich an eine Funktion, die etwas flotter und
> kompakter ist fie folgende:

Was nutzt die "kompakter" Code, der nicht korrekt ist? Der gezeigte Code 
ist nämlich falsch.

Inlining kannst du vermeiden, indem die Funktion in ein eigenes Modul 
kommt und keine Optimierungen wie LTO verwendet werden:
1
extern void duplicate_buffer (uint8_t*);

Da kann nix geinlint werden!

von Karl H. (kbuchegg)


Lesenswert?

Roland Praml schrieb:

> /edit: Karl-Heinz war schneller, aber ich glaube er hat sich verzählt,
> bzw die +5 falsch aus dem Assembler-Code übernommen

Bin mir nicht sicher.
Spielst du auf die Byte-Word Adressrechnerei an?

Da buf ein uint8_t * ist, bin ich davon ausgegangen, dass die 
Adressoffsets im Assembler Code stimmen müssten.


Aber ich denke, wir sind uns alle einig.
Dafür Assembler-Code zu benutzen ist reichlich unsinnig. Das kriegt man 
als Assembler-Programmierer auch nicht besser hin als der Compiler. Ist 
ja schliesslich trivial genug. Das einzige was man dabei machen kann, 
sind Fehler im Assemblercode.
Wenn überhaupt, muss man sich die Frage stellen, die Peter aufgeworfen 
hat: Wozu überhaupt duplizieren?

von Karl H. (kbuchegg)


Lesenswert?

Karl Heinz Buchegger schrieb:
> Roland Praml schrieb:
>
>> /edit: Karl-Heinz war schneller, aber ich glaube er hat sich verzählt,
>> bzw die +5 falsch aus dem Assembler-Code übernommen
>
> Bin mir nicht sicher.
> Spielst du auf die Byte-Word Adressrechnerei an?
>
> Da buf ein uint8_t * ist, bin ich davon ausgegangen, dass die
> Adressoffsets im Assembler Code stimmen müssten.

Ah,
Ich hatte Tomaten auf den Augen. Es geht um das Z+ im lds.
Ja, ok. So gesehen hab ich einen Fehler gemacht.

Berührt aber das Grundsatzproblem nicht.

von Tobias W. (tobiasw)


Lesenswert?

Ersteinmal Danke für die vielen und hilfreichen Antworten.
Ich hatte ja geschrieben, es muss kein Assembler sein, aber mir war 
nicht klar wie ich dem Compiler beibringe den Code etwas kompakter zu 
machen. Dabei meinte ich implizit natürlich, dass er eben nicht so 
schnell wie nur möglich ist, und vielleicht sogar die schleife auflösen, 
wie es der Compiler teils gemacht hat, sondern ein paar Bytes im 
Programmspeicher einzusparen, da mir der langsam aber sicher knapper 
wird.
Dass soetwas
1
    buf[i+12] = buf[i+6] = buf[i];
 geht, war mir bisher unbekannt, aber glücklicherweise lernt man ja nie 
aus und ich hab da sogar noch mehr Potential als so manch ein Anderer... 
;)

Ich habe mir nun angesehen was der Compiler aus
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  for( uint8_t i = 0; i < 6; i++ )
4
    buf[i+11] = buf[i+5] = buf[i];
5
}
macht. Das verfolgt zwar um Welten nicht die Idee meines Assembler-Codes 
aber ist definitiv wesentlich schlanker als 2x memcpy() hintereinander.

Johann L. schrieb:
> Was nutzt die "kompakter" Code, der nicht korrekt ist? Der gezeigte Code
> ist nämlich falsch.

Würde mich noch interessieren wo der Fehler steckt? meinst du, dass da 
2x data steht wo buf stehen sollte? ist dann wohl ein copy-paste-Fehler, 
da ich die Zeilen aus meinen Testprogrammen zusammenkopiert habe...


PS: bevor ichs vergesse: ich nutze 3 Puffer, da dort auch die 
empfangenen Daten drin landen, und diese unterscheiden sich

von Fabian O. (xfr)


Lesenswert?

Tobias Weilenbach schrieb:
> Dass soetwas    buf[i+12] = buf[i+6] = buf[i];
>  geht, war mir bisher unbekannt

Muss man auch nicht unbedingt wissen, um schnellen Code zu schreiben. 
Diese beiden Varianten ergeben bei eingeschalteter Optimierung ziemlich 
sicher ebenso effizienten Maschinencode:
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  for( uint8_t i = 0; i < 6; i++ ) {
4
    buf[i+6]  = buf[i];
5
    buf[i+12] = buf[i];
6
  }
7
}
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  for( uint8_t i = 0; i < 6; i++ ) {
4
    uint8_t val = buf[i];
5
    buf[i+6]  = val;
6
    buf[i+12] = val;
7
  }
8
}

Anders wäre es, wenn buf statt uint8_t* ein volatile uint8_t* wäre. Dann 
wäre die zweite Variante oder die verkettete Schreibweise vorzuziehen, 
um nicht unnötigerweise zwei Mal auf buf[i] zuzugreifen.

von Peter D. (peda)


Lesenswert?

Generell sollte man vorm Optimieren erstmal feststellen, ob das wirklich 
der Ressource-Killer ist, der 50% CPU-Zeit oder 50% Flash verbraucht.
Wenn es dagegen nur <1% sind, lohnt sich ein Anfassen kaum.

Man sollte auch bedenken, daß Assembler die Lesbarkeit und Wartbarkeit 
erheblich vermindert. Auch kann es sein, daß Dein Assemblercode jetzt 
läuft, mit einer neueren Compilerversion aber Fehler macht.

Kleine Funktionen zu optimieren ist also nur der Spaß, den Du dabei 
hast, hat aber keinen praktischen Nutzeffekt.


Peter

von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Tobias Weilenbach schrieb:
> Johann L. schrieb:
>> Was nutzt die "kompakter" Code, der nicht korrekt ist? Der gezeigte Code
>> ist nämlich falsch.
>
> Würde mich noch interessieren wo der Fehler steckt? meinst du, dass da
> 2x data steht wo buf stehen sollte? ist dann wohl ein copy-paste-Fehler,
> da ich die Zeilen aus meinen Testprogrammen zusammenkopiert habe...

buf kann da nicht stehen, denn buf ist kein Symbol sondern ein 
Parameter.

Die Constraints sind falsch.

Es fehlt ein memory-Clobber.

buf wird nicht nach Z allokiert wenn Z geclobbert wird.

etc. etc.

von Tobias W. (tobiasw)


Lesenswert?

Irgendwie will ichs jetz wissen.
Ich hab mich jetzt noch einmal ausgiebig damit beschäftigt und bin zu 
folgendem Ergebnis gekommen:
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  uint8_t i;
4
  __asm__ __volatile__(
5
    "ldi  %0, 6        \n\t"
6
    "0:              \n\t"
7
    "ld  __tmp_reg__,%a1+    \n\t"
8
    "std  %a1+5,__tmp_reg__  \n\t"
9
    "std  %a1+11,__tmp_reg__  \n\t"
10
    "dec  %0          \n\t"
11
    "brne  0b          \n\t"
12
    : "=&d" (i)
13
    : "e" (buf)
14
    : "memory"
15
  );
16
}
Ich überlasse damit dem Compiler die Wahl des Pointer-Registers und das 
Zähler-Register darf er auch fast frei (r16 - r31) wählen.
Muss/Kann ich ihm noch irgendwie mitteilen, dass ich das 
Pointer-Register verändere, oder wie ist das damit?

Herzlichen Dank für alle Einwände soweit. Man lernt ja wirklich viel 
dabei!

von Läubi .. (laeubi) Benutzerseite


Lesenswert?

Tobias Weilenbach schrieb:
> nd mir nun doch langsam der Speicher
> knapper wird

Hast du den mal geschaut was da speicher frist? Flaot lib und/oder keine 
-Os Optimierung?
Was spricht gegen die Idee memcopy zu nutzen aber in eine Funktion 
gekapselt, ggf mit "never inline" pragma?

von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Tobias Weilenbach schrieb:

> Muss/Kann ich ihm noch irgendwie mitteilen, dass ich das
> Pointer-Register verändere, oder wie ist das damit?

Nur noch 2 Fehler ;-)

Ja, muss man ihm sagen. Ansonsten kann es falschen Code geben.  Wenn 
sich der Compiler für Y entscheidet wäre das unangenehm...

Ausserdem ist "e" die falsche Constraint, korrekt ist z.B. "b".

Um zu kennzeichnen, daß buf verändert wird, geht

   ... "+b" (buf) :: "memory"

und auch

   ... "=b" (buf) : "1" (buf) : "memory"

Das LDI kann man auch sparen per

   ... "=d" (i) : "0" (6)

Und auch die magischen Zahlen "5" und "11" lassen sich vermeiden bzw. 
zumindest aus dem asm rausziehen, so daß der Wert besser 
parametrisierbar ist:

  "std  %a1+%2+%2-1, ..." : ... : "n" (6)

von Tobias W. (tobiasw)


Lesenswert?

Hammer! Danke Johann!

Warum ist die Constraint "e" falsch und "b" richtig?
wenn ich 
http://www.rn-wissen.de/index.php/Inline-Assembler_in_avr-gcc#Operanden_und_Constraints 
vertrauen darf, dann ist für "e" nur das Pointer-Register X eine 
zusätzliche Option? oder wo ist der Fehler?

ich habe jetzt auch probiert deine Anregungen umzusetzten, allerdings 
kann ich nicht mehr Compilieren und ich finde den Fehler nicht
1
void duplicate_buffer( uint8_t * buf )
2
{
3
  uint8_t i;
4
  __asm__ __volatile__(
5
    "0:              \n\t"
6
    "ld  __tmp_reg__,%a1+    \n\t"
7
    "std  %a1+%2-1,__tmp_reg__  \n\t"
8
    "std  %a1+%2+%2-1,__tmp_reg__  \n\t"
9
    "dec  %0          \n\t"
10
    "brne  0b          \n\t"
11
    : "=d" (i),
12
      "+b" (buf)
13
    : "n" (6),
14
      "0" (6)
15
    : "memory"
16
  );
17
}
1
warning: asm operand 4 probably doesn't match constraints [enabled by default]
2
error: impossible constraint in 'asm'
ich habe auch schon alles mögliche, das mir eingefallen ist, rum 
probiert, aber das haut nicht hin?
Jetzt versteh ichs nimmer so ganz. Warum macht er das nicht?

von Sven B. (scummos)


Lesenswert?

Hast du mal getestet, was gcc mit -Os tut? Ist das wirklich nicht gut 
genug? Das sollte man ausprobieren, bevor man sich die ganze Arbeit 
macht. ;)

von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Tobias Weilenbach schrieb:
> Warum ist die Constraint "e" falsch und "b" richtig?
> wenn ich
> 
http://www.rn-wissen.de/index.php/Inline-Assembler_in_avr-gcc#Operanden_und_Constraints
> vertrauen darf, dann ist für "e" nur das Pointer-Register X eine
> zusätzliche Option? oder wo ist der Fehler?

X+const Adressierung gibt es nicht. Erhöhe einfach die Registerlast, 
etwa indem du 28 und 30 clobberst zusammen mit "e", und schaust was 
passiert.

> error: impossible constraint in 'asm'
> ich habe auch schon alles mögliche, das mir eingefallen ist,
> rum probiert, aber das haut nicht hin?
> Jetzt versteh ichs nimmer so ganz. Warum macht er das nicht?

Vielleicht eine ältliche Compilerversion? Mit 4.7 und 4.8 geht's 
zumindest. Ausserdem kann man eine 8-Bit Konstante verwenden.

Die doppelte 6 bekommt man wohl nicht raus, es sei denn so:
1
#include <stdint.h>
2
3
void __attribute__((__noinline__,__noclone__))
4
duplicate_buffer (uint8_t *buf)
5
{
6
    uint8_t i;
7
    __asm__ __volatile__ (
8
        "$ ldi %[cnt], %[n]"
9
        "$ 0:"
10
        "$ ld   __tmp_reg__,  %a[addr]+"
11
        "$ std  %a[addr]+%[n]-1,      __tmp_reg__"
12
        "$ std  %a[addr]+%[n]+%[n]-1, __tmp_reg__"
13
        "$ dec  %[cnt]"
14
        "$ brne 0b"
15
        : [cnt] "=d" (i), [addr] "+b" (buf)
16
        : [n] "n" (6)
17
        : "memory");
18
}

Warum funktioniert Perers Lösung nicht?

Beitrag "Re: eigene memcpy Funktion mit 2 Zielen"

von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Und es gibt auch Lösungen in C ohne Schleife und ohne memcpy:
1
void duplicate_buffer (void *buf)
2
{
3
    struct { char c[6]; } (*a)[3] = buf;
4
    (*a)[2] = (*a)[1] = (*a)[0];
5
}

Selbst dann wenn die Stride nicht zur Compilezeit bekannt ist:
1
#include <stdlib.h>
2
3
void duplicate_buffer (void *buf, size_t s)
4
{
5
    struct { char c[s]; } (*a)[3] = buf;
6
    (*a)[2] = (*a)[1] = (*a)[0];
7
}

von Tobias W. (tobiasw)


Lesenswert?

Danke Johann!

Ja Lösungen in C gibts einige, wobei ich sagen muss, dass der Compiler 
aus deinen Ansätzen wieder 2x memcpy(..) macht... die einzige 
optimierung dabei ist dann, dass es nicht mehr inline ist (was 
eigentlich schon 90% der Optimierung ausmacht). Wäre vollkommen 
ausreichend, aber irgendwann kommt einfach der Punkt da will mans 
wissen.
in C ist folgendes die flotteste Implementierung, die ich meinem 
Compiler abfordern kann:
1
void duplicate_buffer (uint8_t *buf)
2
{
3
  for( uint8_t i=6; i>0; i-- )
4
  {
5
    uint8_t tmp = *buf++;
6
    *(buf+5) = tmp;
7
    *(buf+11) = tmp;
8
  }
9
}
der Compiler macht daraus dann:
1
void duplicate_buffer (uint8_t *buf)
2
{
3
000006fe <duplicate_buffer>:
4
5
 6fe:  fc 01         movw  r30, r24
6
 700:  06 96         adiw  r24, 0x06  ; 6
7
  for( uint8_t i=6; i>0; i-- )
8
  {
9
    uint8_t tmp = *buf++;
10
 702:  21 91         ld  r18, Z+
11
    *(buf+5) = tmp;
12
 704:  25 83         std  Z+5, r18  ; 0x05
13
    *(buf+11) = tmp;
14
 706:  23 87         std  Z+11, r18  ; 0x0b
15
16
  for( uint8_t i=6; i>0; i-- )
17
 708:  e8 17         cp  r30, r24
18
 70a:  f9 07         cpc  r31, r25
19
 70c:  d1 f7         brne  .-12       ; 0x702 
20
21
 70e:  08 95         ret
wie man sieht, eliminiert er zwar die Zählervariable und prüft die 
Schleifenbedingung lieber über den Pointer, aber gut für solche 
Feinheiten gibt's ja inline-assembler.


und zu deinem assembler-Code: 1000 Dank! Da steckt wieder viel zu lernen 
drin. $ statt \n\t, dass man den operanden mit [name] Namen geben kann. 
DANKE!
Ich hab jetzt auch meinen assembler-code mit deinem verglichen, schritt 
für schritt probiert wo der Fehler liegt... heute kompiliert er ihn. 
Verstehe ich nicht ganz aber ok. Was da schief lief bleibt wohl im 
Verborgenen. Danke nochmal!

von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Tobias Weilenbach schrieb:

> wie man sieht, eliminiert er zwar die Zählervariable und prüft die
> Schleifenbedingung lieber über den Pointer, aber gut für solche
> Feinheiten gibt's ja inline-assembler.

... oder -fno-tree-loop-optimize.  4.7 erzeugt:

[avrasm]
duplicate_buffer:
  movw r30,r24
  ldi r24,lo8(6)
.L2:
  ld r25,Z+
  std Z+5,r25
  std Z+11,r25
  subi r24,lo8(-(-1))
  brne .L2
  ret
[/pre]

von Helfender (Gast)


Lesenswert?

Kann dein uC 32 Bit Zugriffe?
Dann kannst vielleicht auch noch so etwas probieren:
(nicht probiert, musst evt. korrigieren, aber vom Prinzip her)

void duplicate_buffer (uint8_t *buf)
{
  uint32_t tmp1;
  uint16_t tmp2;

  tmp1 = (uint32_t *)buf
  (uint32_t *)(buf +  6) = tmp1;
  (uint32_t *)(buf + 12) = tmp1;

  tmp2 = (uint16_t *)(buf + 4);
  (uint16_t *)(buf + 10) = tmp2;
  (uint16_t *)(buf + 16) = tmp2;
}

Sonst halt mit jeweils 16 Bit Zugriffen...

von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Das geht schief wenn man das Alignment über das von der Hardware 
erlaubte erhöht, weg dann zu einer Trap führen kann.

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.