Part of macro optimized away when using constant value

Question

This is a WS2812B RGB LED strip driving code for AVR (Arduino hardware). I have some issues with the compiler optimizing away parts of my code. I tried -O1, -O2, -Os, same result.I can't use -O0 because then delays don't work.

Macros

Here is my ws_driverm.h file:

#pragma once    
#include <util/delay_basic.h>
#include <avr/io.h>    
#include "lib/pins.h"

#define WS_T_1H  800
#define WS_T_1L  450
#define WS_T_0H  200
#define WS_T_0L  850
#define WS_T_LATCH 7000

/** Latch and display the RGB values */
#define ws_show(io) do { pin_low(io_pack(io)); delay_ns_c(WS_T_LATCH, 0); } while(0)

/** Send one byte to the RGB strip */
#define ws_send_byte(io, bb) do {                               \
    for (int8_t __wsba_i = 7; __wsba_i >= 0; --__wsba_i) {      \
        if (bb & (1 << __wsba_i)) {                             \
            pin_high(io_pack(io)); delay_ns_c(WS_T_1H, -2);     \
            pin_low(io_pack(io)); delay_ns_c(WS_T_1L, -10);     \
        } else {                                                \
            pin_high(io_pack(io)); delay_ns_c(WS_T_0H, -2);     \
            pin_low(io_pack(io)); delay_ns_c(WS_T_0L, -10);     \
        }                                                       \
    }                                                           \
} while(0)

/** Send RGB color to the strip */
#define ws_send_rgb(io, r, g, b) do {           \
    ws_send_byte(io_pack(io), g);               \
    ws_send_byte(io_pack(io), r);               \
    ws_send_byte(io_pack(io), b);               \
} while(0)

I'm not happy about it being macros, but all my attempts to use pointer to IO port, port address etc failed.

The io is part of my pin handling system, just ignore it, it's not relevant for the question.

Problem

Here's the problem (in main)

When I use it with variables that are guaranteed to change, or mark them volatile, it works as expected:

volatile uint8_t r = 0;
volatile uint8_t g = 255;
volatile uint8_t b = 0;
ws_send_rgb(PIN_WS, r, g, b);
ws_show(PIN_WS);
// results in GREEN color

When the variables are not volatile, or if I just put numbers in the macro invocation, it does not work:

uint8_t r = 0;
uint8_t g = 255;
uint8_t b = 0;
ws_send_rgb(PIN_WS, r, g, b);
ws_show(PIN_WS);
// results in YELLOW color

Same result:

ws_send_rgb(PIN_WS, 0, 255, 0);
ws_show(PIN_WS);
// results in YELLOW color

Clearly the code produced by the macro is for some reason optimized away, if it's constant and zero (at least it looks that way - I'm not really sure why the LED displays yellow).

Interesting: -> If I use 1 in place of 0 for the color, it works as expected.

What is wrong with my macro, and how can I fix it? Dirty workarounds are fine, at this point.

What is the definition of `PIN_WS` and how are defined `pin_low`, `iopack` and `delay_ns_c`? — ouah, Dec 22 '14 at 13:05
They're some macros trickery, I'm not sure if it's a good idea to paste the whole pins.h file here. Basically, they write bits in IO port (PORTB). In essence, `pin_high(PIN_WS)` would result in `do { ((*(volatile uint8_t *)((0x05) + 0x20))) |= (1 << 0); } while(0);` after being chewed through by the preprocessor. Or, more human readable, `PORTB |= (1 << 0);` — MightyPork, Dec 22 '14 at 13:08
It would be revealing to look at an assembler listing of the failing code. — Armali, Feb 09 '17 at 12:56

score 1 · Answer 1 · answered Dec 22 '14 at 13:30

I've solved it this way for now - works pretty good: I added volatile keyword to the loop variable.

No idea why it helped, but it did.

/** Send one byte to the RGB strip */
#define ws_send_byte(io, bb) do {                               \
    for (volatile int8_t __wsba_i = 7; __wsba_i >= 0; --__wsba_i) {      \
        if (bb & (1 << __wsba_i)) {                             \
            pin_high(io_pack(io)); delay_ns_c(WS_T_1H, -2);     \
            pin_low(io_pack(io)); delay_ns_c(WS_T_1L, -10);     \
        } else {                                                \
            pin_high(io_pack(io)); delay_ns_c(WS_T_0H, -2);     \
            pin_low(io_pack(io)); delay_ns_c(WS_T_0L, -10);     \
        }                                                       \
    }                                                           \
} while(0)

score 0 · Answer 2 · answered Dec 22 '14 at 13:04

I'm guessing pin_high and pin_low write to a memory address.

What's happening is (in effect) the compiler is being given code like this:

char *address = ...;
*address = something;
*address = somethingelse;

It's dropping the second line there, because if address points to normal memory, then this does nothing. But it doesn't. Your write to memory has a side effect which is important.

In order to avoid this, you should really fix pin_high and pin_low to contain a memory barrier or similar. However, your volatile keyword fixes it. I suspect it probably fixes it by accident, as it's really the memory address (or rather its contents) which should be volatile, rather than what is being written to it.

A possible route here is to introduce a C function which does nothing and hope that acts as a memory barrier (i.e. something which the compiler things might use the written value). You need this not to be optimized out. For instance:

static void donothingroutine() {}
volatile void (*donothing)() = &donothingroutine;

Then in the macro, add:

donothing();

As the pointer to the routine is declared volatile, from the compiler's point of view donothing() it might be a routine that accesses the 'memory' written to, so it cannot optimise out the write. So sprinkle that liberally inside your macro.

Didn't work, I added `donothingroutine();` between the two writes to the pin, same result. The written memory location is volatile, avr-libc takes care of it. It looks somewhat like `((*(volatile uint8_t *)((0x05) + 0x20))) |= (1 << 0)` — MightyPork, Dec 22 '14 at 13:14
Oh! I found a fix - but it's probably not the best. If I make the loop variable in the macro volatile, it works good. Does that look like a valid solution? — MightyPork, Dec 22 '14 at 13:16
I suspect you are relying on compiler heuristics. If it works for now, great (for now), but you probably need a proper memory barrier. — abligh, Dec 22 '14 at 22:36

Part of macro optimized away when using constant value

Macros

Problem

2 Answers2