Tests crash on i386 #565
Comments
|
Working diff: diff --git a/test/harness.cpp b/test/harness.cpp
index 6012425..6ec319c 100644
--- a/test/harness.cpp
+++ b/test/harness.cpp
@@ -107,7 +107,10 @@ static std::string PrepareSavefile(std::string data) {
if(SolveSpaceUI::SAVED[i].fmt != 'f') continue;
if(SolveSpaceUI::SAVED[i].desc != key) continue;
double f = strtod(value.c_str(), NULL);
+ fprintf(stderr, "f=%s\n", value.c_str());
+ fprintf(stderr, " f_pre =%.20f\n", f);
f = round(f * precision) / precision;
+ fprintf(stderr, " f_post=%.20f\n", f);
std::string newValue = ssprintf("%.20f", f);
ssassert(value.size() == newValue.size(), "Expected no change in value length");
std::copy(newValue.begin(), newValue.end(),Relevant value: Full log (833kB) (it's plaintext, but GH wouldn't accept other mimetypes sensibly) |
|
I think this is because of x87 precision. I suggest simply not using i386, since there is no remaining reason to care about this architecture. But if you want it for some reason, you could try to find the changes to FP environment that bring it more in line with normal double precision floats. |
|
Turns out passing |
|
We should probably enable that for GCC on i386 in the CMake scripts. |
|
If we enable SSE then it is not i386 any more - (it does not have SSE :-). https://gcc.gnu.org/onlinedocs/gcc-3.1/gcc/i386-and-x86-64-Options.html It appears that we should use: '-mfpmath=387' (The reason it is not on by default is that on 80386 the 80387 coprocessor was a separate chip and not every PC had it). |
|
According to this GCC doc, Additionally, Clang seems to use SSE2 for floats when allowed to via |
Both GCC and Clang use x87 instructions by default on 32-bit x86 targets, but the loss of precision resulting from that has yielded crashing tests (see referenced issue), and it can be safely assumed there are very few CPUs that both don't support SSE2 and are expected to run SolveSpace This commit also removes a now-redundant check for 32-bit Windows against TARGET, which doesn't seem to be actually set by CMake at all Ref: solvespace#565
|
In this case I think whatever is failing is a bug (I'll take a look when I end up on a PC) and we should not hide it by using SSE. |
That's incorrect. While you can build software for a target called "i386", no Linux distribution released in the last several years, and indeed no such Linux kernel release, will run without SSE, much less on an actual i386. Some toolchains call the same target "i686", although that's not quite literally true either.
Using nonstandard 80 bit floating point numbers is exactly the cause of the problem @nabijaczleweli is facing here. They're rounded differently than what the tests are expecting and so the test fixtures don't match.
In the future, please make sure to test your speculation before making authoritative statements. I wrote all of these tests and know perfectly well why they break here.
The "bug" here is that the testsuite effectively depend on specific rounding modes and floating point sizes. Although I would agree that a testsuite ideally should not depend on that, this was the best I could come up with. Of course, you are welcome to try and do a better job. |
Both GCC and Clang use x87 instructions by default on 32-bit x86 targets, but the loss of precision resulting from that has yielded crashing tests (see referenced issue), and it can be safely assumed there are very few CPUs that both don't support SSE2 and are expected to run SolveSpace This commit also removes a now-redundant check for 32-bit Windows against TARGET, which doesn't seem to be actually set by CMake at all Ref: solvespace#565
|
You are right. |
|
Thanks. |
Both GCC and Clang use x87 instructions by default on 32-bit x86 targets, but the loss of precision resulting from that has yielded crashing tests (see referenced issue), and it can be safely assumed there are very few CPUs that both don't support SSE2 and are expected to run SolveSpace This commit also removes a now-redundant check for 32-bit Windows against TARGET, which doesn't seem to be actually set by CMake at all Ref: #565
Both GCC and Clang use x87 instructions by default on 32-bit x86 targets, but the loss of precision resulting from that has yielded crashing tests (see referenced issue), and it can be safely assumed there are very few CPUs that both don't support SSE2 and are expected to run SolveSpace This commit also removes a now-redundant check for 32-bit Windows against TARGET, which doesn't seem to be actually set by CMake at all Ref: solvespace#565
System information
SolveSpace version: current HEAD (e355a09)
Operating system: Debian sid i386 + libc6 2.30-2
Expected behavior
Tests run
Actual behavior
Additional information
The text was updated successfully, but these errors were encountered: