Warum wird Variable wegoptimiert?

[Walter T. (nicolas)]

Guten Morgen,
ich habe gerade ein Brett vorm Kopf und suche jemanden mit Kneifzange. 
Ich nutze den ARM-GCC und in einer Funktion wird die Variable "value" 
wegoptimiert - und ich sehe gerade nicht warum.

void glcd_putsth(char value,uint8_t RS) {
  // Neuimplementierung write-Zyklus gemaess Datenblatt S. 14
  
  #ifdef GLCD_NOBUSYFLAG
    delay_us(100);
  #else
    delay_us(100);
    //glcd_waitWhileBusy();
  #endif
  
  //GLCD_DATA_DDR = 0xFF;
  
  ClearBits(GLCD_CTRL_GPIO,GLCD_ECLK); //GLCD_CTRL_PORT &= ~(1 << GLCD_ECLK); // Enable Low
  ClearBits(GLCD_CTRL_GPIO,GLCD_RW);   //GLCD_CTRL_PORT &= ~(1 << GLCD_RW);   // Write
  if(RS)                               
    SetBits(GLCD_CTRL_GPIO,GLCD_DI);   //GLCD_CTRL_PORT |=  (1 << GLCD_DI); // Data
  else
    ClearBits(GLCD_CTRL_GPIO,GLCD_DI); //GLCD_CTRL_PORT &= ~(1 << GLCD_DI); // Instruction
  delay_us(0.450); // max(t_ASU,t_t_WL)
  SetBits(GLCD_CTRL_GPIO,GLCD_ECLK);   //GLCD_CTRL_PORT |=  (1 << GLCD_ECLK);

  // geht:
  //GPIO_ResetBits(GLCD_DATA_GPIO, 0xFF<<GLCD_DATA_OFFSET);
  //GPIO_SetBits(GLCD_DATA_GPIO,  value<<GLCD_DATA_OFFSET);

  // geht nicht:
  //ClearBits(GLCD_DATA_GPIO, ~value<<GLCD_DATA_OFFSET);
  //SetBits(GLCD_DATA_GPIO,  value<<GLCD_DATA_OFFSET);

  // geht nicht:
  uint32_t temp = ((((uint16_t) value)<<GLCD_DATA_OFFSET)& 0x0000FFFF) | ((0x000000FF<<(GLCD_DATA_OFFSET+0x10)) & 0xFFFF0000);
  GLCD_DATA_GPIO->BSRR = temp;


  delay_us(0.450);  // max(t_DSU, t_WH)
  ClearBits(GLCD_CTRL_GPIO,GLCD_ECLK); // GLCD_CTRL_PORT &= ~(1 << GLCD_ECLK); // Enable Low
}

Entsprechende Zeile aus dem Listing:

