user3835277
user3835277

Reputation:

Is there anyway to get around this compiler optimization in C?

I want to note that, as pointed out by Olaf, the compiler is not at fault.


Disclaimer: I'm not entirely sure this behavior is due to compiler optimization.

Anyways, in C I'm trying to determine whether the n-th bit (n should be between 0 and 7, inclusive) of an 8-bit byte is 1 or 0. I initially came up with this solution:

#include <stdint.h>
#include <stdbool.h>

bool one_or_zero( uint8_t t, uint8_t n ) // t is some byte, n signifies which bit
{
    return (t << (n - (n % 8) - 1)) >> 7;
}

Which, from my previous understanding, would do the following to a byte:

Suppose t = 5 and n = 2. Then the byte t can be represented as 0000 0101. I assumed that (t << (n - (n % 8) - 1)) would shift the bits of t so that t is 1010 0000. This assumption is only somewhat correct. I also assumed the next bit shift (>> 7) would shift the bits of t so that t is 0000 0001. This assumption is also only somewhat correct.

TL;DR: I thought the line return (t << (n - (n % 8) - 1)) >> 7; did this:

  1. t is 0000 0101
  2. The first bit shift occurs; t is now 1010 0000
  3. The second bit shift occurs; t is now 0000 0001
  4. t is returned as 0000 0001

Although I intend for that to happen, it does not. Instead, I have to write the following, to get my intended results:

bool one_or_zero( uint8_t t, uint8_t n ) // t is some byte, n signifies which bit
{
    uint8_t val = (t << (n - (n % 8) - 1));
    return val >> 7;
}

I know that adding uint8_t val isn't a massive performance drain. Still, I'd like to know two things:

  1. Do I have to initialize another variable to do what I intend?
  2. Why doesn't the one-liner do the same thing as the two-liner?

I'm under the impression that when the compiler optimizes my code, it smashes the two bit shifts together so only one occurs. This seems like a nice thing, but it doesn't "clear" the other bits as intended.

Upvotes: 4

Views: 148

Answers (2)

too honest for this site
too honest for this site

Reputation: 12263

That code is very complicated just to check a bit in an integer. Try the standard method:

return (t & (1U << n)) != 0;

If you have to check n is valid, add an assertion. else masking (n & 7) or modulus (n % 8) (this will be optimized by the compiler to the mask-operation) will force the shift count in a valid range. As that pattern will be recognized by many compilers, they might transform this to a single bit-test CPU instruction if available.

To avoid magic numbers, you should replace the modulus 8 by: (sizeof(t) * CHAR_BIT). That will follow any type t might have. The mask is always one less than the modulus.

Your code:

(n - (n % 8) - 1))

If n < 8 it yields a negative value (-1 precisely). Negative shifts present undefined behaviour, so anything can happen (watch out for nasal demons).

Upvotes: 7

Stian Svedenborg
Stian Svedenborg

Reputation: 1825

I believe you are the victim of integer promotion.

When you have an expression: x operator y there are a few things you should be aware of. The first is that the result (and in the process the other operand) of the expression is promoted to the "largest" type of the two operands.

In your example, this means the following:

  1. (t << (n - (n % 8) - 1)) >> 7; The constant 8 is an int therefore n%8 is also an int.
  2. (t << (n - (integer) - 1)) >> 7 (n - integer - 1) is also an integer, which means that the temporary value (t << integer) will be stored in an int. This means that you don't "cut off" the most significant bits like you intend, because the result is stored in (most likely) 32 bits, and not 8 like you presume.

If you on the other hand temporarily store the int result in an uint8_t you will correctly cut off the leading bits and get what you intend.

The you can work around the problem by casting your operands to uint8_t during the computation:

(t << (uint8_t)(n - (n % 8) - 1))) >> 7;

Or even better, use a mask like suggested in the answer by Olaf:

(t & ((uint8_t)1 << n)) != 0 

Upvotes: 0

Related Questions