Compiler optimization for size (-Os) eats my code. Why?

[tomchr]

I'm using the latest MCUXpresso IDE and am writing a bit of code to read a control bus. Nothing fancy. I debounce the bus for a bit of extra EMI/RFI robustness. The code is basically straight from Ganssle, J.G. (2004) A Guide to Debouncing.

The code works fine with the 'debug' configuration, but when I switch to 'release', the code breaks. 

I've traced it down to compiler optimization. Optimization levels of -O0 to -O3 work. The optimization for size (-Os) breaks my code. I'm trying to understand why.

Here's my code:

#define DEBOUNCE_CHECKS 10u

/*******************************************************************************

 * Global variables

 ******************************************************************************/

volatile uint32_t systick_counter;

volatile uint8_t raw_switches[DEBOUNCE_CHECKS], raw_controlBus[DEBOUNCE_CHECKS];

volatile uint8_t raw_switches_index = 0;

void SysTick_Handler(void)

{

    if (systick_counter != 0U)

    {

        systick_counter--;

    }

    // Read control bus

    raw_controlBus[raw_switches_index] = 0;

    if(GPIO_PortRead(GPIO, BOARD_INITPINS_REMOTE_PORT) & BOARD_INITPINS_REMOTE_PIN_MASK) {

    raw_controlBus[raw_switches_index] |= CTRL_REMOTE_BIT;

    }

    if(GPIO_PortRead(GPIO, BOARD_INITPINS_CTRL5_PORT) & BOARD_INITPINS_CTRL5_PIN_MASK) {

    raw_controlBus[raw_switches_index] |= CTRL_CAP_BIT2_BIT;

    }

[......]

    if(raw_switches_index++ > DEBOUNCE_CHECKS) {

    raw_switches_index = 0;

    }

}

The issue is the in the last if() statement. raw_switches_index starts at 0 (as initialized) and counts 1, 2, 3 ... 10, 1, 2, 3 ... 10, 1, 2, 3 ... It was supposed to count 0, 1, 2 ,3 ... 10, 0, 1, 2, 3 ...

Why is this happening and how can I avoid it? I can certainly run without compiler optimization or with -O3, but I would like to understand what's going on here. I've tried taking the increment outside the if(), so:

raw_switches_index++;

if(raw_switches_index > DEBOUNCE_CHECKS) ....

I've tried pre-incrementing and I've tried raw_switches_index = raw_switches_index + 1. The compiler just laughs at me, which is not very nice.

Anyway. I'm really curious what could cause this. The reduction in code size that results for the compiler optimization is nice, but the reduction in functionality is not.

Thanks,

Tom

[tomchr]

I'm now not able to reproduce the 'broken' behaviour either. For a while I did have raw_switches_index count differently depending on optimization level. But now its counting behaviour is the same (broken) behaviour regardless op optimization level.

Following your comment about pre- vs post-increment, I think I've found the problem. Here's a code snippet:

#define MAX_INDEX 2u

volatile uint8_t array[MAX_INDEX];

volatile uint8_t raw_switches_index;

void SysTick_Handler(void)

{

    array[raw_switches_index] = 0;

    if(raw_switches_index++ > MAX_INDEX) {

        raw_switches_index = 0;

    }

}

This is a problem as I will end up writing past the end of the array when raw_switches_index reaches 3. That overwrites the value of raw_switches_index with the value 0. Now, why this would allow the code to work with no compiler optimization (in fact it worked all the way to -O3) only to break when the code was optimized for size (-Os) is beyond me. It would be nice if I could reproduce it in a smaller code snippet.

Either way, I'll leave it for now and go fix my code. Note to self: Don't write to random locations in memory. Unpredictable behaviour may result. I can tell it's been a good decade since I last did any programming.  

Thanks for your help. Happy New Year.

Tom

[tomchr]

Pardon the delay, @xiangjun_rong.

Here's the shortest code that fails. Put a watch on raw_switches_index and monitor its value on each successive systick. It should count 0,1,2,0,1,2,0,1,2... but I have had variations of 0,1,2,1,2,1,2,1,2 and 0,1,2,3,0,1,2,3,0,1,2,3... depending on compiler optimization settings and included libraries/semihosting. 

The .txt file contains the disassembled code.

Tom

[converse · Accepted solution]

With your example, I cannot reproduce the problem (at least, I get the same behaviour regardless of optimisation level). However, I think I see the problem in your code:

if (raw_switches_index++ > 2) {
    raw_switches_index = 0;
}

this code is post-increment, so it checks raw_switches_index is >2 and THEN increments it, so if the value is 2 beforehand, it can (quite validly) now have a value of 3.

If you want to restrict its value to 0, 1 or 2, then you should pre-increment

if (++raw_switches_index > 2) {
    raw_switches_index = 0;
}

[tomchr]

Thank you for your response.

In my case the function is the systick interrupt service routine. All the variables it works on are declared as volatile. 

Tom

[converse]

It is possible, although unlikely, that there is a compiler bug.

Can you disassemble the working and non-working object files and post them? (right-click on the .o file and select Binary Utilities->Disassemble)

Also, if you could provide a minimum working example that demonstrates the problem.

[tomchr]

Update: 

The 'release' setup with compiler optimization of -Os results in 6952 B of code but loses functionality as described above.

Changing the optimization to -O3 and the library from redlib(semihost-nf) to redlib(none) results in 6544 B of code and this code has the desired functionality.

Go figure.... 

Tom

[xiangjun_rong]

Hi, Tom,

The release version of the project only selects "Optimization for size(-Os)" for the Optimization Level as the following figure. for size optimization, if you has function body, but you do not call the function, the function will be removed from the *.elf. For the variable for example hardware status which may be modified by hardware, you'd better add volatile before the variable.

for example volatile uint32_t capture_Value;

Hope it can help you

BR

XiangJun Rong

[xiangjun_rong]

Hi,

for the line volatile uint8_t raw_switches_index = 0;, can you change it as:

volatile uint8_t raw_switches_index;

void main()

{

.........................

raw_switches_index=0;

....................

}

when you declare a variable as raw_switches_index=0, it must be saved in flash and copy to RAM as a variable.

Pls have a try.

BR

XiangJun Rong