#13948 closed defect (fixed)
Let MPIR build with Clang
Reported by: | jpflori | Owned by: | leif |
---|---|---|---|
Priority: | blocker | Milestone: | sage-5.12 |
Component: | packages: standard | Keywords: | spkg mpir clang |
Cc: | leif, jhpalmieri | Merged in: | sage-5.12.rc1 |
Authors: | Jeroen Demeyer | Reviewers: | John Palmieri |
Report Upstream: | Reported upstream. Developers acknowledge bug. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
There are two problems with the MPIR configure
script for Clang:
1) It uses __builtin_malloc()
which Clang doesn't have, which is an MPIR upstream bug (up to and including MPIR 2.6.0). The spkg simply removes this test.
2) The "long long reliability" test fails with a linker error. This looks like a Clang bug. The spkg changes the test slightly for Clang.
spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/mpir-2.6.0.p3.spkg (spkg diff)
apply 13948_clang_doc.patch (documentation only)
Attachments (2)
Change History (46)
comment:1 Changed 10 years ago by
- Dependencies set to #13137
- Owner changed from tbd to leif
comment:2 follow-up: ↓ 3 Changed 9 years ago by
comment:3 in reply to: ↑ 2 Changed 9 years ago by
Replying to leif:
(I do have a trivial patch to
acinclude.m4
to make MPIR configure [and of course build and pass its test suite] withclang
. Haven't yet submitted it upstream either IIRC.)
I thinkTM this is the resulting patch to configure
(resulting from patching acinclude.m4
in two places):
-
mpir-2.6.0/configure
5132 5132 rm -f conftest* a.out b.out a.exe a_out.exe 5133 5133 cat >conftest.c <<EOF 5134 5134 /* The following aborts with gcc-4.3.2 on a 64bit system which is an unusable compiler */ 5135 #if defined(__GNUC__) && !defined(__INTEL_COMPILER) 5135 #if defined(__GNUC__) && !defined(__INTEL_COMPILER) && !defined(__clang__) 5136 5136 int __attribute__((noinline)) 5137 5137 foo(int i) 5138 5138 { … … 5588 5588 Extracted from tests/mpn/t-iord_u.c. Causes Apple's gcc 3.3 build 1640 and 5589 5589 1666 to segfault with e.g., -O2 -mpowerpc64. */ 5590 5590 5591 #if def __GNUC__5591 #if defined(__GNUC__) && !defined(__clang__) 5592 5592 typedef unsigned long long t1;typedef t1*t2; 5593 5593 __inline__ t1 e(t2 rp,t2 up,int n,t1 v0) 5594 5594 {t1 c,x,r;int i;if(v0){c=1;for(i=1;i<n;i++){x=up[i];r=x+1;rp[i]=r;}}return c;} … … 6393 6393 rm -f conftest* a.out b.out a.exe a_out.exe 6394 6394 cat >conftest.c <<EOF 6395 6395 /* The following aborts with gcc-4.3.2 on a 64bit system which is an unusable compiler */ 6396 #if defined(__GNUC__) && !defined(__INTEL_COMPILER) 6396 #if defined(__GNUC__) && !defined(__INTEL_COMPILER) && !defined(__clang__) 6397 6397 int __attribute__((noinline)) 6398 6398 foo(int i) 6399 6399 { … … 6849 6849 Extracted from tests/mpn/t-iord_u.c. Causes Apple's gcc 3.3 build 1640 and 6850 6850 1666 to segfault with e.g., -O2 -mpowerpc64. */ 6851 6851 6852 #if def __GNUC__6852 #if defined(__GNUC__) && !defined(__clang__) 6853 6853 typedef unsigned long long t1;typedef t1*t2; 6854 6854 __inline__ t1 e(t2 rp,t2 up,int n,t1 v0) 6855 6855 {t1 c,x,r;int i;if(v0){c=1;for(i=1;i<n;i++){x=up[i];r=x+1;rp[i]=r;}}return c;}
comment:4 Changed 9 years ago by
And that's the corresponding one against upstream's acinclude.m4
:
-
mpir-2.6.0/acinclude.m4
480 480 # first see a simple "main()" works, then go on to other checks 481 481 GMP_PROG_CC_WORKS_PART([$1], []) 482 482 483 GMP_PROG_CC_WORKS_PART_MAIN([$1], [gcc-4.3.2 on 64 bit is bad, try -O1 or -fno-strict-aliasing for the flags],484 [/* The following aborts with gcc-4.3.2 on a 64 bit system which is an unusable compiler */485 #if defined(__GNUC__) && !defined(__INTEL_COMPILER) 483 GMP_PROG_CC_WORKS_PART_MAIN([$1], [gcc-4.3.2 on 64-bit is bad, try -O1 or -fno-strict-aliasing for the flags], 484 [/* The following aborts with gcc-4.3.2 on a 64-bit system which is an unusable compiler */ 485 #if defined(__GNUC__) && !defined(__INTEL_COMPILER) && !defined(__clang__) 486 486 int __attribute__((noinline)) 487 487 foo(int i) 488 488 { … … 566 566 GMP_PROG_CC_WORKS_PART([$1], [long long reliability test 1], 567 567 [/* The following provokes a segfault in the compiler on powerpc-apple-darwin. 568 568 Extracted from tests/mpn/t-iord_u.c. Causes Apple's gcc 3.3 build 1640 and 569 1666 to segfault with e.g., -O2 -mpowerpc64.*/569 1666 to segfault with, e.g., -O2 -mpowerpc64. */ 570 570 571 #if def __GNUC__571 #if defined(__GNUC__) && !defined(__clang__) 572 572 typedef unsigned long long t1;typedef t1*t2; 573 573 __inline__ t1 e(t2 rp,t2 up,int n,t1 v0) 574 574 {t1 c,x,r;int i;if(v0){c=1;for(i=1;i<n;i++){x=up[i];r=x+1;rp[i]=r;}}return c;}
comment:5 Changed 9 years ago by
- Description modified (diff)
comment:6 Changed 9 years ago by
- Dependencies #13137 deleted
comment:7 follow-up: ↓ 9 Changed 9 years ago by
Leif: do you plan to make a proper spkg? Have you reported it upstream? (If both answers are "NO", that's fine, I can do it).
comment:8 Changed 9 years ago by
- Description modified (diff)
comment:9 in reply to: ↑ 7 ; follow-up: ↓ 10 Changed 9 years ago by
Replying to jdemeyer:
Leif: do you plan to make a proper spkg?
Plan to: Yes.
Right now: No. (So feel free to do so.)
Have you reported it upstream?
Yes, quite a while ago (on mpir-devel).
I also reported how to fix it, but haven't (yet) submitted the "final" acinclude.m4
patch above IIRC.
(I was actually going to ask Bill about the MPIR release schedule, also because of this.)
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 11 Changed 9 years ago by
- Report Upstream changed from N/A to Reported upstream. Developers acknowledge bug.
Replying to leif:
Replying to jdemeyer:
Have you reported it upstream?
Yes, quite a while ago (on mpir-devel).
http://groups.google.com/group/mpir-devel/browse_thread/thread/7888d9a026432871#
(See also http://groups.google.com/group/mpir-devel/browse_thread/thread/c9d70a195f846258#.)
I also reported how to fix it, but haven't (yet) submitted the "final"
acinclude.m4
patch above IIRC.
Yep.
comment:11 in reply to: ↑ 10 Changed 9 years ago by
- Report Upstream changed from Reported upstream. Developers acknowledge bug. to Completely fixed; Fix reported upstream
Replying to leif:
I also reported how to fix it, but haven't (yet) submitted the "final"
acinclude.m4
patch above IIRC.
Patch now submitted upstream.
comment:12 Changed 9 years ago by
Any chance to get an spkg to review? Or should I begin working on Cygwin64 support without fearing a future rebase?
comment:13 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:14 Changed 9 years ago by
I am going to move forward on this and simply make a spkg which completely removes the offending test.
comment:15 Changed 9 years ago by
- Description modified (diff)
- Report Upstream changed from Completely fixed; Fix reported upstream to Reported upstream. Developers acknowledge bug.
comment:16 Changed 9 years ago by
- Description modified (diff)
comment:17 Changed 9 years ago by
- Status changed from new to needs_review
comment:18 Changed 9 years ago by
I'm using OS X 10.8.5 with Xcode 5.0, which means that gcc --version
returns
$ gcc --version Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1 Apple LLVM version 5.0 (clang-500.2.76) (based on LLVM 3.3svn) Target: x86_64-apple-darwin12.5.0 Thread model: posix
The old mpir spkg fails to build for known reasons. The new mpir spkg fails also, though. Log file.
(I also tried unsetting MAKE
and it didn't help.)
comment:19 follow-up: ↓ 20 Changed 9 years ago by
Please try the following: extract the spkg and try to "manually" configure it by doing:
$ cd mpir-2.6.0.p3/src $ ./configure
I'm interested to see the output of that, as well as the resulting config.log
file.
comment:20 in reply to: ↑ 19 Changed 9 years ago by
Replying to jdemeyer:
Please try the following: extract the spkg and try to "manually" configure it by doing:
$ cd mpir-2.6.0.p3/src $ ./configureI'm interested to see the output of that, as well as the resulting
config.log
file.
This fails almost immediately because it thinks that clang is a broken compiler: config.log.
comment:21 Changed 9 years ago by
Aah sorry, I meant extract the spkg, apply the patches and then do what you did.
comment:22 Changed 9 years ago by
Okay, here is config.log and the output from running `./configure`.
comment:23 Changed 9 years ago by
Hmm, at first sight looks like a genuine compiler bug, but that's hard to say from here...
comment:24 Changed 9 years ago by
Could you try the same (i.e. manual ./configure
) with export CC=clang
?
comment:25 follow-up: ↓ 27 Changed 9 years ago by
With export CC=clang
: config.log and configure output.
By the way, running make
seems to succeed after all of these manual runs of ./configure
. Once I set CFLAGS
as Sage does -- that is,
export CFLAGS="-m32 -O2 -fomit-frame-pointer -mtune=corei7-avx -march=corei7-avx -g"
then do ./configure
and make
, it fails. Remove the -m32
flag and it works again. Can we just remove -m32
in the spkg?
comment:26 Changed 9 years ago by
It seems that gcc
is identical to clang
, so that didn't make a difference.
Can we just remove
-m32
in the spkg?
There is no -m32
in the spkg. The problem is that -m64
simply doesn't work, so MPIR's configure
falls back to -m32
.
comment:27 in reply to: ↑ 25 Changed 9 years ago by
Replying to jhpalmieri:
Remove the
-m32
flag and it works again.
Exactly what does "it" refer to here? Did you manage to ./configure
MPIR as 64-bit?
comment:28 Changed 9 years ago by
I mean that if I do
export CFLAGS="-O2 -fomit-frame-pointer -mtune=corei7-avx -march=corei7-avx -g" ./configure make
it finishes without error. (I haven't tried a full Sage build on top of this, of course.) If CFLAGS
also includes -m32
, then make
fails.
In the spkg, we could have some code like
-
spkg-install
diff --git a/spkg-install b/spkg-install
a b 275 275 echo " ABI: $user_abi" 276 276 echo " (CPP, CPPFLAGS, CXX and CXXFLAGS are listed below; these don't get modified.)" 277 277 278 # Fix bug with Xcode 5.0 on Darwin: mpir doesn't build using the -m32 279 # flag with the gcc from this version of Xcode. 280 if [ blah ]; then # using Xcode 5.0 on Darwin 281 mpir_cflags=... # remove -m32 from $mpir_cflags 282 fi 283 278 284 # Finally: use MPIR's flags, plus those required by Sage for the 279 285 # package to build properly, plus those specified by the user. 280 286 CFLAGS="$mpir_cflags $required_cflags $user_cflags"
I have no idea if this is actually a good idea, though.
comment:29 Changed 9 years ago by
Now I have tried a full Sage build on top of this, and the build finishes. I guess that isn't too surprising, because mpir gets rebuilt with Sage's GCC anyway.
I'll try again with a more carefully constructed spkg, and I'll also run the full test suite. Here's my actual proposed patch to spkg-install:
-
spkg-install
diff --git a/spkg-install b/spkg-install
a b 275 275 echo " ABI: $user_abi" 276 276 echo " (CPP, CPPFLAGS, CXX and CXXFLAGS are listed below; these don't get modified.)" 277 277 278 # Fix bug with Xcode 5.0 or later on Darwin: mpir doesn't build using 279 # the -m32 flag with the gcc from this version of Xcode, so remove 280 # -m32 from CFLAGS. 281 if [ "$UNAME" = "Darwin" ]; then 282 # Xcode version detection, etc., taken from spkg/base/prereq 283 XCODE_VERS=`xcodebuild -version 2> /dev/null | grep Xcode | sed -e 's/[A-Za-z ]//g'` 284 if [ -z $XCODE_VERS ]; then 285 XCODE_VERS="2" 286 fi 287 XCODE_VERS_MAJOR=`echo $XCODE_VERS | cut '-d.' -f1` 288 if [ $XCODE_VERS_MAJOR -gt 4 ]; then 289 mpir_cflags=`echo $mpir_cflags | sed 's/-m32//'` # remove -m32 290 fi 291 fi 292 278 293 # Finally: use MPIR's flags, plus those required by Sage for the 279 294 # package to build properly, plus those specified by the user. 280 295 CFLAGS="$mpir_cflags $required_cflags $user_cflags"
comment:30 Changed 9 years ago by
I used an spkg with this patch, took an otherwise clean tarball for 5.12.rc0, did export SAGE_CHECK=yes
. Then make ptestlong
was completely successful.
comment:31 follow-up: ↓ 35 Changed 9 years ago by
What does
$ gcc -E -dM -x c /dev/null |grep -i clang
say on that machine?
Instead of fiddling with CFLAGS
, I propose to try to "fix" the test causing the 64-bit build to fail.
comment:32 Changed 9 years ago by
I prepared a new version of the spkg with a simple fix, can you test it? Simply try to install the spkg normally through Sage. If that fails, send me the file configure-empty.log
Changed 9 years ago by
comment:33 Changed 9 years ago by
- Description modified (diff)
comment:34 Changed 9 years ago by
- Description modified (diff)
comment:35 in reply to: ↑ 31 ; follow-up: ↓ 36 Changed 9 years ago by
Replying to jdemeyer:
What does
$ gcc -E -dM -x c /dev/null |grep -i clangsay on that machine?
gcc -E -dM -x c /dev/null |grep -i clang #define __VERSION__ "4.2.1 Compatible Apple LLVM 5.0 (clang-500.2.76)" #define __clang__ 1 #define __clang_major__ 5 #define __clang_minor__ 0 #define __clang_patchlevel__ 0 #define __clang_version__ "5.0 (clang-500.2.76)"
The new spkg seems to work. I'm trying a full Sage build to make sure.
comment:36 in reply to: ↑ 35 Changed 9 years ago by
Replying to jhpalmieri:
gcc -E -dM -x c /dev/null |grep -i clang #define __VERSION__ "4.2.1 Compatible Apple LLVM 5.0 (clang-500.2.76)" #define __clang__ 1 #define __clang_major__ 5 #define __clang_minor__ 0 #define __clang_patchlevel__ 0 #define __clang_version__ "5.0 (clang-500.2.76)"
This confirms my suspicion that gcc
really is Clang.
comment:37 Changed 9 years ago by
- Description modified (diff)
comment:38 follow-up: ↓ 39 Changed 9 years ago by
- Priority changed from major to blocker
- Reviewers set to John Palmieri
- Status changed from needs_review to positive_review
The doc patch is certainly fine. As far as the spkg goes, first, is it worth it to rebase the patch files so we don't get the "offset -128 lines" messages? Second, it seems to work, and I understand in principle what the changes are, but I'm not an expert in acinclude.m4
files. I think I'll give it a positive review anyway: this ticket is very important (I think a blocker) since anyone with OS X and the most recent Xcode won't be able to build Sage without it.
comment:39 in reply to: ↑ 38 Changed 9 years ago by
- Milestone changed from sage-5.13 to sage-5.12
Replying to jhpalmieri:
As far as the spkg goes, first, is it worth it to rebase the patch files so we don't get the "offset -128 lines" messages?
In my opinion, no. Offsets are harmless.
this ticket is very important (I think a blocker)
I agree. In fact, it justifies an extra rc for sage-5.12.
Changed 9 years ago by
comment:40 Changed 9 years ago by
By the way, thank you for taking care of this so quickly after my message to sage-devel. It wasn't so urgent until Apple released Xcode 5.0.
comment:41 Changed 9 years ago by
- Merged in set to sage-5.12.rc1
- Resolution set to fixed
- Status changed from positive_review to closed
comment:42 in reply to: ↑ description ; follow-up: ↓ 43 Changed 7 years ago by
Replying to jpflori:
There are two problems with the MPIR
configure
script for Clang:1) It uses
__builtin_malloc()
which Clang doesn't have, which is an MPIR upstream bug (up to and including MPIR 2.6.0). The spkg simply removes this test.2) The "long long reliability" test fails with a linker error. This looks like a Clang bug.
The latter one apparently now hits GCC (5.0.1 RC1) as well... B)
(From config.log
it also seems that the __builtin_alloca()
availability test fails for a different reason, but that's harmless.)
comment:43 in reply to: ↑ 42 ; follow-up: ↓ 44 Changed 7 years ago by
Replying to leif:
2) The "long long reliability" test fails with a linker error. This looks like a Clang bug.
The latter one apparently now hits GCC (5.0.1 RC1) as well... B)
The difference appears to be
#define __GNUC_STDC_INLINE__ 1
in GCC 5.x vs.
#define __GNUC_GNU_INLINE__ 1
in older versions. (In each case the other macro isn't defined, corresponding to 0
.)
comment:44 in reply to: ↑ 43 Changed 7 years ago by
Replying to leif:
Replying to leif:
2) The "long long reliability" test fails with a linker error. This looks like a Clang bug.
The latter one apparently now hits GCC (5.0.1 RC1) as well... B)
The difference appears to be
#define __GNUC_STDC_INLINE__ 1
in GCC 5.x vs.
#define __GNUC_GNU_INLINE__ 1
in older versions. (In each case the other macro isn't defined, corresponding to
0
.)
Follow-up: #18247
Pinging myself... ;-)
(I do have a trivial patch to
acinclude.m4
to make MPIR configure [and of course build and pass its test suite] withclang
. Haven't yet submitted it upstream either IIRC.)