void glcd_putsth(char value,uint8_t RS) {
 8000ac2:  460e        mov  r6, r1
  // Neuimplementierung write-Zyklus gemaess Datenblatt S. 14
  
  #ifdef GLCD_NOBUSYFLAG
    delay_us(100);
  #else
    delay_us(100);
 8000ac4:  fbb4 f4f0   udiv  r4, r4, r0
 8000ac8:  2164        movs  r1, #100  ; 0x64
 8000aca:  4361        muls  r1, r4
 8000acc:  2203        movs  r2, #3
 8000ace:  fbb1 f0f2   udiv  r0, r1, r2
 8000ad2:  f000 f9a5   bl  8000e20 <__aeabi_ui2d>
 8000ad6:  a320        add  r3, pc, #128  ; (adr r3, 8000b58 <L_LOOPUS_203+0x14>)
 8000ad8:  e9d3 2300   ldrd  r2, r3, [r3]
 8000adc:  f000 fa16   bl  8000f0c <__aeabi_dmul>
 8000ae0:  f000 fc26   bl  8001330 <__aeabi_d2uiz>

08000ae4 <L_LOOPUS_161>:
 8000ae4:  3801        subs  r0, #1
 8000ae6:  f47f affd   bne.w  8000ae4 <L_LOOPUS_161>
    //glcd_waitWhileBusy();
  #endif
  
  //GLCD_DATA_DDR = 0xFF;
  
  ClearBits(GLCD_CTRL_GPIO,GLCD_ECLK); //GLCD_CTRL_PORT &= ~(1 << GLCD_ECLK); // Enable Low
 8000aea:  4b21        ldr  r3, [pc, #132]  ; (8000b70 <L_LOOPUS_203+0x2c>)
 8000aec:  f04f 7000   mov.w  r0, #33554432  ; 0x2000000
  ClearBits(GLCD_CTRL_GPIO,GLCD_RW);   //GLCD_CTRL_PORT &= ~(1 << GLCD_RW);   // Write
 8000af0:  f04f 7180   mov.w  r1, #16777216  ; 0x1000000
    //glcd_waitWhileBusy();
  #endif
  
  //GLCD_DATA_DDR = 0xFF;
  
  ClearBits(GLCD_CTRL_GPIO,GLCD_ECLK); //GLCD_CTRL_PORT &= ~(1 << GLCD_ECLK); // Enable Low
 8000af4:  6118        str  r0, [r3, #16]
  ClearBits(GLCD_CTRL_GPIO,GLCD_RW);   //GLCD_CTRL_PORT &= ~(1 << GLCD_RW);   // Write
 8000af6:  6119        str  r1, [r3, #16]
  if(RS)                               
 8000af8:  b10e        cbz  r6, 8000afe <L_LOOPUS_161+0x1a>
    SetBits(GLCD_CTRL_GPIO,GLCD_DI);   //GLCD_CTRL_PORT |=  (1 << GLCD_DI); // Data
 8000afa:  2280        movs  r2, #128  ; 0x80
 8000afc:  e001        b.n  8000b02 <L_LOOPUS_161+0x1e>
  else
    ClearBits(GLCD_CTRL_GPIO,GLCD_DI); //GLCD_CTRL_PORT &= ~(1 << GLCD_DI); // Instruction
 8000afe:  f44f 0200   mov.w  r2, #8388608  ; 0x800000
 8000b02:  611a        str  r2, [r3, #16]
  delay_us(0.450); // max(t_ASU,t_t_WL)
 8000b04:  4620        mov  r0, r4
 8000b06:  f000 f98b   bl  8000e20 <__aeabi_ui2d>
 8000b0a:  a315        add  r3, pc, #84  ; (adr r3, 8000b60 <L_LOOPUS_203+0x1c>)
 8000b0c:  e9d3 2300   ldrd  r2, r3, [r3]
 8000b10:  f000 f9fc   bl  8000f0c <__aeabi_dmul>
 8000b14:  2200        movs  r2, #0
 8000b16:  4b17        ldr  r3, [pc, #92]  ; (8000b74 <L_LOOPUS_203+0x30>)
 8000b18:  f000 fb22   bl  8001160 <__aeabi_ddiv>
 8000b1c:  a30e        add  r3, pc, #56  ; (adr r3, 8000b58 <L_LOOPUS_203+0x14>)
 8000b1e:  e9d3 2300   ldrd  r2, r3, [r3]
 8000b22:  f000 f9f3   bl  8000f0c <__aeabi_dmul>
 8000b26:  f000 fc03   bl  8001330 <__aeabi_d2uiz>

08000b2a <L_LOOPUS_194>:
 8000b2a:  3801        subs  r0, #1
 8000b2c:  f47f affd   bne.w  8000b2a <L_LOOPUS_194>
  SetBits(GLCD_CTRL_GPIO,GLCD_ECLK);   //GLCD_CTRL_PORT |=  (1 << GLCD_ECLK);
 8000b30:  490f        ldr  r1, [pc, #60]  ; (8000b70 <L_LOOPUS_203+0x2c>)
  // geht nicht:
  //ClearBits(GLCD_DATA_GPIO, ~value<<GLCD_DATA_OFFSET);
  //SetBits(GLCD_DATA_GPIO,  value<<GLCD_DATA_OFFSET);

  // geht nicht:
  uint32_t temp = ((((uint16_t) value)<<GLCD_DATA_OFFSET)& 0x0000FFFF) | ((0x000000FF<<(GLCD_DATA_OFFSET+0x10)) & 0xFFFF0000);
 8000b32:  022d        lsls  r5, r5, #8
  if(RS)                               
    SetBits(GLCD_CTRL_GPIO,GLCD_DI);   //GLCD_CTRL_PORT |=  (1 << GLCD_DI); // Data
  else
    ClearBits(GLCD_CTRL_GPIO,GLCD_DI); //GLCD_CTRL_PORT &= ~(1 << GLCD_DI); // Instruction
  delay_us(0.450); // max(t_ASU,t_t_WL)
  SetBits(GLCD_CTRL_GPIO,GLCD_ECLK);   //GLCD_CTRL_PORT |=  (1 << GLCD_ECLK);
 8000b34:  f44f 7300   mov.w  r3, #512  ; 0x200
  // geht nicht:
  //ClearBits(GLCD_DATA_GPIO, ~value<<GLCD_DATA_OFFSET);
  //SetBits(GLCD_DATA_GPIO,  value<<GLCD_DATA_OFFSET);

  // geht nicht:
  uint32_t temp = ((((uint16_t) value)<<GLCD_DATA_OFFSET)& 0x0000FFFF) | ((0x000000FF<<(GLCD_DATA_OFFSET+0x10)) & 0xFFFF0000);
 8000b38:  f045 457f   orr.w  r5, r5, #4278190080  ; 0xff000000
  // GLCD_DATA_OFFSET = 8
  GLCD_DATA_GPIO->BSRR = temp;
 8000b3c:  f5a1 6280   sub.w  r2, r1, #1024  ; 0x400
  if(RS)                               
    SetBits(GLCD_CTRL_GPIO,GLCD_DI);   //GLCD_CTRL_PORT |=  (1 << GLCD_DI); // Data
  else
    ClearBits(GLCD_CTRL_GPIO,GLCD_DI); //GLCD_CTRL_PORT &= ~(1 << GLCD_DI); // Instruction
  delay_us(0.450); // max(t_ASU,t_t_WL)
  SetBits(GLCD_CTRL_GPIO,GLCD_ECLK);   //GLCD_CTRL_PORT |=  (1 << GLCD_ECLK);
 8000b40:  610b        str  r3, [r1, #16]

  // geht nicht:
  uint32_t temp = ((((uint16_t) value)<<GLCD_DATA_OFFSET)& 0x0000FFFF) | ((0x000000FF<<(GLCD_DATA_OFFSET+0x10)) & 0xFFFF0000);
  // temp wird zu 0xff000000, d.h. value wird als 0 gesehen
  // GLCD_DATA_OFFSET = 8
  GLCD_DATA_GPIO->BSRR = temp;
 8000b42:  6115        str  r5, [r2, #16]

08000b44 <L_LOOPUS_203>:


  delay_us(0.450);  // max(t_DSU, t_WH)
 8000b44:  3801        subs  r0, #1
 8000b46:  f47f affd   bne.w  8000b44 <L_LOOPUS_203>
  ClearBits(GLCD_CTRL_GPIO,GLCD_ECLK); // GLCD_CTRL_PORT &= ~(1 << GLCD_ECLK); // Enable Low
 8000b4a:  4b09        ldr  r3, [pc, #36]  ; (8000b70 <L_LOOPUS_203+0x2c>)
 8000b4c:  f04f 7000   mov.w  r0, #33554432  ; 0x2000000
 8000b50:  6118        str  r0, [r3, #16]
 8000b52:  bd70        pop  {r4, r5, r6, pc}
 8000b54:  f3af 8000   nop.w
 8000b58:  88b97785   .word  0x88b97785
 8000b5c:  3feec4ef   .word  0x3feec4ef
 8000b60:  cccccccd   .word  0xcccccccd
 8000b64:  3fdccccc   .word  0x3fdccccc
 8000b68:  20000000   .word  0x20000000
 8000b6c:  000f4240   .word  0x000f4240
 8000b70:  40011000   .word  0x40011000
 8000b74:  40080000   .word  0x40080000

08000b78 <glcd_area>:
}

GLCD_DATA_OFFSET ist 8, Ergebnis ist, daß alle Pins ständig high sind.

So langsam setzt bei mir die Codeblindheit ein und ich sehe Fehler, wo 
keine sind. Kann mir jemand kurz helfen?

Viele Grüße
W.T.

[(prx) A. K. (prx)]

Bring mal den ganzen Code zu der Funktion. Da fehlt was am Anfang. 
Verlass dich nicht darauf, dass die Quellcodezeilen immer an der 
richtigen Stelle eingefügr werden, das geht oft daneben.

[Ralf G. (ralg)]

Welchen Wert hat denn 'GLCD_DATA_OFFSET'? Bist du dir mit '8' sicher. Im 
Code ist alles auskommentiert.

[(prx) A. K. (prx)]

Lass das mit den Fliesskommazahlen bei delay_us lieber. Diese Funktion 
ist für so kurze Zeiten problematisch. Woher stammt die?

Bei dem Delay kommt mir noch was komisch vor:

08000b2a <L_LOOPUS_194>:
 8000b2a:  3801        subs  r0, #1
 8000b2c:  f47f affd   bne.w  8000b2a <L_LOOPUS_194>

R0 ist jetzt 0. Nirgendwo hier wird R0 verändert. R0 bleibt also 0.

08000b44 <L_LOOPUS_203>:
 8000b44:  3801        subs  r0, #1
 8000b46:  f47f affd   bne.w  8000b44 <L_LOOPUS_203>

Wenn dieser Code mit R0=0 anläuft, dann ist Geduld angesagt.

[Walter T. (nicolas)]

Danke für's Lesen.

Ich hoffe, ich habe jetzt alle Teile aus dem Listing richtig 
zusammengeklaubt (list.lss). Wer sich das wirklich antun will kann auch 
einen Blick ins komplette Listing werfen (lang_und_zaeh.lss).

Daß GLCD_DATA_OFFSET 8 ist bin ich mir sicher - daß mir da ein zweites 
Makro hineinfuhrwerkt war ja auch mein erster Gedanke. Und immerhin 
funktioniert ja noch die Variante mit der Standard-Peripheral-Library 
meines STM32, also muß die Pinzuordnung schon einmal stimmen.

[(prx) A. K. (prx)]

Walter Tarpan schrieb:
> Daß GLCD_DATA_OFFSET 8 ist bin ich mir sicher

Der Code fürs BSSR passt dazu und ist auch völlig korrekt:

 8000b32:  022d        lsls  r5, r5, #8
 8000b38:  f045 457f   orr.w  r5, r5, #0xff000000
 8000b42:  6115        str  r5, [r2, #16]

Ich frage mich eher, ob das Timing stimmt. Einerseits die 
Delay-Funktion, wie beschrieben. Andererseits ist GPIO_xxxBits viel 
langsamer als die anderen nicht funktionierenden Varianten.

[Walter T. (nicolas)]

A. K. schrieb:
> [...]
>
> Ich frage mich eher, ob das Timing stimmt. Einerseits die
> Delay-Funktion, wie beschrieben. Andererseits ist GPIO_xxxBits viel
> langsamer als die anderen nicht funktionierenden Varianten.

Das war auch mein erster Gedanke. Deswegen hatte ich bei Vorversuchen 
auch viele nops eingefügt.

A. K. schrieb:
> Lass das mit den Fliesskommazahlen bei delay_us lieber. Diese Funktion
> ist für so kurze Zeiten problematisch. Woher stammt die?

Damit hast Du mich auf den rechten Weg gebracht: Die delay-Routine habe 
ich in den Weiten des WWWs gesucht und in eine Makro-Form gebracht, weil 
ich ARM-Assembler nicht kann. Und sie ist so definiert:

#define delay_us(delayval) delay_us_( (uint32_t)( (SystemCoreClock) / 1000000 *delayval/3 * TUNE_FAC) )

#define delay_us_(delayval)  \
  asm volatile (\
    "L_LOOPUS_%=:     \n\t" \
    "subs %0, %0, #1  \n\t" \
    "bne  L_LOOPUS_%= \n\t" \
    :  /* no outputs */ : "r" (delayval) )

Wenn ich die letzte Zeile durch 9 "nop"s ersetze geht alles. Was auch 
erklärt, warum sich der Debugger ständig aufgehängt hat. Ich vermisse 
irgendwie die hervorragend funktionierenden _delay_xx vom AVR-GCC.

Vielen Dank für die hilfreichen Kommentare!

[(prx) A. K. (prx)]

Bei dieser Delay-Funktion sollte an Stelle der Variablen SystemCoreClock 
eine Konstante stehen. Andernfalls gibts, wie hier geschehen, eine 
Berechnung zur Laufzeit und kurze Delays werden grottenfalsch.

Ausserdem kann es je nach exaktem Typ des Controllers und dem jeweiligen 
Alignment der Schleife gewisse Schmutzeffekte geben, so dass das exakt 
gleiche Delay an verschiedenen Stellen im Programm deutlich 
unterschiedlich ausfällt.

[Johann L. (gjlayde)]

Walter Tarpan schrieb:
> #define delay_us_(delayval)  \
>   asm volatile (\
>     "L_LOOPUS_%=:     \n\t" \
>     "subs %0, %0, #1  \n\t" \
>     "bne  L_LOOPUS_%= \n\t" \
>     :  /* no outputs */ : "r" (delayval) )
> [/c]

Das ist falsch, denn delayval wird verändert! Die Constraint könnte 
sein:

>     :  "=r" (delayval) : "0" (delayval)

Allerdings muß delayval dann ein LVALUE sein.  Anstatt des Makros würde 
ich eine inline-Funktion machen.

Ich kenn zwar kein ARM-Code, aber das schein nicht zu funktionieren wenn 
delayval = 0 ist.  Du könntest

static inline __attribute__((__always_inline__))
void delay_us (unsigned delayval)
{
   if (delayval)
       __asm volatile ("0:\n\t"
                       "subs %0, %0, #1  \n\t"
                       "bne  0b"
                       : "+r" (delayval));
}

[Carsten H. (blaubaer77)]

Hallöchen,

in der Codezeile:

 uint32_t temp = ((((uint16_t) value)<<GLCD_DATA_OFFSET)& 0x0000FFFF) | ((0x000000FF<<(GLCD_DATA_OFFSET+0x10)) & 0xFFFF0000);

der Teil:

(((uint16_t) value)<<GLCD_DATA_OFFSET)& 0x0000FFFF)

ergibt doch immer 0.
Der Wert von Value wird um 8 Bit nach oben verschoben und dann werden 
die oberen Bits ausmaskiert. Damit kann der Compiler die Variable 
wegoptimieren.

Gruß Carsten.

[Walter T. (nicolas)]

Carsten H. schrieb:
> Der Wert von Value wird um 8 Bit nach oben verschoben und dann werden
> die oberen Bits ausmaskiert. Damit kann der Compiler die Variable
> wegoptimieren.

Also bei mir landet ein 8-Bit-Wert beim Verschieben um 8 Bit immer noch 
im 16-Bit-Bereich. Und auf meinen µP auch. Also läuft das alles 
problemlos seit ein paar Stunden.

Johann L. schrieb:
> Ich kenn zwar kein ARM-Code, aber das schein nicht zu funktionieren wenn
> delayval = 0 ist.  Du könntest
> static inline __attribute__((_always_inline_))
> void delay_us (unsigned delayval)
> {
>    if (delayval)
>        __asm volatile ("0:\n\t"
>                        "subs %0, %0, #1  \n\t"
>                        "bne  0b"
>                        : "+r" (delayval));
> }

Mit dem delay muß ich mal ausprobieren. Eine gute Delay-Funktion ist 
nämlich immer noch Mangelware.

[axel (Gast)]

Das müssten schon 16 Bit sein, erst dann ist der Wert von value komplett 
durch die Maskierung weg.

[(prx) A. K. (prx)]

Johann L. schrieb:
> Das ist falsch, denn delayval wird verändert!

Das ist der Grund, weshalb die zweite 0,450er Schleife stecken bleibt.

> Ich kenn zwar kein ARM-Code, aber das schein nicht zu funktionieren wenn
> delayval = 0 ist.

Korrekt.

[(prx) A. K. (prx)]

Carsten H. schrieb:
> Der Wert von Value wird um 8 Bit nach oben verschoben und dann werden
> die oberen Bits ausmaskiert.

Wie ich oben schon schrieb ist dieser Code völlig korrekt.

Wenn dich das (uint16_t)value irritiert: Der Cast ist für'n A..... Ich 
würde aber empfehlen, als Parameter kein "char" zu verwenden. Das könnte 
negativ sein und dann geht es bei einem Offset < 8 den Bach runter.

axel schrieb:
> Das müssten schon 16 Bit sein, erst dann ist der Wert von value komplett
> durch die Maskierung weg.

Allerdings würde man dann vmtl. kein 32-Bit I/O-Register vorfinden.

[Walter T. (nicolas)]

A. K. schrieb:
>
> Wenn dich das (uint16_t)value irritiert: Der Cast ist für'n A..... Ich
> würde aber empfehlen, als Parameter kein "char" zu verwenden. Das könnte
> negativ sein und dann geht es bei einem Offset < 8 den Bach runter.
>

Bitte den obigen Code nicht zu ernst nehmen: Der Cast ist wirklich 
Überflüssig. Es ist dieser Code, der entsteht, wenn man nicht mehr 
weiter weiß (Variante1 funktioniert, Variante2 nicht und beide sehen 
völlig gleich aus!) und das Assembler-Listing noch nicht richtig lesen 
kann und dann alles explizit und fix castet, damit ja keine merkwürdige 
Integer-Promotion-Regel mehr dazwischenpaßt.

[Walter T. (nicolas)]

Ich habe jetzt mal die Anregung bezüglich der Delay-Sachen umgesetzt. 
Meine delay.h sieht jetzt so aus:

#ifndef _delay_h_
#define _delay_h_

#define TUNE_FAC 0.96154
#define SYSCLK 24000000ULL

#include <stdint.h>

#define delay_us(delayval) delay_us_( (uint32_t)( (SYSCLK) / 3000000ULL *delayval * TUNE_FAC) )

static inline void delay_us_(uint64_t delayval) {
  if(delayval) {
    asm volatile (\
      "L_LOOPUS_%=:     \n\t" \
      "subs %0, %0, #1  \n\t" \
      "bne  L_LOOPUS_%= \n\t" \
      :  "+r" (delayval) );
   }
  else {
    asm volatile("nop");
  }
}

#endif

und zum Testen gibt es eine einfache Routine:

void delay_test(void) {
  GPIO_InitTypeDef GPIO_InitStructure;
  RCC_APB2PeriphClockCmd(RCC_APB2Periph_GPIOB,ENABLE);

  GPIO_InitStructure.GPIO_Mode  = GPIO_Mode_Out_PP;
  GPIO_InitStructure.GPIO_Pin   = GPIO_Pin_2;
  GPIO_InitStructure.GPIO_Speed = GPIO_Speed_50MHz;
  GPIO_Init(GPIOB, &GPIO_InitStructure);

  printf("Test delay \n PB2 Pulsabstand 3µs\n Beenden durch Knopfdruck\a");

  while(1) {
    GPIOB->BSRR = GPIO_Pin_2;
    delay_us(3);
    GPIOB->BSRR = GPIO_Pin_2<<16;
    delay_us(6);
    if(key_getstroke()) return; // Liest Tastendruck ein
  }
}

Wirft man einen Blick aufs Oszilloskop (sorry für den schlechten 
Screenshot, die sphärische Verzerrung bekomme ich nicht heraus) sieht 
man, daß der vorgeschlagene Delay sehr zuverlässig funktioniert (die 
Verschiedungen kommen vom SysTick, ist der aus sind alle schön gleich 
breit).

Das ist genau das, was ich gesucht habe. Vielen Danke für die Hilfe!

[Rufus Τ. F. (rufus)]

Walter Tarpan schrieb:
> Wirft man einen Blick aufs Oszilloskop (sorry für den schlechten
> Screenshot, die sphärische Verzerrung bekomme ich nicht heraus)

Das ist ein Witz, oder? Der Screenshot ist --abgesehen davon, daß 
eigentlich das Oszilloskop so etwas selbst anfertigen können sollte-- 
doch vollkommen in Ordnung.

[Walter T. (nicolas)]

Rufus Τ. Firefly schrieb:
> Das ist ein Witz, oder?

Ja.

[(prx) A. K. (prx)]

Bis zum Cortex M51 mit 64-Bit Core ist es zwar noch ein Weilchen hin, 
aber der kluge Mann sorgt vor. ;-)