Is this clang optimization a bug?

Question

I ran into an interesting issue when compiling some code with -O3 using clang on OSX High Sierra. The code is this:

#include <stdint.h>
#include <limits.h> /* for CHAR_BIT */
#include <stdio.h> /* for printf() */
#include <stddef.h> /* for size_t */

uint64_t get_morton_code(uint16_t x, uint16_t y, uint16_t z)
{
    /* Returns the number formed by interleaving the bits in x, y, and z, also
     * known as the morton code.
     *
     * See https://graphics.stanford.edu/~seander/bithacks.html#InterleaveTableO
bvious.
     */
    size_t i;
    uint64_t a = 0;

    for (i = 0; i < sizeof(x)*CHAR_BIT; i++) {
        a |= (x & 1U << i) << (2*i) | (y & 1U << i) << (2*i + 1) | (z & 1U << i)
 << (2*i + 2);
    }

    return a;
}

int main(int argc, char **argv)
{
    printf("get_morton_code(99,159,46) = %llu\n", get_morton_code(99,159,46));
    return 0;
}

When compiling this with cc -O1 -o test_morton_code test_morton_code.c I get the following output:

get_morton_code(99,159,46) = 4631995

which is correct. However, when compiling with cc -O3 -o test_morton_code test_morton_code.c:

get_morton_code(99,159,46) = 4294967295

which is wrong.

What is also odd is that this bug appears in my code when switching from -O2 to -O3 whereas in the minimal working example above it appears when going from -O1 to -O2.

Is this a bug in the compiler optimization or am I doing something stupid that's only appearing when the compiler is optimizing more aggressively?

I'm using the following version of clang:

snotdaqs-iMac:snoFitter snoperator$ cc --version
Apple LLVM version 9.1.0 (clang-902.0.39.1)
Target: x86_64-apple-darwin17.5.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Answer 1

UndefinedBehaviorSanitizer is really helpful in catching such mistakes:

$ clang -fsanitize=undefined -O3 o3.c
$ ./a.out
o3.c:19:2: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int'
get_morton_code(99,159,46) = 4294967295

A possible fix would be replacing the 1Us with 1ULL, an unsigned long long is at least 64 bit and can be shifted that far.

Answer 2

When i is 15 in the loop, 2*i+2 is 32, and you are shifting an unsigned int by the number of bits in an unsigned int, which is undefined.

You apparently intend to work in a 64-bit field, so cast the left side of the shift to uint64_t.

A proper printf format for uint64_t is get_morton_code(99,159,46) = %" PRIu64 "\n". PRIu64 is defined in the <inttypes.h> header.