search

eLEcTRiCZiTy / libfixmath

libfixmath
0 stars 0 forks source link

fix16_div bug due to __builtin_clzl #26

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
To reproduce issue:

    #include <iostream>
    #include "fixmath.h"

    std::cout << 
        fix16_to_float(
            fix16_div(
                fix16_from_float(10.0f),
                fix16_from_float(1.1f)
            )
        );
    std::cout << std::endl;

Expected versus actual output:

    expected: 9.09087
    actual: 0

Version/operating system/compiler details:

    libfixmath version: r64
    operating system: Linux Raspberry 3.5.0-22-generic #34-Ubuntu SMP Tue Jan 8 21:47:00 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux 
    compiler: gcc version 4.7.2 (Ubuntu/Linaro 4.7.2-2ubuntu1)
    compilation flags include: -O0 -g -std=c++0x

Details:

    Bug seems to be due to __builtin_clzl, which returns 44 for fix16_from_int(10), causing all sorts of problems. I guess you may be falsely assuming this function counts the number of leading zeros of a 32-bit integer, whereas really an int may have more than 32 bits.

Original issue reported on code.google.com by mark.kat...@gmail.com on 6 Nov 2013 at 7:51

GoogleCodeExporter commented 8 years ago 
Hmm, isn't this a duplicate of issue #17, which is already fixed? The macro 
should be based on sizeof(long) so that bitcount is taken into account:
https://code.google.com/p/libfixmath/source/browse/trunk/libfixmath/fix16.c#278

Running the attached test.cc on my computer:

petteri@oddish:~/libfixmath/unittests$ uname -a
Linux oddish 3.11.0-12-generic #19-Ubuntu SMP Wed Oct 9 16:20:46 UTC 2013 
x86_64 x86_64 x86_64 GNU/Linux
petteri@oddish:~/libfixmath/unittests$ g++ -I ../libfixmath -o test test.cc 
../libfixmath/fix16.c
petteri@oddish:~/libfixmath/unittests$ ./test
sizeof(int) = 4
sizeof(long) = 8
9.09087

Original comment by Petteri.Aimonen on 7 Nov 2013 at 8:20

Attachments:

GoogleCodeExporter commented 8 years ago 
Ah, just saw the revision number r64. Upgrade to newest svn and it should work.

flatmush: maybe we should put a new release on the downloads tab some time? :)

Original comment by Petteri.Aimonen on 7 Nov 2013 at 8:22

GoogleCodeExporter commented 8 years ago 
I would second that suggestion. If this is a known bug that has been fixed then 
maybe the default suggested download shouldn't be a version of the library 
which still contains this bug.

Original comment by mark.kat...@gmail.com on 7 Nov 2013 at 12:07

GoogleCodeExporter commented 8 years ago 
flatmush: I took the liberty of marking the downloads as deprecated. Google 
Code will discontinue the supports for downloads soon anyway, so maybe it is 
better to only have svn.

Original comment by Petteri.Aimonen on 8 Dec 2013 at 6:18