Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need patch for Fedora package for non x86 architectures #25

Closed
pcpa opened this issue Jan 27, 2015 · 7 comments
Closed

Need patch for Fedora package for non x86 architectures #25

pcpa opened this issue Jan 27, 2015 · 7 comments

Comments

@pcpa
Copy link

pcpa commented Jan 27, 2015

Arm apparently built fine, but s390 failed:
https://bugzilla.redhat.com/show_bug.cgi?id=1186162

But I am afraid there will be issues on other architectures.

The patch I made, http://pkgs.fedoraproject.org/cgit/mp.git/diff/mp-arm.patch?id=95f0b59ae66eb4c870f4a3110a3362cd83fb4344
is not enough.

@vitaut
Copy link
Contributor

vitaut commented Jan 27, 2015

Thanks for the bug report.

Here's a patch that should fix the problem:

From 986a91a46e206e268eeda7c3ff614f4cd982a4dc Mon Sep 17 00:00:00 2001
From: Victor Zverovich <victor.zverovich@gmail.com>
Date: Tue, 27 Jan 2015 14:22:08 -0800
Subject: [PATCH] Improve portability of fpinit

---
 src/asl/solvers/fpinit.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/asl/solvers/fpinit.c b/src/asl/solvers/fpinit.c
index ae06cf0..1afc23a 100644
--- a/src/asl/solvers/fpinit.c
+++ b/src/asl/solvers/fpinit.c
@@ -125,14 +125,18 @@ fpinit_ASL(Void)
    _FPU_GETCW(__fpu_control);
    __fpu_control &= ~_FPU_EXTENDED;    /* clear rounding precision bits */
    __fpu_control |= _FPU_DOUBLE;       /* set the ones we want set */
+   _FPU_SETCW(__fpu_control);
 #else
 #if defined(_FPU_IEEE) && defined(_FPU_EXTENDED) && defined(_FPU_DOUBLE)
    __fpu_control = _FPU_IEEE - _FPU_EXTENDED + _FPU_DOUBLE;
-#else
+   _FPU_SETCW(__fpu_control);
+#elif defined(__i386__) || defined(__x86_64__)
    __fpu_control = 0x27f;
+   _FPU_SETCW(__fpu_control);
+#elif defined(FE_ALL_EXCEPT)
+   fedisableexcept(FE_ALL_EXCEPT);
 #endif
 #endif /* ASL_FPINIT_KEEP_TRAPBITS */
-   _FPU_SETCW(__fpu_control);
 #endif
    }
 #endif /*} NO_fpu_control */
-- 
1.9.1

However, I haven't tried it as I don't have access to a s390 machine.

Please let me know if it helps and I'll merge it into master.

@pcpa
Copy link
Author

pcpa commented Jan 28, 2015

I do not have a shell to see exactly what happened, but submitting a scratch build failed.
http://s390.koji.fedoraproject.org/kojifiles/work/tasks/6258/1716258/build.log
(temporary link) I will ask someone with shell access to see if it was a coredump.
I can also try to create a vm, but that would take "forever" under emulation...
For reference, the bug report that showed the values were x86 specific
https://bugzilla.redhat.com/show_bug.cgi?id=1186162

@pcpa
Copy link
Author

pcpa commented Jan 28, 2015

This should give a hint of what is the problem. If you need some further testing please let me know.

[ RUN      ] ASLBuilderTest.InitASLFull
/root/rpmbuild/BUILD/mp-35060ba2a59f2b0f0fd622ed9df678f142f846ed/test/asl/aslbuilder-test.cc:187: Failure
Value of: actual.i.xscanf_
  Actual: 0x800b64c0
Expected: expected.i.xscanf_
Which is: 0x800b6100
/root/rpmbuild/BUILD/mp-35060ba2a59f2b0f0fd622ed9df678f142f846ed/test/asl/aslbuilder-test.cc:277: Failure
Value of: actual.i.binary_nl_
  Actual: 2
Expected: expected.i.binary_nl_
Which is: 0
/root/rpmbuild/BUILD/mp-35060ba2a59f2b0f0fd622ed9df678f142f846ed/test/asl/aslbuilder-test.cc:384: Failure
Value of: actual.i.iadjfcn
  Actual: 0x800c5390
Expected: expected.i.iadjfcn
Which is: NULL
/root/rpmbuild/BUILD/mp-35060ba2a59f2b0f0fd622ed9df678f142f846ed/test/asl/aslbuilder-test.cc:385: Failure
Value of: actual.i.dadjfcn
  Actual: 0x800c5390
Expected: expected.i.dadjfcn
Which is: NULL
[  FAILED  ] ASLBuilderTest.InitASLFull (1 ms)
[ RUN      ] ASLBuilderTest.ASLBuilderAdjFcn

@vitaut
Copy link
Contributor

vitaut commented Jan 28, 2015

Well, at least it gets to the tests now. The test failure looks related to the detection of floating-point arithmetic, I'll look into it in more details shortly. Thanks for providing the test log BTW, this is very useful.

@vitaut
Copy link
Contributor

vitaut commented Jan 28, 2015

This should (hopefully) be fixed in 0b53c90.

@pcpa
Copy link
Author

pcpa commented Jan 28, 2015

Confirmed! With that patch it pass all tests :)
I believe something like this may also be good, but should not make difference
currently for Fedora:

 #elif defined(__i386__) || defined(__x86_64__)
    __fpu_control = 0x27f;
    _FPU_SETCW(__fpu_control);
+#elif defined(_FPU_DEFAULT)
+   __fpu_control = _FPU_DEFAULT;
+   _FPU_SETCW(__fpu_control);
 #elif defined(FE_ALL_EXCEPT)
    fedisableexcept(FE_ALL_EXCEPT);

as _FPU_DEFAULT appears to be defined in most, if not all glibc ports,
and defaults to double precision IEEE floating math.

@vitaut
Copy link
Contributor

vitaut commented Jan 28, 2015

Great, thanks for the confirmation!

I'm not sure about _FPU_DEFAULT though, as it doesn't mask interrupts. I think it's better to fall back on fedisableexcept which does. Besides ASL should work with non-IEEE floating point too.

@vitaut vitaut closed this as completed Jan 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants