SPI Master Code Optimierung

[nga (Gast)]

Hallo Forum,

ich habe das Problem dass mein AVR-GCC nicht das macht was ich will:
Hier der Code:

//slave.h:
typedef struct {
  const uint8_t slaveSelectPin;
  volatile uint8_t * slaveSelectPort;
  const uint8_t outputSignalPin;
  volatile uint8_t * outputSignalPort;
} slave_t;
uint8_t transmitData(const slave_t *slave, uint8_t data);

//slave.c:
const slave_t slave0= {
  .slaveSelectPin = PB0,
  .slaveSelectPort = &PORTB,
  .outputSignalPin = PD2,
  .outputSignalPort = &PORTD
};
const slave_t slave1= {
  .slaveSelectPin = PB1,
  .slaveSelectPort = &PORTB,
  .outputSignalPin = PD3,
  .outputSignalPort = &PORTD
};
const slave_t slave2= {
  .slaveSelectPin = PB3,
  .slaveSelectPort = &PORTB,
  .outputSignalPin = PB2,
  .outputSignalPort = &PORTB
};

uint8_t transmitData(const slave_t *slave, uint8_t data) {
  uint8_t tmp;
  *(slave->slaveSelectPort) &= ~(1<<slave->slaveSelectPin);
  tmp = spi_transmit(data);
  *(slave->slaveSelectPort) |= (1<<slave->slaveSelectPin);
  return tmp;
}

Dieser Code erzeugt eine wahnsinnig lange ASM-Funktion:

000000e2 <transmitData>:
  e2:  cf 93         push  r28
  e4:  df 93         push  r29
  e6:  ec 01         movw  r28, r24
  e8:  86 2f         mov  r24, r22
  ea:  0e 94 60 00   call  0xc0  ; 0xc0 <spi_transmit>
  ee:  e9 81         ldd  r30, Y+1  ; 0x01
  f0:  fa 81         ldd  r31, Y+2  ; 0x02
  f2:  90 81         ld  r25, Z
  f4:  21 e0         ldi  r18, 0x01  ; 1
  f6:  30 e0         ldi  r19, 0x00  ; 0
  f8:  08 80         ld  r0, Y
  fa:  02 c0         rjmp  .+4        ; 0x100 <transmitData+0x1e>
  fc:  22 0f         add  r18, r18
  fe:  33 1f         adc  r19, r19
 100:  0a 94         dec  r0
 102:  e2 f7         brpl  .-8        ; 0xfc <transmitData+0x1a>
 104:  92 2b         or  r25, r18
 106:  90 83         st  Z, r25
 108:  df 91         pop  r29
 10a:  cf 91         pop  r28
 10c:  08 95         ret

Man beachte dass der Teil zum löschen des Pins auskommentiert ist, sonst 
wäre die gesamte Sequenz hinter dem call spi_transmit noch einma davor.

Nun meine Frage:
Kann ich den Compiler irgendwie dazu bewegen meinen Code mit dem Umweg 
über die "Konstante" genauso zu übersetzen wie wenn ich eine direkte 
Portzuweisung machen würde?

mfg
nga

[Jörg W. (dl8dtl) (Moderator)]

Wenn du die Funktion "static" machst, dann kann er sie inline
ersetzen.  Ansonsten kann der Compiler natürlich innerhalb von
transmitData() nicht ahnen, mit welchen Parametern sie irgendwann
konkret gerufen wird.  Wäre höchstens, dass die linker relaxations
das noch rausreißen können, aber das kann ich mir angesichts der
Komplexität der notwendigen Optimierung schwer vorstellen.

[nga (Gast)]

Tatsache, das mit dem static hilft schon einmal.
Danke schon mal.
Würde evtl ein C++ Compiler besseren Code erzeugen (wegen echten 
Konstanten)? Evtl auch mit templates?
eigentlich sollte es ein C-Programm werden...

Oder gibt es andere Vorschläge?

[Jörg W. (dl8dtl) (Moderator)]

nga schrieb:
> Würde evtl ein C++ Compiler besseren Code erzeugen (wegen echten
> Konstanten)?

Auch nur unter vergleichbaren Umständen: wenn die Adressen der
jeweiligen Ports und Portbits zur Compilezeit nicht bekannt sind, dann
kann der Compiler natürlich keine IN/OUT-Befehle generieren (für die
die Port- und Bitnummer ja im Opcode stehen), sondern nur LDS/STS.

[Eric B. (beric)]

Wie wird es wenn du alles in ein Array packst und über den Index 
auswählst? Das hat noch den entscheidenden Vorteil, dass der Aufrufer 
nix mit den slave_t zu tun haben muss.

Ausserdem kann das Bit-shiften in der Configuration stattfinden.

//slave.h:
#define SLAVE_0 0
#define SLAVE_1 1
#define SLAVE_2 2

uint8_t transmitData(uint8_t id, uint8_t data);

//slave.c:
typedef struct
{
  const    uint8_t   slaveSelectPin;
  volatile uint8_t * slaveSelectPort;
  const    uint8_t   outputSignalPin;
  volatile uint8_t * outputSignalPort;
} slave_t;

const slave_t slave[]=
{
  { 1 << PB0, &PORTB, 1 << PD2, &PORTD },
  { 1 << PB1, &PORTB, 1 << PD3, &PORTD },
  { 1 << PB3, &PORTB, 1 << PB2, &PORTB }
};

 
uint8_t transmitData(uint8_t id, uint8_t data)
{
  uint8_t tmp;

  slave[id].slaveSelectPort &= ~(slave[id].slaveSelectPin);
  tmp = spi_transmit(data);
  slave[id].slaveSelectPort |=   slave[id].slaveSelectPin;

  return tmp;
}

EDIT:
Zur Run-Time Optimierung könnte man dann noch switch/casen über den 
index:

uint8_t transmitData(uint8_t id, uint8_t data)
{
  uint8_t tmp;

  switch(id)
  {
    case 0:
      slave[0].slaveSelectPort &= ~(slave[0].slaveSelectPin);
      tmp = spi_transmit(data);
      slave[0].slaveSelectPort |=   slave[0].slaveSelectPin;
    break;

    case 1:
      slave[1].slaveSelectPort &= ~(slave[1].slaveSelectPin);
      tmp = spi_transmit(data);
      slave[1].slaveSelectPort |=   slave[1].slaveSelectPin;
    break;

    case 2:
      slave[2].slaveSelectPort &= ~(slave[2].slaveSelectPin);
      tmp = spi_transmit(data);
      slave[2].slaveSelectPort |=   slave[2].slaveSelectPin;
    break;

    default:
      tmp = ~tmp;
    break;
  }

  return tmp;
}

[Peter D. (peda)]

nga schrieb:
> Dieser Code erzeugt eine wahnsinnig lange ASM-Funktion:

Nicht nur das.
Du erzeugst 3 Structs a 6 Byte, die müssen befüllt, übergeben und wieder 
auseinander gefummelt werden.
Das kostet natürlich.

Du mußt schon dem Compiler etwas entgegenkommen und möglichts oft 
Konstanten benutzen.

Es reicht außerdem völlig, wenn man unterschiedliche Select-Pins hat.
Und da bietet es sich an, alle auf den selben Port zu legen und nur die 
Maske zu übergeben:

#include "sbit.h"

#define SSCK            PORT_B0
#define SMOSI           PORT_B1
#define SMISO_in        PIN_B2

#define SSEL_PORT       PORTB
#define SEL_MASK0       0x08
#define SEL_MASK1       0x10
#define SEL_MASK2       0x20
#define SEL_ALL         (SEL_MASK0 | SEL_MASK1 | SEL_MASK2)

static uint8_t sw_spi( uint8_t val )
{
  for( uint8_t i = 8; i; i-- ){
    SMOSI = 0;
    if( val & 0x80 )
      SMOSI = 1;
    SSCK = 1;
    SSCK = 0;
    val <<= 1;
    if( SMISO_in )
      val++;
  }
  return val;
}

void sw_spi_block( uint8_t *pt, uint8_t length, uint8_t ssel_mask )
{
  SSEL_PORT ^= ssel_mask;
  while( length-- ){
    *pt = sw_spi( *pt );
    pt++;
  }
  SSEL_PORT |= SEL_ALL;
}

Anbei das Listing.

[nga (Gast)]

kurzes Update:

Peter Dannegger schrieb:
> Du mußt schon dem Compiler etwas entgegenkommen und möglichts oft
> Konstanten benutzen.

Das ist natürlich klar. Ich habe es ja versucht mittels const den 
Compiler die Variable als Konstante zu sehen, leider ist C da nicht so 
klar...

Peter Dannegger schrieb:
> Es reicht außerdem völlig, wenn man unterschiedliche Select-Pins hat.
> Und da bietet es sich an, alle auf den selben Port zu legen und nur die
> Maske zu übergeben

Ich stimme dir voll zu. Aber in dem struct werden auch noch die Daten 
gespeichert, deswegen wollte ich die 'Konstanten' dort speichern.

Bleibt abschließend zu sagen, dass

Eric B. schrieb:
> Zur Run-Time Optimierung könnte man dann noch switch/casen über den
> index:

das hier genau wie gewollt funktioniert hat:
es werden sbi und cbi Instruktionen erzeugt!

Danke an alle

[Eric B. (beric)]

nga schrieb:
> Ich stimme dir voll zu. Aber in dem struct werden auch noch die Daten
> gespeichert, deswegen wollte ich die 'Konstanten' dort speichern.

Da liegt dann der grundsätzliche Fehler: Konfiguration (konstant -> kann 
ins ROM/Flash) und Nutzdaten (variabel -> muss ins RAM) soll man nicht 
mischen.