12

Say I have a set of flags, encoded in a uint16_t flags. For example, AMAZING_FLAG = 0x02. Now, I have a function. This function needs to check if I want to change the flag, because if I want to do that, I need to write to flash. And that is expensive. Therefore, I want a check which tells me if flags & AMAZING_FLAG is equal to doSet. This is the first idea:

setAmazingFlag(bool doSet)
{
    if ((flags & AMAZING_FLAG) != (doSet ? AMAZING_FLAG : 0)) {
        // Really expensive thing
        // Update flags
    }
}

This is not an intuitive if statement. I feel like there should be a better way, something like:

if ((flags & AMAZING_FLAG) != doSet){

}

But this does not actually work, true seems to be equal to 0x01.

So, is there a neat way to compare a bit to a boolean?

Cheiron
  • 3,620
  • 4
  • 32
  • 63
  • 2
    It's not entirely clear what your logic needs to be. Is it this: `(flags & AMAZING_FLAG) && doSet`? – kaylum Feb 20 '20 at 10:32
  • The question is not clear. We want to check whether the 'flags' is 0x01 or not. Is that you want? If yes, then we can use bitwise operator '&'. – Vikas Vijayan Feb 20 '20 at 10:38
  • if you want to make it more readable call setAmazingFlag only when doSet is true, then the function name checks out better otherwise you have a function that may or may not do what the name says, makes for bad code reading – AndersK Feb 20 '20 at 10:44
  • If all you're trying to do is make it more readable, just make a function `flagsNotEqual``. – Jeroen3 Feb 20 '20 at 10:51

5 Answers5

18

To convert any non-zero number to 1 (true), there is an old trick: apply the ! (not) operator twice.

