Closed wzdev-ci closed 14 years ago
Buginator commented
also tried -mfpmath=sse -msse, and it didn't help.
error |09:39:50: [i64Sqrt] Assert in Warzone: ../../../../lib/framework/trig.c:572 ((int64_t)(r*r - n) <= 0 && (int64_t)((r + 1)*(r + 1) - n) > 0), last script event: ''
error |09:39:50: [trigInitialise] Sanity check failed, sqrt(18445618199572250625) gave 4294836224 instead of 4294836225!
Safety0ff commented
I did a test on my 32 bit system, using sqrtl it returns the proper value (4294836225,) using sqrt it returns 4294836224.
Safety0ff commented
The square root of 18445618199572250624 (18445618199572250625 adjusted to fit in the 53 bit floating point fraction,) is 4294836224.99999999988358112538. Since it gets rounded down ( 0.5f is not added,) it comes back and bites us here:
(int64_t)((r + 1)*(r + 1) - n) > 0
(zero not greater than zero)
Cyp commented
4294836224.99999999988358112538 is rounded up to 4294836225, before converting to uint64_t, since sqrt() returns a 53-bit fraction double.
It should work fine, as long as sqrt() takes a double and returns a double, or even takes a long double and returns a long double.
Sounds like as if it's taking a double and returning a long double...
Maybe add an assignment to a temporary volatile double before casting to uint64_t, inside an #ifdef CYGWIN block... (Would only be needed in i64Sqrt(), not in iSqrt().) Or try a different compiler version... Make sure it's not being compiled with -ffmath.
Safety0ff commented
You're right about the rounding and assigning to the volatile double worked.
Safety0ff commented
Using a function pointer worked as well.
Buginator commented
A small update, when the compiler is 32bit, it works as expected. If it is 64bit, then we get the above errors, and we do use -m32 as well.
Cyp commented
Hmmm, does newnet work when the cross-compiler is 32bit, then?
Buginator commented
Replying to Warzone2100/old-trac-import#1786 (comment:8):
Hmmm, does newnet work when the cross-compiler is 32bit, then?
When I was testing with you, that is what I was using.
It still doesn't work with MSVC though.
Per commented
I think we can remove this safety check and rather focus on eliminating floating point usage everywhere that affects game play.
Cyp commented
The i64Sqrt function does affect gameplay, and using the FPU and subtracting 1 if it happened to round up instead of down is probably the fastest way without a really really huge lookup table. So sqrt() and when generating the sin() tables are the two places where floating point that affects gameplay should actually remain.
Safety0ff uploaded file float.diff
(21.3 KiB)
Safety0ff commented
That patch is a naive attempt at integrating the stuff in that link (link thanks to dak180.)
Things to do, check cross compilation. Add nice SSE switches.
Safety0ff commented
Other notes:
XPFPA_SWITCH_DOUBLE();
uint64_t r = sqrt(n);
XPFPA_RESTORE();
worked as well.
Safety0ff commented
32bit CC works for me, I think Buginator had mentioned it being a issue when you CC using a 64 bit system.
For 32bit I used the -msse2 -mfpmath=sse options, it only errors it gave where the mandelbrot tests.
When I cross compiled with the above patch the only errors I got were fptest1 and fptest2.
Buginator set resolution to fixed
Buginator commented
Cyp_ fixed this in [11121]
Buginator changed status from new
to closed
cybersphinx changed milestone from 3.0
to unspecified
cybersphinx commented
Milestone 3.0 deleted
resolution_fixed
type_bug
| by BuginatorCyp, got some time to make trunk builds, and they are screwed up when done with the CC compiler. Oddly enough, MSVC still works (at least, the game starts)
I trimmed the log, since it was over 10MB and counting.
Ideas on what needs to be changed?
Issue migrated from trac:1786 at 2022-04-15 21:52:54 -0700