talda / libfixmath

Automatically exported from code.google.com/p/libfixmath
0 stars 0 forks source link

Unittests fail on division, on x86_64 architecture. #17

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. cd unittests && make

What is the expected output? What do you see instead?
FAILED: fix16_unittests.c:194 failures == 0 
(If I add a simple 
    printf("Number of failures: %d\n", failures);
I get: 2961 failures)

The issue is that most of the divisions yield a wrong result.

What version of the product are you using? On what operating system?
Using the latest svn revision (r82)

Please provide any additional information below.

The complete compilation log and verbose tests run (with each failed test 
result) is attached.

These gcc version and architectures FAILED the tests:

% gcc --version 
gcc (GCC) 4.7.0 20120507 (Red Hat 4.7.0-5)
% uname -a
Linux xxx 3.5.2-3.fc17.x86_64 #1 SMP Tue Aug 21 19:06:52 UTC 2012 x86_64 x86_64 
x86_64 GNU/Linux

% gcc --version
gcc (Debian 4.7.1-2) 4.7.1
% uname -a
Linux xxx 3.1.0-1-amd64 #1 SMP Fri Dec 23 16:37:11 UTC 2011 x86_64 GNU/Linux

But this gcc version + architecture passes the tests successfully:

% gcc --version
gcc (GCC) 4.7.1 20120721 (prerelease)
Linux xxx 3.3.1-1-ARCH #1 SMP PREEMPT Tue Apr 3 14:43:38 UTC 2012 i686 GNU/Linux

Seems like the issue is for the x86_64 architecture, then.

Original issue reported on code.google.com by louizat...@gmail.com on 31 Aug 2012 at 9:56

Attachments:

GoogleCodeExporter commented 9 years ago
For now try reverting to the r52 division code which I believe is tested, this 
is a critical issue and I'll look into it as soon as I get time.

Original comment by Flatmush@googlemail.com on 31 Aug 2012 at 10:03

GoogleCodeExporter commented 9 years ago
Also fails with gcc 4.6.3 on x86_64.

Original comment by louizat...@gmail.com on 31 Aug 2012 at 10:04

GoogleCodeExporter commented 9 years ago
Hum, I tried
svn update -r52

and the tests still fail at that revision.

FAILED: fix16_unittests.c:194 failures == 0                                     

make: *** [run_fix16_unittests] Error 1                                         

12:06 louiz@abricot /tmp/libfixmath-read-only/unittests % svn log               

------------------------------------------------------------------------        

r52 | Petteri.Aimonen@gmail.com | 2012-01-26 16:43:30 +0100 (Thu, 26 Jan 2012) 
| 9 lines

Original comment by louizat...@gmail.com on 31 Aug 2012 at 10:07

GoogleCodeExporter commented 9 years ago
I've not really had much todo with the testing so I'm not sure how it judges 
success, but you should know that division in fixed point (like integer 
division) has limited accuracy since the result has dynamic range.
If you divide by a small/large or small/large number then you'll likely 
under/overflow. I know the old division code worked because it's been used in a 
number of my projects so it may be the case that the tests just expect a higher 
accuracy than is possible but I'll have a dig and see what I can find.
For cases where guaranteed accuracy isn't important it may be better to use 
saturated divides (e.g. graphics/games), otherwise you must handle the under or 
overflow cases.

Original comment by Flatmush@googlemail.com on 31 Aug 2012 at 10:12

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r83.

Original comment by Petteri.Aimonen on 31 Aug 2012 at 10:16

GoogleCodeExporter commented 9 years ago
This was a real bug in my code; I had not realized that the return value of GCC 
builtin clzl (count leading zeros) depends on the sizeof(long). Revision 83 
fixes this.

Original comment by Petteri.Aimonen on 31 Aug 2012 at 10:17

GoogleCodeExporter commented 9 years ago
I need to have deterministic results (always the same result for a given 
operation, whatever the arch, compiler, etc) for my non-integer operations. 
That means that I need the guaranteed accuracy. I’ll handle the overflow 
cases then.

I discovered this issue when some operation like 5.1/1.2 failed. I’ll try my 
own tests with the r52 division code and tell you if I have more success with 
that.

(And thank you very much for your fast and useful answers!)

Original comment by louizat...@gmail.com on 31 Aug 2012 at 10:21

GoogleCodeExporter commented 9 years ago
You’re too fast for me, you fix the issue before I can even respond to your 
previous message!
And you’re awesome, the tests successfully pass! Thank you again! :)

Original comment by louizat...@gmail.com on 31 Aug 2012 at 10:23

GoogleCodeExporter commented 9 years ago
Wow, nice one Petteri :)

You mind telling us what you're using libfixmath for louizatakk? You don't have 
to of-course but it's always interesting to see what people are using the code 
for.

Original comment by Flatmush@googlemail.com on 31 Aug 2012 at 10:41

GoogleCodeExporter commented 9 years ago
Oh, I didn’t pay attention, you’re two different persons. So thanks to both 
of you :)

I’m using it in an online RTS game, where only “commands” are passed from 
the server to the client (for example “unit X move there”) and then the 
whole game simulation runs in parallel on all machines. That’s why, for that 
part of the game, I cannot use floats (result could be different on each 
machine, and the games would be desynchronized).

Original comment by louizat...@gmail.com on 31 Aug 2012 at 11:28