2
votes

I've noticed some really odd behaviour in my software. It's taken literally months to track down.

I'm using ST's USB Virtual COM Port example code as part of my project, and occasionally the binary that is produced just totally fails to work when compiling with -Os. If I change something unrelated then suddenly it'll spring back to life - and compiling without -Os everything always works perfectly.

I tracked it down to USB initialisation code that causes an interrupt handler to be called repeatedly. The code is this:

#define     __IO    volatile  
#define RegBase  (0x40005C00L)  /* USB_IP Peripheral Registers base address */
#define CNTR    ((__IO unsigned *)(RegBase + 0x40))
#define ISTR    ((__IO unsigned *)(RegBase + 0x44))
#define _SetCNTR(wRegValue)  (*CNTR   = (uint16_t)wRegValue)
#define _SetISTR(wRegValue)  (*ISTR   = (uint16_t)wRegValue)

  _SetISTR(0); // Clear all pending interrupts
  wInterrupt_Mask = IMR_MSK; // 7168
  _SetCNTR(wInterrupt_Mask); // enable interrupts for WKUP/RESET/SUSP

This would be fine, but GCC (through many different versions, although 4.8.4 at the moment) produces this code:

r1 = 7168
r2 = 0x40005c44 (USB_ISTR)
r3 = 0x40005c40 (USB_CNTR)
r4 = 0
   14b42:   6019        str r1, [r3, #0]    ; USB_CNTR = 7168
   14b44:   490b        ldr r1, [pc, #44]   ; r1 = &wInterrupt_Mask
   14b46:   6014        str r4, [r2, #0]    <--------------- hangs here - USB_ISTR = 0 (USB_ISTR)

So these statements are in totally the wrong order, and it screws everything up. Both registers are even marked as volatile!

Even if I do this:

  _SetISTR(0); // Clear all pending interrupts
  _SetISTR(0);
  _SetISTR(0);
  _SetISTR(0);
  _SetISTR(0); // I really mean it GCC
  wInterrupt_Mask = IMR_MSK; // 7168
  _SetCNTR(wInterrupt_Mask); // enable interrupts for WKUP/RESET/SUSP

I get this:

   12d60:   601c        str r4, [r3, #0]
   12d62:   6011        str r1, [r2, #0] ; CNTR
   12d64:   601c        str r4, [r3, #0]
   12d66:   601c        str r4, [r3, #0]
   12d68:   601c        str r4, [r3, #0]
   12d6a:   601c        str r4, [r3, #0]

So while it fixes the problem, GCC has still re-ordered the write in a totally bizarre way (for no gain), and I'm not totally certain that it won't decide to set CNTR first at some point in the future.

So - why has GCC done this, and what can I do to avoid it? Obviously arbitrary re-ordering of register writes on an embedded system is pretty bad news. Is there a nice way to fix it in this case, and is there a way to be sure that it's not re-ordering writes anywhere else?

thanks!

1
Ok, so it seems you can add asm volatile ("" : : : "memory"); between writes, but I still don't get it. Another post states 'GCC is forbidden from reordering volatile loads and store memory accesses with respect to each other' so could this be a compiler bug? - Gordon Williams
Please see Nine ways to break your systems code using volatile. Personally, I am really unsure what the 'C' standard zealots might say about this. Probably they would argue for days and you would have no clear answer. Note: that volatile unsigned * is different than unsigned * volatile. It would be nice to see that post for the devilish details. Why not put the barrier and move on? - artless noise
Are you sure you don't have anything like #define volatile /* nothing */ in your code? I have never experienced such behavior as you describe - for the last few years of using GCC and -Os optimization. ST's code is usually a complete crap, so I wouldn't be much surprised if they screwed something in their magic macros. - Freddie Chopin
I even took your code for a test, and - obviously - it works exactly as expected, all the stores of 0 are before the store of 7168 (optimization -Os). Don't get me wrong, I usually see someone claiming to found a bug in compiler a few times a month, and for the past few years each of these discoveries was a "PEBKAC", not GCC's fault... always. - Freddie Chopin
Further note (and I'm not trying to be snarky): this is why the Short, Self Contained, Correct (Compilable), Example is important. It takes a specific build environment out of context, and if failing to produce one, the culprit has just been found... - unixsmurf

1 Answers

-1
votes

I know this is an old post, but I would wonder how was wInterrupt_Mask declared? Was it declared volatile, too? And if not, would that allow GCC to reorder it arbitrarily?