Forum: Compiler & IDEs Code in den Atomic Block verschleppt


Announcement: there is an English version of this forum on EmbDev.net. Posts you create there will be displayed on Mikrocontroller.net and EmbDev.net.
von Andreas R. (andreasr)


Bewertung
0 lesenswert
nicht lesenswert
Dies ist die aufgewärmte Version des Threads 
Beitrag "Schwerer Bug in AVR-GCC 4.1.1" der gesperrt ist. Bislang 
hat die ganz unten vorgeschlagene Lösung mit _asm__ __volatile_ ( "" : 
: "memory" (x)) immer funktioniert. Nun habe ich trotz der clobber Liste 
das Problem.

Folgender Code:
1
volatile uint8_t* pI2CBuff = &txbuffer[i*4];
2
        // Folgende Anweisung verhindert, dass die Berechnung von pI2CBuff in den ATOMIC_BLOCK verzögert wird!
3
        // https://www.mikrocontroller.net/topic/65923
4
        __asm__ __volatile__ ( "" : : "memory" (pI2CBuff));
5
        ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
6
        {
7
          *pI2CBuff++ = tmp[0];
8
          *pI2CBuff++ = tmp[1];
9
          *pI2CBuff++ = tmp[2];
10
          *pI2CBuff++ = tmp[3];
11
        }
Was macht der Compiler (avrgcc 5.4.0, Optimierung -Os) daraus?
1
 16e:  2f b7         in  r18, 0x3f  ; 63
2
 170:  f8 94         cli
3
 172:  f7 01         movw  r30, r14
4
 174:  ee 0f         add  r30, r30
5
 176:  ff 1f         adc  r31, r31
6
 178:  ee 0f         add  r30, r30
7
 17a:  ff 1f         adc  r31, r31
8
 17c:  ef 59         subi  r30, 0x9F  ; 159
9
 17e:  ff 4f         sbci  r31, 0xFF  ; 255
10
 180:  00 83         st  Z, r16
11
 182:  19 83         std  Y+1, r17  ; 0x01
12
 184:  8a 83         std  Y+2, r24  ; 0x02
13
 186:  9b 83         std  Y+3, r25  ; 0x03
14
 188:  2f bf         out  0x3f, r18  ; 63
Das heißt er berechnet die Adresse des Pointers pI2CBuff doch erst im 
Atomic Block, d.h. nach dem cli. Weiß jemand wie man das verhindern 
kann???

von Falk B. (falk)


Bewertung
0 lesenswert
nicht lesenswert
Andreas R. schrieb:
> Das heißt er berechnet die Adresse des Pointers pI2CBuff doch erst im
> Atomic Block, d.h. nach dem cli. Weiß jemand wie man das verhindern
> kann???

Den Pointer selber volatile definieren, nicht nur das Ziel, auf welches 
er zeigt.

volatile uint8_t* volatile pI2CBuff = &txbuffer[i*4];

von Falk B. (falk)


Bewertung
0 lesenswert
nicht lesenswert

von Andreas R. (andreasr)


Bewertung
0 lesenswert
nicht lesenswert
> volatile uint8_t* volatile pI2CBuff = &txbuffer[i*4];


das führt aber dazu dass der Pointer selbst nicht mehr in den Registern 
gehalten werden kann was zu noch ineffizienterem Code führt:
1
      in  r18, 0x3f  ; 63
2
 1ae:  f8 94         cli
3
 1b0:  e9 81         ldd  r30, Y+1  ; 0x01
4
 1b2:  fa 81         ldd  r31, Y+2  ; 0x02
5
 1b4:  af 01         movw  r20, r30
6
 1b6:  4f 5f         subi  r20, 0xFF  ; 255
7
 1b8:  5f 4f         sbci  r21, 0xFF  ; 255
8
 1ba:  5a 83         std  Y+2, r21  ; 0x02
9
 1bc:  49 83         std  Y+1, r20  ; 0x01
10
 1be:  00 83         st  Z, r16
11
 1c0:  e9 81         ldd  r30, Y+1  ; 0x01
12
 1c2:  fa 81         ldd  r31, Y+2  ; 0x02
13
 1c4:  af 01         movw  r20, r30
14
 1c6:  4f 5f         subi  r20, 0xFF  ; 255
15
 1c8:  5f 4f         sbci  r21, 0xFF  ; 255
16
 1ca:  5a 83         std  Y+2, r21  ; 0x02
17
 1cc:  49 83         std  Y+1, r20  ; 0x01
18
 1ce:  10 83         st  Z, r17
19
 1d0:  e9 81         ldd  r30, Y+1  ; 0x01
20
 1d2:  fa 81         ldd  r31, Y+2  ; 0x02
21
 1d4:  af 01         movw  r20, r30
22
 1d6:  4f 5f         subi  r20, 0xFF  ; 255
23
 1d8:  5f 4f         sbci  r21, 0xFF  ; 255
24
 1da:  5a 83         std  Y+2, r21  ; 0x02
25
 1dc:  49 83         std  Y+1, r20  ; 0x01
26
 1de:  80 83         st  Z, r24
27
 1e0:  e9 81         ldd  r30, Y+1  ; 0x01
28
 1e2:  fa 81         ldd  r31, Y+2  ; 0x02
29
 1e4:  af 01         movw  r20, r30
30
 1e6:  4f 5f         subi  r20, 0xFF  ; 255
31
 1e8:  5f 4f         sbci  r21, 0xFF  ; 255
32
 1ea:  5a 83         std  Y+2, r21  ; 0x02
33
 1ec:  49 83         std  Y+1, r20  ; 0x01
34
 1ee:  90 83         st  Z, r25
35
 1f0:  2f bf         out  0x3f, r18  ; 63

von Oliver S. (oliverso)


Bewertung
-1 lesenswert
nicht lesenswert
Falk B. schrieb:
> Den Pointer selber volatile definieren, nicht nur das Ziel, auf welches
> er zeigt.
>
> volatile uint8_t* volatile pI2CBuff = &txbuffer[i*4];

Dann berechnet der den Pointer nicht nur einmal, sondern gleich viermal 
in dem Block. Und das zu Recht...

Andreas R. schrieb:
> Weiß jemand wie man das verhindern
> kann???

Neueren gcc verwenden? Mein kleines Testprogrämmchen mit gcc 10.1 hat 
das Problem nicht. Wobei es vermutlich sehr vom nicht gezeigten Code 
drumherum abhängt, was der gcc draus macht.

Ich würde es mal so probieren:
1
      volatile uint8_t* volatile pI2CBuffv = &txbuffer[i*4];
2
      volatile uint8_t* pI2CBuff = pI2CBuffv;

von Andreas R. (andreasr)


Bewertung
0 lesenswert
nicht lesenswert
Oliver S. schrieb:
> Ich würde es mal so probieren:
>
1
>       volatile uint8_t* volatile pI2CBuffv = &txbuffer[i*4];
2
>       volatile uint8_t* pI2CBuff = pI2CBuffv;
3
>
Das fixt das Problem. Danke!

Compiler will (kann) ich nicht updaten, ist mit Atmel (Microchip) Studio 
verbandelt.

von Andreas R. (andreasr)


Bewertung
0 lesenswert
nicht lesenswert
Oliver S. schrieb:
> Wobei es vermutlich sehr vom nicht gezeigten Code
> drumherum abhängt, was der gcc draus macht.


Der Code "drumherum" ist nicht so spannend; es wird ein ADC Wert und 
eine CRC 16 in einen 4 byte temp buffer geschrieben und dieser danach 
atomar in den Ausgangsbuffer des USI I2C Slaves (Martin Junghans 
jtronics@gmx.de) an eine bestimmte Stelle kopiert.
1
volatile uint8_t txbuffer[buffer_size];      // Transmission buffer to be read from the master
2
3
int __attribute__((OS_main)) main(void)
4
{
5
  InitIO();
6
  
7
    uint8_t i2cBit = (PIN & (1U << PIN_ADDR0)) ? 0 : 1;
8
  usiTwiSlaveInit(I2C_START_ADDR | i2cBit);  // TWI slave init
9
  
10
  InitADC();
11
  sei();
12
    while (1) 
13
    {
14
    for (uint8_t i = 0; i < 2; i++)
15
    {
16
      uint8_t flags;
17
      uint16_t sum;
18
      ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
19
      {
20
        flags = GmvData[i].Flags;
21
        GmvData[i].Flags = flags & ~1;
22
        sum = GmvData[i].Sum;
23
      }
24
      if ((flags & 3) == 3)
25
      {
26
        // new Data and GMV full
27
        uint16_t x = sum / GMV_SIZE;
28
        uint8_t tmp[4];
29
        tmp[0] = (x >> 0) & 0xff;
30
        tmp[1] = (x >> 8) & 0xff;
31
        x = 0xffff;
32
        x = _crc16_update(x, tmp[0]);
33
        x = _crc16_update(x, tmp[1]);
34
        tmp[2] = (x >> 0) & 0xff;
35
        tmp[3] = (x >> 8) & 0xff;
36
        volatile uint8_t* volatile pI2CBuffv = &txbuffer[i*4];
37
        volatile uint8_t* pI2CBuff = pI2CBuffv;
38
        ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
39
        {
40
          *pI2CBuff++ = tmp[0];
41
          *pI2CBuff++ = tmp[1];
42
          *pI2CBuff++ = tmp[2];
43
          *pI2CBuff++ = tmp[3];
44
        }
45
      }
46
    }
47
    }
48
}

von Falk B. (falk)


Bewertung
0 lesenswert
nicht lesenswert
Andreas R. schrieb:
> Der Code "drumherum" ist nicht so spannend;

Ich würde da aber auf JEDEN Fall einen sinnvollen Kommentar ergänzen, 
warum die beiden Pointer exakt so benutzt werden.

Beitrag #6589067 wurde von einem Moderator gelöscht.
von Johann L. (gjlayde) Benutzerseite


Bewertung
1 lesenswert
nicht lesenswert
Andreas R. schrieb:
1
>         __asm__ __volatile__ ( "" : : "memory" (pI2CBuff));
Das ist Käse:  Der Input-Operand hat Constraints "m", "e", "o", "r" und 
"y", ist also die Vereinigung all dieser Constraints.  Das ist kaum das, 
was du willst.

1) Wenn Speicher geklobbert werden soll, genügt ein Clobber auf memory:
1
__asm__ __volatile__ ("" : : : "memory");

2) Wenn nur ein bestimmtes Array-Object geclobbert werden soll, dann
1
__asm__ __volatile__ ("" : "+m" (*pI2CBuff));
was aber natürlich nicht funktioniert, wenn pI2CBuff nur eine Adresse 
ins Array ist.

3) In deinem Fall ist pI2CBuff aber nur eine einfache Variable, die in 
ein Register passt.  Ergo:
1
__asm__ __volatile__ ("" : "+r" (pI2CBuff));
Es geht nämlich nicht um den Inhalt des Arrays, sondern um die 
berechnete Adresse.

: Bearbeitet durch User
von Andreas R. (andreasr)


Bewertung
0 lesenswert
nicht lesenswert
Johann L. schrieb:

> 3) In deinem Fall ist pI2CBuff aber nur eine einfache Variable, die in
> ein Register passt.  Ergo:
1
__asm__ __volatile__ ("" : "+r" (pI2CBuff))

Genau das hab ich gesucht :-))
Vielen Dank!!!
1
 1e6:  2f b7         in  r18, 0x3f  ; 63
2
 1e8:  f8 94         cli
3
 1ea:  00 83         st  Z, r16
4
 1ec:  11 83         std  Z+1, r17  ; 0x01
5
 1ee:  82 83         std  Z+2, r24  ; 0x02
6
 1f0:  93 83         std  Z+3, r25  ; 0x03
7
 1f2:  2f bf         out  0x3f, r18  ; 63

: Bearbeitet durch User

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.