ridiculousfish / libdivide

Official git repository for libdivide: optimized integer division
http://libdivide.com
Other
1.09k stars 77 forks source link

Fix -Wshift-count-overflow on avr #41

Closed jnohlgard closed 6 years ago

jnohlgard commented 6 years ago

Using libdivide on avr results in the following compile time warning/error:

include/libdivide.h:585:52: error: right shift count >= width of type [-Werror=shift-count-overflow]
         un64 = (u1 << s) | ((u0 >> (64 - s)) & (-s >> 31));
                                                    ^~
cc1: all warnings being treated as errors

I think this change is the correct cast to get the right behaviour on platforms where int is < 32 bits.

kimwalisch commented 6 years ago

Hi,

Thanks for reporting!

Using libdivide on avr results in the following compile time warning/error:

Do you have a little more information about the avr CPU architecture you are using?

Honestly I have never used a CPU architecture that is neither 32 or 64-bit and so I have not tested libdivide on such a CPU architecture.

That's why I am curious if libdivide passes its test suite on avr.

kimwalisch commented 6 years ago
static uint64_t libdivide_128_div_64_to_64()
{
    // ...
    int s;
    s = libdivide__count_leading_zeros64(v);

    // ...
    un64 = (u1 << s) | ((u0 >> (64 - s)) & (-s >> 31));
}

The code clearly assumes that the type of s is signed 32-bit as the return type of libdivide__count_leading_zeros64() is int32_t. Hence I am in favour of just changing the type of sfrom int to int32_t instead of your cast.

kimwalisch commented 6 years ago

Fixed by https://github.com/ridiculousfish/libdivide/commit/7d8a4c7bf1984101d509d5c0b5901573d083623f