if (!!(flags & AMAZING_FLAG) != doSet){
user253751
  • 57,427
  • 7
  • 48
  • 90
  • 4
    Or `(bool)(flags & AMAZING_FLAG) != doSet` -- which I find more direct. (Though [this](https://stackoverflow.com/questions/31551888/casting-int-to-bool-in-c-c) suggest that Mr Microsoft has an issue with this.) Also, `((flags & AMAZING_FLAG) != 0)` is probably exactly what it compiles to and is absolutely explicit. – Chris Hall Feb 20 '20 at 11:03
  • " Also, ((flags & AMAZING_FLAG) != 0) is probably exactly what it compiles to and is absolutely explicit ". Would it? What if doSet is true? – Cheiron Feb 20 '20 at 11:06
  • @ChrisHall Does (bool)2 result in 1 or is it still 2, in C? – user253751 Feb 20 '20 at 12:42
  • 3
    C11 Standard, 6.3.1.2 Boolean type: "When any scalar value is converted to _Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1.". And 6.5.4 Cast operators: "Preceding an expression by a parenthesized type name converts the value of the expression to the named type." – Chris Hall Feb 20 '20 at 13:03
  • @Cheiron, to be clearer: `((bool)(flags & AMAZING_FLAG) != doSet)` has the same effect as `(((flags & AMAZING_FLAG) != 0) != doSet)` and they will both probably compile to exactly the same thing. The suggested `(!!(flags & AMAZING_FLAG) != doSet)` is equivalent, and I imagine will compile to the same thing as well. It's entirely a matter of taste which one you think is clearer: the `(bool)` cast requires you to remember how casts and conversions to _Bool work; the `!!` requires some other mental gymnastics, or for you to know the "trick". – Chris Hall Feb 20 '20 at 13:28
  • Note: don't use `(BOOL)` on Windows. Windows's BOOL is an integer type. – user253751 Feb 20 '20 at 14:00
2

You need to convert the bit mask to a boolean statement, which in C is equivalent to values 0 or 1.

  • (flags & AMAZING_FLAG) != 0. The most common way.

  • !!(flags & AMAZING_FLAG). Somewhat common, also OK to use, but a bit cryptic.

  • (bool)(flags & AMAZING_FLAG). Modern C way from C99 and beyond only.

Take any of the above alternatives, then compare it with your boolean using != or ==.

Lundin
  • 195,001
  • 40
  • 254
  • 396
1

From a logical point of view, flags & AMAZING_FLAG is only a bit operation masking all other flags. The result is a numerical value.

To receive to a boolean value, you would use a comparison

(flags & AMAZING_FLAG) == AMAZING_FLAG

and can now compare this logical value to doSet.

if (((flags & AMAZING_FLAG) == AMAZING_FLAG) != doSet)

In C there may be abbreviations, because of the implicit conversion rules of numbers to boolean values. So you could also write

if (!(flags & AMAZING_FLAG) == doSet)

to write that more terse. But the former version is better in terms of readability.

Ctx
  • 18,090
  • 24
  • 36
  • 51
1

You can create a mask based on doSet value:

#define AMAZING_FLAG_IDX 1
#define AMAZING_FLAG (1u << AMAZING_FLAG_IDX)
...

uint16_t set_mask = doSet << AMAZING_FLAG_IDX;

Now your check can look like this:

setAmazingFlag(bool doSet)
{
    const uint16_t set_mask = doSet << AMAZING_FLAG_IDX;

    if (flags & set_mask) {
        // Really expensive thing
        // Update flags
    }
}

On some architectures, !! may be compiled to a branch and by this, you may have two branches:

  1. Normalisation by !!(expr)
  2. Compare to doSet

The advantage of my proposal is a guaranteed single branch.

Note: make sure you don't introduce undefined behaviour by shifting left by more than 30 (assuming integer is 32 bits). This can be easily achieved by a static_assert(AMAZING_FLAG_IDX < sizeof(int)*CHAR_BIT-1, "Invalid AMAZING_FLAG_IDX");

Alex Lop.
  • 6,810
  • 1
  • 26
  • 45
  • 1
    All manner of boolean expressions have the potential to result in a branch. I'd disassemble first, before applying strange manual optimization tricks. – Lundin Feb 20 '20 at 11:46
  • 1
    It's mostly a matter for the compiler's optimizer and not so much the ISA itself. – Lundin Feb 20 '20 at 11:53
  • 1
    @Lundin I disagree. Some architectures have ISA for conditional bit set/reset in which case no brach is needed, others don't. https://godbolt.org/z/4FDzvw – Alex Lop. Feb 20 '20 at 12:14
  • Note: If you do this, please define `AMAZING_FLAG` in terms of `AMAZING_FLAG_IDX` (e.g. `#define AMAZING_FLAG ((uint16_t)(1 << AMAZING_FLAG_IDX))`), so you don't have the same data defined in two places such that one could be updated (say, from `0x4` to `0x8`) while the other (`IDX` of `2`) is left unchanged. – ShadowRanger Feb 20 '20 at 21:10
  • @AlexLop. Not sure what I'm about to see in that link. For me, both functions for both architectures has exactly one conditional branch. – pipe Feb 20 '20 at 21:59
  • @pipe yes but the function with shift has less opcodes. Also that site has very few arch options. It doesn’t have ARC, for example, which has such conditional opcodes... – Alex Lop. Feb 21 '20 at 04:36
  • @ShadowRanger of course, that was obvious for me but you are right, better to write it explicitly in the answer. I will update it. – Alex Lop. Feb 21 '20 at 04:38
  • Using `1u` is better the 1 to not shift into a sign bit (so update on the right track), yet code still does a _signed_ shift with `doSet << AMAZING_FLAG_IDX`. – chux - Reinstate Monica Feb 21 '20 at 04:49
  • @chux-ReinstateMonica Nothing wrong to have a signed shift. Also suggested to have a static assert to cover this case. Also you can cast ‘doSet’ to ‘uint32_t’ but shifting by more than 31 will still invoke undefined behavior – Alex Lop. Feb 21 '20 at 04:55
  • Alex, [Nothing wrong to have a signed shift.](https://stackoverflow.com/questions/60317850/comparing-a-bit-to-a-boolean/60319011?noredirect=1#comment106723808_60319011): With 16-bit `int`, (something common with embedded processors - may or may not apply to OP's case), `doSet*1u << 15` is OK. `doSet << 15` is UB. It isn't the shifting of `n` or more bits that given _unsigned_ an advantage, it is the shifting of `n-1` bits. – chux - Reinstate Monica Feb 21 '20 at 15:23
-1

There are multiple ways to perform this test:

The ternary operator might generate costly jumps:

if ((flags & AMAZING_FLAG) != (doSet ? AMAZING_FLAG : 0))

You can also use boolean conversion, which may or may not be efficient:

if (!!(flags & AMAZING_FLAG) != doSet)

Or its equivalent alternative:

if (((flags & AMAZING_FLAG) != 0) != doSet)

If multiplication is cheap, you can avoid jumps with:

if ((flags & AMAZING_FLAG) != doSet * AMAZING_FLAG)

If flags is unsigned and the compiler is very smart, the division below might compile to a simple shift:

if ((flags & AMAZING_FLAG) / AMAZING_FLAG != doSet)

If the architecture uses two's complement arithmetic, here is another alternative:

if ((flags & AMAZING_FLAG) != (-doSet & AMAZING_FLAG))

Alternately, one could define flags as a structure with bitfields and use a much simpler an readable syntax:

if (flags.amazing_flag != doSet)

Alas, this approach is usually frowned upon because the specification of bitfields does not allow precise control over bit-level implementation.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • The primary goal of any piece of code should be readability, which most of your examples are not. An even bigger problem arises when you actually load your examples into an compiler: the initial two example implementations have the best performance: least amount of jumps and loads (ARM 8.2, -Os) (they are equivalent). All other implementations perform worse. In general, one should avoid doing fancy stuff (micro-optimization), because you will make it harder for the compiler to figure out what is going on, which degrades performance. Therefore, -1. – Cheiron Mar 15 '20 at 21:56