A C preprocessor macro to pack bitfields into a byte?

Question

I'm getting into microcontroller hacking and while I'm very comfortable with bitwise operators and talking right to the hardware, I'm finding the resulting code very verbose and boilerplate. The higher level programmer in me wants to find an effective but efficient way to clean it up.

For instance, there's a lot of setting flags in registers:

/* Provided by the compiler */
#define SPIE 7
#define SPE 6
#define DORD 5
#define MSTR 5
#define CPOL 4
#define CPHA 3

void init_spi() {
  SPCR = (1 << SPE) | (1 << SPIE) | (1 << MSTR) | (1 << SPI2X);      
}

Thankfully there are macros that hide actual port IO operations (the left hand side), so it looks like a simple assignment. But all that syntax to me is messy.

Requirements are:

• it only has to handle up to 8 bits,
• the bit positions must be able to be passed in any order, and
• should only require set bits to be passed.

The syntax I'd like is:

SPCR = bits(SPE, SPIE, MSTR, SPI2X);

The best I've come up with so far is a combo macro/function:

#define bits(...) __pack_bits(__VA_ARGS__, -1)

uint8_t __pack_bits(uint8_t bit, ...) {
    uint8_t result = 0;
    va_list args;
    va_start(args, bit);

    result |= (uint8_t) (1 << bit);

    for (;;) {
        bit = (uint8_t) va_arg(args, int);
        if (bit > 7) 
            break;
        result |= (uint8_t) (1 << bit);
    }
}

This compiles to 32 bytes on my particular architecure and takes 61-345 cycles to execute (depends on how many bits were passed).

Ideally this should be done in preprocessor since the result is a constant, and the output machine instructions shouldbe just an assignment of an 8 bit value to a register.

Can this be done any better?

Answer 1

Yea, redefine the macros ABC as 1 << ABC, and you simplify that. ORing together bit masks is a very common idiom that anyone will recognize. Getting the shift positions out of your face will help a lot.

Your code goes from

#define SPIE 7
#define SPE 6
#define DORD 5
#define MSTR 5
#define CPOL 4
#define CPHA 3

void init_spi() {
  SPCR = (1 << SPE) | (1 << SPIE) | (1 << MSTR) | (1 << SPI2X);      
}

to this

#define BIT(n) (1 << (n))
#define SPIE BIT(7)
#define SPE BIT(6)
#define DORD BIT(5)
#define MSTR BIT(5)
#define CPOL BIT(4)
#define CPHA BIT(3)

void init_spi() {
  SPCR =  SPE | SPIE | MSTR | SPI2X;
}

This suggestion does assume that the bit-field definitions are used many times more than there are definitions of them.

---

I feel like there might be some way to use variadic macros for this, but I can't figure on anything that could easily be used as an expression. Consider, however, creating an array literal inside a function generating your constant:

#define BITS(name, ...) \
     char name() { \
         char[] bits = { __VA_ARGS__ }; \
         char byte = 0, i; \
         for (i = 0; i < sizeof(bits); ++i) byte |= (1 << bits[i]); \
         return byte; }

/*  Define the bit-mask function for this purpose */
BITS(SPCR_BITS, SPE, SPIE, MSTR, SPI2X)

void init_spi() {
    SPCR = SPCR_BITS();
}

If your compiler is good, it will see that the entire function is constant at compile-time, and inline the resultant value.

Answer 2

Why not create your own definitions in addition to the pre-defined ones...

#define BIT_TO_MASK(n) (1 << (n))

#define SPIE_MASK BIT_TO_MASK(SPIE)
#define SPE_MASK  BIT_TO_MASK(SPE)
#define DORD_MASK BIT_TO_MASK(DORD)
#define MSTR_MASK BIT_TO_MASK(MSTR)
#define CPOL_MASK BIT_TO_MASK(CPOL)
#define CPHA_MASK BIT_TO_MASK(CPHA)

void init_spi() {
  SPCR =  SPE_MASK | SPIE_MASK | MSTR_MASK | SPI2X_MASK;
}