Opened 11 years ago
Last modified 10 years ago
#11616 closed enhancement
Upgrade MPIR to a more recent upstream release — at Version 76
Reported by: | leif | Owned by: | leif |
---|---|---|---|
Priority: | blocker | Milestone: | sage-5.0 |
Component: | packages: standard | Keywords: | sd32, GMP, SandyBridge, Westmere, yasm re2c race condition, Linux ia64 Itanium GCC 4.7.0 bug |
Cc: | justin, wbhart, jpflori | Merged in: | |
Authors: | Leif Leonhardy, Jeroen Demeyer | Reviewers: | Jeroen Demeyer, Leif Leonhardy, Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This is a follow-up to #8664 (and a couple of other tickets).
The following new spkg is based on the latest MPIR 2.1.3 spkg, the p9 from #12131:
New spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/mpir-2.4.0.p3.spkg
mpir-2.4.0.p3 (Jeroen Demeyer, April 26th, 2012)
Trac #11616, reviewer fixes:
- When the first configure run (with CFLAGS unset) of MPIR fails, bail
out with an error. I am not aware of any system where MPIR fails
to configure with CFLAGS unset but succeeds with CFLAGS set.
This implies the following simplifications:
- We no longer read CC and CFLAGS from /usr/include/gmp.h or /usr/local/include/gmp.h
- We no longer try to add -march=native, we simply use MPIR's flags.
- Extract $CC and $CFLAGS from Makefile instead of mpir.h, which is simpler and more reliable.
- Added
quote_asm.patch
to add proper quoting to the m4 in .asm files. - Use
patch
to patch gmp-h.in instead of copying the file. - In get_processor_specific_cflags() in spkg-install, also check for -mpower* and -mno-power* flags, as MPIR adds -mpower on 32-bit PowerPC systems.
mpir-2.4.0.p2 (Leif Leonhardy, April 4th, 2012)
#11616 (upgrading MPIR), further fixes:
- Before enabling
-march=native
, minimalistically check whether the system's assembler also understands the instructions the compiler emits with that option. (Work-around for e.g. GCC 4.6.3 on MacOS X 10.x and Intel Core i7-family CPUs with AVX.) - Do not unconditionally unset
PYTHON
, since Sage (>=5.0.beta10) no longer pollutes the environment with its package version variables, which previous- ly confused yasm'sconfigure
. - Fix extraction of
__GMP_CC
and__GMP_CFLAGS
fromgmp.h
, since MPIR meanwhile defines these to preprocessor variables (rather than literals). Also don't use\+
insed
patterns, as this is less portable. - Work around GCC 4.7.0 bug (compilation error) on Linux ia64 (Itanium) by almost completely disabling optimization on that platform if GCC 4.7.x is detected. This doesn't hurt much if we later rebuild MPIR with a (non- broken) GCC from the new GCC spkg. Cf. #12765.
- Do not build the C++ interface and static libraries when bootstrapping the
GCC spkg, i.e. if
SAGE_BUILD_TOOLCHAIN=yes
. (GMP/MPIR is a prerequisite for it, and MPIR will later get rebuilt with both enabled, with the newly built GCC.) Cf. #12782. - Fix a potential race condition in yasm's build by patching the re2c source. Cf. #11844.
- Add "
patch
loop" to apply any patches (*.patch
) located inpatches/
. Currently only the re2c patch matches that; the prepatched header to support Sun's C compiler is still copied over (and only on SunOS, although it doesn't do any harm on other platforms). - Minor clean-up; e.g. redirect error messages and warnings to
stderr
, quote parameter to--libdir
, add some comments and messages, also save user's setting ofLDFLAGS
andABI
.
mpir-2.4.0.p1 (Leif Leonhardy, March 21st, 2012)
- Upstream upgrade to MPIR 2.4.0 (#11616). The 2.4.0.p0 spkg isn't in this history, as it was based on the 2.1.3.p4 spkg, i.e., is "on another branch", and never got merged into Sage.
- Remove forcing a sequential
make install
again, since the potential race condition was fixed in MPIR 2.1.4. - Fix
.hgtags
, which contained duplicate entries, and was missing others.
This fixes also:
- #11844: a potential race condition due to yasm when building MPIR in parallel. We've never run into this [before] though. The MPIR 2.4.0.p2 spkg now includes a patch to upstream fixing that.
- #12782: when building MPIR to bootstrap GCC (i.e. when
SAGE_BUILD_TOOLCHAIN=yes
), do not build the C++ interface (and not the static library). This would allow to build Sage on systems which have a C compiler but not a C++ compiler, and also saves time.
The list of changes between MPIR 2.1.3 (more precisely, 2.1.1) and MPIR 2.4.0 is fairly long, so I haven't put them into the description, but attached them in a plain text file.
To install / test the spkg, it is sufficient to just
- Download the new spkg and copy it into
$SAGE_ROOT/spkg/standard/
.
- Install the MPIR spkg:
$ ./sage -f spkg/standard/mpir-<version>.spkg
- Re-install all packages depending on MPIR:
(or omit
$ env SAGE_UPGRADING=yes make build
build
to also rebuild the documentation in the samemake
run).
To run just MPIR's test suite, you can reinstall the spkg with SAGE_CHECK=yes
:
$ env SAGE_CHECK=yes ./sage -f spkg/standard/mpir-<version>.spkg
Or, if you haven't yet installed the spkg (but copied it into $SAGE_ROOT/spkg/standard/
as mentioned above), do:
$ env SAGE_CHECK=yes ./sage -i spkg/standard/mpir-<version>.spkg $ env SAGE_UPGRADING=yes make build # rebuilds all dependent packages
Afterwards you can run make doc
to (re)build the documentation, and / or make ptestlong
to run Sage's full test suite in parallel.
Change History (80)
comment:1 Changed 11 years ago by
- Description modified (diff)
Changed 11 years ago by
comment:2 follow-up: ↓ 3 Changed 11 years ago by
- Description modified (diff)
- Status changed from new to needs_review
Setting this to "needs review" since the MPIR 2.1.3.p4 from #8664 got positive review again, though so far only by a single reviewer.
The current packages are mainly meant for testing the new upstream releases, hopefully on a variety of platforms supported by Sage; some improvements or changes to Sage's part will most probably follow.
It would just be nice to relatively early know whether any of them (more important, MPIR 2.4.0) causes any problems on one of our platforms. (MPIR 2.5.0 is scheduled for September 1st, which isn't that far...)
comment:3 in reply to: ↑ 2 Changed 11 years ago by
Replying to leif:
(MPIR 2.5.0 is scheduled for September 1st, which isn't that far...)
News from mpir-devel:
"The release date for MPIR-2.5 was penciled in for 1st Sept, however we have decided to put it back a month to 1st Oct to allow us to get some more code in this release rather than wait for the MPIR-2.6 release on 1st Dec."
comment:4 Changed 11 years ago by
I've successfully built and tested sage using the 2.4.0.p0 spkg on skynet/eno. I'm currently building/testing on some other machines as well (although, I don't have access to a ppc mac, so I can't help in that regard).
comment:5 Changed 11 years ago by
Hmm, well I'm getting some random test failures on sage.math and skynet/iras with 2.4.0.p0. I'll try again with 2.3.1.p0 and report back.
comment:6 Changed 11 years ago by
- Keywords sd32 added
comment:7 Changed 11 years ago by
- Description modified (diff)
comment:8 Changed 11 years ago by
- Description modified (diff)
Added a reference to #11844 (potential race condition due to yasm when building MPIR in parallel).
comment:9 Changed 11 years ago by
- Description modified (diff)
Added a note on the need to set ABI=32
on 64-bit machines running 32-bit operating systems. (The same is true for the already merged MPIR 2.1.3.p4 spkg.)
Also added a reference to #9858; FLINT 1.5.0's test suite won't build with any of the MPIR 2.x spkgs because it uses deprecated functions, and updated the installation instructions.
comment:10 Changed 11 years ago by
- Status changed from needs_review to needs_work
- Work issues set to Rebase the spkg(s) on the MPIR 2.1.3.p5 spkg from #11896.
comment:11 Changed 11 years ago by
- Dependencies #8664 #5847 deleted
- Description modified (diff)
- Work issues changed from Rebase the spkg(s) on the MPIR 2.1.3.p5 spkg from #11896. to Rebase the spkg(s) on the MPIR 2.1.3.p7 spkg from #11964
comment:12 Changed 10 years ago by
- Work issues changed from Rebase the spkg(s) on the MPIR 2.1.3.p7 spkg from #11964 to Rebase the spkg(s) on the MPIR 2.1.3.p9 spkg from #12131
comment:13 Changed 10 years ago by
MPIR 2.5.1 has been released on March 11th. We could also give that a try, although 2.4.0 has IMHO been tested a lot and didn't raise any problems within Sage, AFAIK, at least apparently not reproducible ones.
comment:14 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
- Work issues Rebase the spkg(s) on the MPIR 2.1.3.p9 spkg from #12131 deleted
It's not unlikely that I'll soon provide an MPIR 2.4.0.p2 spkg, also fixing a potential race condition in the yasm
build, as well as an "experimental" MPIR 2.5.1 spkg.
Please build, test, and review!
comment:15 Changed 10 years ago by
The following can be removed now:
# The Yasm build uses PYTHON from the environment to find python, so unset # it since the setting from 'spkg/standard/newest_version' confuses it: unset PYTHON # This can be removed once #10492 has been merged, which turns the "package # version" environment variables into solely `make` variables, such that the # (shell) environment does no longer get polluted.
Also: do you feel like adding a work-around for #12765:
# Work around a bug in GCC 4.7.0 on ia64: # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48496 if [ "`uname -m`" = ia64 ] && [ "`testcc.sh $CC`" = GCC ] ; then if $CC -dumpversion | grep >/dev/null '^4\.7\.0$' ; then echo >&2 "Warning: Working around bug in gcc 4.7.0" CFLAGS="$CFLAGS -O0" fi fi
After sage-5.0.beta12 is released, I plan to test this spkg on the buildbots.
comment:16 Changed 10 years ago by
Also: could you use patch
for patching? Looking at patches/gmp-h.in.diff
, there is no need for a conditional patch. The patch will only actually do something if __SUNPRO_CC
is defined.
comment:17 follow-up: ↓ 18 Changed 10 years ago by
Note that I have a mpir-2.1.3.p10 at #12782, we should coordinate somehow.
comment:18 in reply to: ↑ 17 ; follow-up: ↓ 19 Changed 10 years ago by
comment:19 in reply to: ↑ 18 Changed 10 years ago by
comment:20 follow-up: ↓ 21 Changed 10 years ago by
If you're creating a new spkg anyway, could you incorporate my changes from #12782?
comment:21 in reply to: ↑ 20 ; follow-up: ↓ 22 Changed 10 years ago by
comment:22 in reply to: ↑ 21 Changed 10 years ago by
Replying to leif:
Replying to jdemeyer:
If you're creating a new spkg anyway, could you incorporate my changes from #12782?
Yes, although I kept the
unset PYTHON
, since it is safe and otherwise doesn't hurt.
We can also skip building a static library when building just for GCC in the first place I think.
Changed 10 years ago by
Diff between the previous spkg in Sage and my new p1 spkg. For reference / review only.
comment:23 Changed 10 years ago by
- Cc justin added
- Description modified (diff)
- Keywords yasm re2c race condition Linux ia64 Itanium GCC 4.7.0 bug added
I've uploaded a second, p2 spkg with further fixes, also incorporating [the] changes from / for
- #11844 (potential race condition in
yasm
build), - #12765 (build error on Linux ia64 (Itanium) with GCC 4.7.0, which is a GCC bug),
- #12782 (build MPIR without C++ interface [and without static libraries] when bootstrapping the GCC spkg),
configure
error with the GCC spkg on MacOS X (on a Core i7 with AVX), as reported on sage-release.
See the changelog entries in the description and the attached diffs for details and further changes.
The p2 is still somewhat preliminary (subject to further testing), although I don't expectTM further changes to be necessary, despite that haven't committed the changes yet.
comment:24 Changed 10 years ago by
- Description modified (diff)
comment:25 Changed 10 years ago by
- Description modified (diff)
comment:26 follow-up: ↓ 27 Changed 10 years ago by
Why don't you simply remove lines 179--189 regarding PYTHON? I don't think it serves any purpose anymore. Also, it shouldn't be needed for upgrades since PYTHON
is only set by the old spkg/install
and spkg/install
is always immediately upgraded.
comment:27 in reply to: ↑ 26 ; follow-up: ↓ 28 Changed 10 years ago by
Replying to jdemeyer:
Why don't you simply remove lines 179--189 regarding PYTHON? I don't think it serves any purpose anymore.
It does, because occasionally people (try to) install newer MPIR spkgs into older (e.g. 4.8) Sage installations. And doesn't hurt anyway, just makes Sage more robust.
comment:28 in reply to: ↑ 27 Changed 10 years ago by
Replying to leif:
It does, because occasionally people (try to) install newer MPIR spkgs into older (e.g. 4.8) Sage installations. And doesn't hurt anyway
It does hurt at least a little simply because it makes spkg-install
and SPKG.txt
larger than needed. I would remove it (but I'm not setting the ticket to needs_work for this).
comment:29 Changed 10 years ago by
Also: could you use patch
for patching also the file gmp-h.in
unconditionally? I think having both patches and copied files is just asking for trouble.
comment:30 follow-up: ↓ 31 Changed 10 years ago by
Replying to jdemeyer:
Replying to leif:
It does, because occasionally people (try to) install newer MPIR spkgs into older (e.g. 4.8) Sage installations. And doesn't hurt anyway
It does hurt at least a little simply because it makes
spkg-install
andSPKG.txt
larger than needed. I would remove it (but I'm not setting the ticket to needs_work for this).
Of course. But regarding such, I wouldn't have opened dozens of tickets and touched as many spkgs for the "newer autotools --libdir
issue"; a single symbolic link from $SAGE_LOCAL/lib64
to ./lib
would have sufficed, as we don't mix 32- and 64-bit builds anyway. IIRC someone now wants to create the lib64
directory (or a link?)...
comment:31 in reply to: ↑ 30 Changed 10 years ago by
Replying to leif:
But regarding such, I wouldn't have opened dozens of tickets and touched as many spkgs for the "newer autotools
--libdir
issue";
Agreed, I don't think we went with the best solution for that.
comment:32 Changed 10 years ago by
Replying to jdemeyer:
Also: could you use
patch
for patching also the filegmp-h.in
unconditionally? I think having both patches and copied files is just asking for trouble.
I don't think it's asking for trouble, but see the Special Update/Build Instructions / TODO.
I so far was just too lazy, and -- more important -- didn't want to remove Dave's messages and comments... ;-)
comment:33 follow-up: ↓ 35 Changed 10 years ago by
Concerning __GMP_CC
and __GMP_CFLAGS
: how about simply running the preprocessor on
#include "$header_file" sage_gmp_cc=__GMP_CC
and then sed
ding out the last line?
comment:34 Changed 10 years ago by
Leif: leave your package as it is. I might make a reviewer patch fixing a few small things.
comment:35 in reply to: ↑ 33 Changed 10 years ago by
Replying to jdemeyer:
Concerning
__GMP_CC
and__GMP_CFLAGS
: how about simply running the preprocessor on#include "$header_file" sage_gmp_cc=__GMP_CCand then
sed
ding out the last line?
Well, that has a couple of pitfalls as well, since such headers contain a lot, have conditional parts, and usually include other files, such that one would probably have to set up include directories as well as potentially preprocessor variables, too. sed
is faster and easier; I could also use [e]grep
(and "postprocess" its output), although I prefer the way it's made now.
comment:36 Changed 10 years ago by
- Status changed from needs_review to needs_work
This fails on hawk (OpenSolaris? 06.2009-32), I'll investigate:
m4 -DHAVE_CONFIG_H -D__GMP_WITHIN_GMP -DOPERATION_popcount -DPIC popcount.asm >tmp-popcount.s libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_perfect_square_p -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c perfect_square_p.c -fPIC -DPIC -o .libs/perfect_square_p.o libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_bdivmod -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c bdivmod.c -fPIC -DPIC -o .libs/bdivmod.o /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo gcd | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o gcd.lo gcd.c m4 -DHAVE_CONFIG_H -D__GMP_WITHIN_GMP -DOPERATION_hamdist -DPIC hamdist.asm >tmp-hamdist.s libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_gcd -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c gcd.c -fPIC -DPIC -o .libs/gcd.o libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_perfect_square_p -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c perfect_square_p.c -o perfect_square_p.o >/dev/null 2>&1 /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo gcd_1 | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o gcd_1.lo gcd_1.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_bdivmod -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c bdivmod.c -o bdivmod.o >/dev/null 2>&1 libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_gcd_1 -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c gcd_1.c -fPIC -DPIC -o .libs/gcd_1.o /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo gcdext | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o gcdext.lo gcdext.c /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo tdiv_qr | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o tdiv_qr.lo tdiv_qr.c /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo jacobi_base | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o jacobi_base.lo jacobi_base.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_gcd_1 -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c gcd_1.c -o gcd_1.o >/dev/null 2>&1 libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_gcdext -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c gcdext.c -fPIC -DPIC -o .libs/gcdext.o libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_tdiv_qr -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c tdiv_qr.c -fPIC -DPIC -o .libs/tdiv_qr.o libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_jacobi_base -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c jacobi_base.c -fPIC -DPIC -o .libs/jacobi_base.o /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo get_d | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o get_d.lo get_d.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_gcd -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c gcd.c -o gcd.o >/dev/null 2>&1 m4:hamdist.asm:41 bad macro name define(,1) libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_get_d -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c get_d.c -fPIC -DPIC -o .libs/get_d.o /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo mullow_n | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o mullow_n.lo mullow_n.c /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo mulhigh_n | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o mulhigh_n.lo mulhigh_n.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_jacobi_base -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c jacobi_base.c -o jacobi_base.o >/dev/null 2>&1 libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_mullow_n -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c mullow_n.c -fPIC -DPIC -o .libs/mullow_n.o m4:popcount.asm:41 bad macro name define(,1) make[4]: *** [popcount.lo] Error 1 /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo mullow_n_basecase | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o mullow_n_basecase.lo mullow_n_basecase.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_mulhigh_n -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c mulhigh_n.c -fPIC -DPIC -o .libs/mulhigh_n.o libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_mullow_n_basecase -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c mullow_n_basecase.c -fPIC -DPIC -o .libs/mullow_n_basecase.o libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_get_d -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c get_d.c -o get_d.o >/dev/null 2>&1 /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo mullow_basecase | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o mullow_basecase.lo mullow_basecase.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_mullow_n -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c mullow_n.c -o mullow_n.o >/dev/null 2>&1 /bin/sh ../libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_`echo redc_1 | sed 's/_$//'` -m32 -O2 -fomit-frame-pointer -mtune=core2 -march=core2 -c -o redc_1.lo redc_1.c make[4]: *** [hamdist.lo] Error 1
comment:37 Changed 10 years ago by
- Cc wbhart added
Looks like an upstream bug to me: they write
define(OPERATION_popcount, 1)
when they probably mean
define(`OPERATION_popcount', 1)
comment:38 Changed 10 years ago by
Reported this m4 quoting bug to mpir-devel.
comment:39 Changed 10 years ago by
- Description modified (diff)
comment:40 Changed 10 years ago by
It's much simpler to read CC and CFLAGS from Makefile, so I'll make that change.
comment:41 Changed 10 years ago by
I'm confused. Why are we reading /usr/include/gmp.h
???
comment:42 Changed 10 years ago by
It looks like the flags from /usr/include/gmp.h are only used when the flags from MPIR could not be read. But that means that ./configure failed, so we won't be able to build anyway...
comment:43 Changed 10 years ago by
Work in progress, based on your .p2: http://boxen.math.washington.edu/home/jdemeyer/spkg/mpir-2.4.0.p2.spkg
comment:44 Changed 10 years ago by
- Priority changed from major to blocker
- Reviewers set to Jeroen Demeyer
comment:45 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:46 Changed 10 years ago by
Leif, I simplified the spkg-install by bailing out with an error if the first configure run fails. This means I could remove the part which reads CC/CFLAGS from /usr/include/gmp.h and the part which tries to add -march=native.
The new spkg was tested with success on: bsd (OS X 10.6 x86_64), hawk (OpenSolaris x86 32bit), boxen (Linux x86_64), moufang (OS X 10.4 ppc), silius (Linux ppc), silius (Linux ppc64), cleo (Linux ia64 with GCC-4.7.0).
comment:47 Changed 10 years ago by
- Description modified (diff)
comment:48 follow-ups: ↓ 51 ↓ 52 Changed 10 years ago by
- Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Leif Leonhardy
It would have been better if you made a p3, and not deleted my diff with the changes of my p2.
I agree with some, but not all of your changes:
- There's no need to remove the conditional unsetting of
PYTHON
, as I already argued above.
- I wouldn't add a patch for the
.asm
files, but instead "patch" them (withsed
) on the fly. Otherwise this just blows up the Mercurial repository, and hopefully upstream will fix this in the next release. (MPIR 2.5.2 was originally scheduled for about one or two weeks ago.)
- Reading
CC
andCFLAGS
from system-widegmp.h
files is also for informational purposes. Using the strings from that header (and not MPIR'sMakefile
, which btw. could contain tabs as well) is the safer way, as these definitions are documented and supposed to be (easily read out and) used by other programs. (The [structure of the]Makefile
is subject to change without notice.) You should also check whether reading theCFLAGS
from theMakefile
apparently succeeded.
- I don't know why you removed the
-march=native
stuff; I does in general make sense, especially since (at least the version in Sage -- also regarding how long it takes to get a "new" version in) will not always properly detect the CPU, i.e., will choose a more generic target CPU [family] than-march=native
does.
- It's not necessary to move the work-around for GCC 4.7.x on ia64, as we don't support Itanium on anything but Linux. Moreover, using the odd
testcc.sh
script fails if$CC
contains more than one word, and using shell pattern matching is not only more efficient but also more readable.
- I'm pretty sure we still need a work-around for newer CPUs (and GCCs) on MacOS X, slightly different to what I previously added to the p2, at least to avoid strange error messages.
comment:49 Changed 10 years ago by
- Description modified (diff)
comment:50 Changed 10 years ago by
I am not aware of any system where MPIR fails to configure with
CFLAGS
unset but succeeds withCFLAGS
set.
This can very well happen if e.g. the compiler defaults to some target CPU / instruction set the assembler doesn't fully support, and CFLAGS
restrict the architecture. Same for CFLAGS
working around some brokenness of the compiler.
comment:51 in reply to: ↑ 48 Changed 10 years ago by
Replying to leif:
It would have been better if you made a p3, and not deleted my diff with the changes of my p2.
Since your changes weren't committed, I thought it was open to changes. Anyway, this would probably "blow up" the Mercurial repository more than my .asm patch ;-)
There's no need to remove the conditional unsetting of
PYTHON
, as I already argued above.
I really don't see the need for the PYTHON
stuff. If somebody just installs this spkg with "./sage -i
", then PYTHON
will not be set. PYTHON
will only be set if this spkg is installed through spkg/install
. I cannot think of a plausable scenario where this would happen.
I wouldn't add a patch for the
.asm
files, but instead "patch" them (withsed
) on the fly. Otherwise this just blows up the Mercurial repository
The patch isn't that large, so it's not going to "blow up" the Mercurial repository. Besides, I don't want to worry about the portability of the find/sed commands that I'm using.
Reading
CC
andCFLAGS
from system-widegmp.h
files is also for informational purposes.
Who reads log files anyway?
Using the strings from that header (and not MPIR's
Makefile
, which btw. could contain tabs as well) is the safer way, as these definitions are documented and supposed to be (easily read out and) used by other programs. (The [structure of the]Makefile
is subject to change without notice.)
Agreed that it's subject to change without notice, but at least we'll notice if it changes. Reading them from Makefile is absolutely the easiest way.
You should also check whether reading the
CFLAGS
from theMakefile
apparently succeeded.
This is harder to do. In theory, CFLAGS
could be empty. Moreover, it looks quite unlikely to me that reading CC
would succeed but reading CFLAGS
would fail.
I don't know why you removed the
-march=native
stuff;
I removed it because it wasn't used! Reading the CC and CFLAGS from MPIR should always succeed now (as far as I can tell), so the code using -march=native
would never be reached.
I does in general make sense, especially since (at least the version in Sage -- also regarding how long it takes to get a "new" version in) will not always properly detect the CPU, i.e., will choose a more generic target CPU [family] than
-march=native
does.
Perhaps always using -march=native
does make sense, but that's outside the scope of this ticket.
It's not necessary to move the work-around for GCC 4.7.x on ia64, as we don't support Itanium on anything but Linux.
So what? Why a priori restrict it to Linux?
Moreover, using the odd
testcc.sh
script fails if$CC
contains more than one word
Agreed, $CC
should be quoted.
using shell pattern matching is not only more efficient
Seriously? You're worrying about efficientcy of shell scripts?
but also more readable.
I find my code quite readable.
I'm pretty sure we still need a work-around for newer CPUs (and GCCs) on MacOS X, slightly different to what I previously added to the p2, at least to avoid strange error messages.
Let's see what users report... It doesn't make sense to me to add a complicated workaround for a corner case which doesn't occur in practice.
comment:52 in reply to: ↑ 48 Changed 10 years ago by
comment:53 follow-ups: ↓ 54 ↓ 58 Changed 10 years ago by
Replying to jdemeyer:
Replying to leif:
It would have been better if you made a p3, and not deleted my diff with the changes of my p2.
Since your changes weren't committed, I thought it was open to changes. Anyway, this would probably "blow up" the Mercurial repository more than my .asm patch ;-)
The raw patch alone is 32 KB. If one makes changes to other people's code, one should always make a diff; that's what reviewer patches are for. I also wanted to make further changes. Apart from that, it's confusing to have different spkgs with the same patch level at different locations.
There's no need to remove the conditional unsetting of
PYTHON
, as I already argued above.I really don't see the need for the
PYTHON
stuff. If somebody just installs this spkg with "./sage -i
", thenPYTHON
will not be set.PYTHON
will only be set if this spkg is installed throughspkg/install
. I cannot think of a plausable scenario where this would happen.
XD Just follow the instructions given in this ticket's description (and elsewhere):
$ cp mpir-x,y,z.pN.spkg "$SAGE_ROOT"/spkg/standard/ $ env SAGE_UPGRADING=yes make build
I wouldn't add a patch for the
.asm
files, but instead "patch" them (withsed
) on the fly. Otherwise this just blows up the Mercurial repositoryThe patch isn't that large, so it's not going to "blow up" the Mercurial repository. Besides, I don't want to worry about the portability of the find/sed commands that I'm using.
:-) Elsewhere we do...
Reading
CC
andCFLAGS
from system-widegmp.h
files is also for informational purposes.Who reads log files anyway?
Then why cat config.log
in case of an error, and give all the other messages? ;-)
Using the strings from that header (and not MPIR's
Makefile
, which btw. could contain tabs as well) is the safer way, as these definitions are documented and supposed to be (easily read out and) used by other programs. (The [structure of the]Makefile
is subject to change without notice.)Agreed that it's subject to change without notice, but at least we'll notice if it changes.
Not necessarily.
Reading them from Makefile is absolutely the easiest way.
Is it? You use almost the same sed
commands. And it's more error-prone; just imagine we have
CFLAGS = -foo -bar # This is a comment.
Then mpir_cflags
would certainly not be empty, but ... (Likewise for CC
.) Even the usage of one or more tabs can break your code.
You should also check whether reading the
CFLAGS
from theMakefile
apparently succeeded.This is harder to do. In theory,
CFLAGS
could be empty.
In that case we might (try to) add -march=native
, using the functions that have already been there... ;-)
Moreover, it looks quite unlikely to me that reading
CC
would succeed but readingCFLAGS
would fail.
See above. A tab character is sufficient, a comment leads to weird errors later.
I don't know why you removed the
-march=native
stuff;I removed it because it wasn't used! Reading the CC and CFLAGS from MPIR should always succeed now (as far as I can tell), so the code using
-march=native
would never be reached.
That was true for my p2 (provided upstream_cflags
weren't empty), and was subject to change, as I didn't notice that in the first place. And as you mentioned, MPIR's CFLAGS
may not contain processor-specific flags. Different compilers might use / need different flags, and the code was open for extensions regarding that.
I does in general make sense, especially since (at least the version in Sage -- also regarding how long it takes to get a "new" version in) will not always properly detect the CPU, i.e., will choose a more generic target CPU [family] than
-march=native
does.Perhaps always using
-march=native
does make sense, but that's outside the scope of this ticket.
Is it? But removing the code is in its scope? Doesn't make sense to first remove it here and later again add slightly changed code on another ticket.
It's not necessary to move the work-around for GCC 4.7.x on ia64, as we don't support Itanium on anything but Linux.
So what? Why a priori restrict it to Linux?
I didn't know you were porting MacOS to Itanium (a dead architecture btw.)...
Moreover, using the odd
testcc.sh
script fails if$CC
contains more than one wordAgreed,
$CC
should be quoted.
Perhaps rather change testcc.sh
, although
if $CC -E -x c /dev/null -dM 2>&1 | grep __GNUC__ >/dev/null; then # It's GCC. ... fi
does it, and is much simpler.
using shell pattern matching is not only more efficient
Seriously? You're worrying about efficientcy of shell scripts?
It's also more reliable. (And in general, we have packages where configure
takes longer than the whole build.)
but also more readable.
I find my code quite readable.
You mean like e.g.
if [ -n "$AS" -a "$AS" != "as" ]; then ...
instead of
if [[ $AS != as ]]; then ...
or
arg=`echo "$arg" | sed \ -e 's/^-\(compatibility_version\)/-dylib_\1/' \ -e 's/^-\(current_version\)/-dylib_\1/' \ -e 's/^-\(install_name\)/-dylib_\1/' \
instead of
case "$arg" in -compatibility_version) arg=-dylib_compatibility_version;; -current_version) arg=-dylib_current_version;; -install_name) arg=-dylib_install_name esac
?
I'm pretty sure we still need a work-around for newer CPUs (and GCCs) on MacOS X, slightly different to what I previously added to the p2, at least to avoid strange error messages.
Let's see what users report... It doesn't make sense to me to add a complicated workaround for a corner case which doesn't occur in practice.
Well, it's certainly a corner case (btw. like most of the regularly occuring MacOS trouble ;-) ), but it did happen, so I added a test to catch it, give an appropriate message, and work around it.
comment:54 in reply to: ↑ 53 Changed 10 years ago by
Replying to leif:
Well, it's certainly a corner case (btw. like most of the regularly occuring MacOS trouble ;-) ), but it did happen
It did happen with a different version of the MPIR spkg, so I don't see why that is relevant.
comment:55 Changed 10 years ago by
- Cc jpflori added
comment:56 Changed 10 years ago by
- Description modified (diff)
comment:57 Changed 10 years ago by
Leif, I took your .p2, committed the changes and based my .p3 on that. If a future version of the MPIR spkg wants to re-enable some of the code I deleted, it's in the Mercurial history, so it's not lost forever.
comment:58 in reply to: ↑ 53 ; follow-up: ↓ 63 Changed 10 years ago by
Replying to leif:
If one makes changes to other people's code, one should always make a diff; that's what reviewer patches are for. I also wanted to make further changes. Apart from that, it's confusing to have different spkgs with the same patch level at different locations.
Agreed, that's fixed now.
XD Just follow the instructions given in this ticket's description (and elsewhere):
I fixed your instructions.
The patch isn't that large, so it's not going to "blow up" the Mercurial repository. Besides, I don't want to worry about the portability of the find/sed commands that I'm using.
:-) Elsewhere we do...
Again: since I'm not sure how to do it in a portable way, I prefer to be safe and not do it.
Reading them from Makefile is absolutely the easiest way.
Is it? You use almost the same
sed
commands. And it's more error-prone; just imagine we haveCFLAGS = -foo -bar # This is a comment.Then
mpir_cflags
would certainly not be empty, but ... (Likewise forCC
.) Even the usage of one or more tabs can break your code.
This is all true, but the point is that MPIR (actually, automake) doesn't put those comments, nor do they use TABs. We cannot predict how Makefile
will look like in the future, but we also cannot predict how gmp.h
or mpir.h
will look like in the future. I think it is very unlikely that a future MPIR will break my script in a way that nobody notices.
Is it? But removing the code is in its scope?
The removed code wasn't used. Feel free to make a new MPIR spkg to re-enable -march=native
. Even better: fix the upstream code to add march=native
if supported.
comment:59 Changed 10 years ago by
Justin Walker reported success on OS X 10.7 with my previous .p2 spkg (essentially equal to the current .p3). The relevant part of the log:
**************************************************** Host system: Darwin Eisenstein.local 11.1.0 Darwin Kernel Version 11.1.0: Tue Jul 26 16:07:11 PDT 2011; root:xnu-1699.22.81~1/RELEASE_X86_64 x86_64 **************************************************** C compiler: gcc C compiler version: Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/private/tmp/sage-5.0.beta12-gcc/local/libexec/gcc/x86_64-apple-darwin11.1.0/4.6.3/lto-wrapper Target: x86_64-apple-darwin11.1.0 Configured with: ../src/configure --prefix=/private/tmp/sage-5.0.beta12-gcc/local --with-local-prefix=/private/tmp/sage-5.0.beta12-gcc/local --with-gmp=/private/tmp/sage-5.0.beta12-gcc/local --with-mpfr=/private/tmp/sage-5.0.beta12-gcc/local --with-mpc=/private/tmp/sage-5.0.beta12-gcc/local --with-system-zlib --disable-multilib --enable-checking=yes Thread model: posix gcc version 4.6.3 (GCC) **************************************************** [...] Settings chosen by MPIR when configuring with CFLAGS unset: CC: gcc -std=gnu99 CFLAGS: -O2 -m64 -march=corei7 -mtune=corei7-avx Settings required to properly build MPIR, taking into account SAGE_DEBUG etc.: CFLAGS: LDFLAGS: ABI: Settings from the "global" environment: CC: gcc CFLAGS: LDFLAGS: ABI: (CPP, CPPFLAGS, CXX and CXXFLAGS are listed below; these don't get modified.) Using MPIR's settings (plus mandatory ones). Finally using the following settings: CC=gcc CFLAGS=-O2 -m64 -march=corei7 -mtune=corei7-avx CPP= CPPFLAGS= CXX=g++ CXXFLAGS= LDFLAGS= ABI= (These settings may still get overridden by 'configure' or Makefiles.) Configuring MPIR with the following options: --prefix="/private/tmp/sage-5.0.beta12-gcc/local" --libdir="/private/tmp/sage-5.0.beta12-gcc/local/lib" --enable-cxx --enable-static --enable-gmpcompat --enable-shared
comment:60 Changed 10 years ago by
I restored the unset PYTHON
part.
I also added a check for -mpower*
flags in spkg-install, as MPIR adds -mpowerpc
on 32-bit PowerPC systems (at least on Skynet silius in 32-bit mode).
The spkg is still in the same location, needs review.
comment:61 Changed 10 years ago by
- Description modified (diff)
comment:62 Changed 10 years ago by
Looks good to me. If you commit the outstanding changes then I'll give it a positive review.
comment:63 in reply to: ↑ 58 ; follow-ups: ↓ 64 ↓ 66 ↓ 67 Changed 10 years ago by
Replying to jdemeyer:
Replying to leif:
Reading them from Makefile is absolutely the easiest way.
Is it? You use almost the same
sed
commands. And it's more error-prone; just imagine we haveCFLAGS = -foo -bar # This is a comment.Then
mpir_cflags
would certainly not be empty, but ... (Likewise forCC
.) Even the usage of one or more tabs can break your code.This is all true, but the point is that MPIR (actually, automake) doesn't put those comments, nor do they use TABs. We cannot predict how
Makefile
will look like in the future, but we also cannot predict howgmp.h
ormpir.h
will look like in the future. I think it is very unlikely that a future MPIR will break my script in a way that nobody notices.
This doesn't make sense at all. Why do you refuse to use the documented, supposed (and hence safest) way (or location) to get the flags from? (And it's in no way more complicated than what you currently do. You could of course also run the preprocessor on gmp.h
, but that's IMHO slight overkill.)
The structure and/or contents of the generated Makefiles is likely to change with other versions of autotools, probably without intent of the MPIR developers.
In any case, you should have just changed the get_upstream_selected_cflags()
function, instead of removing it.
You should also add -mno-power*
to get_processor_specific_cflags()
, since at least a user might use such.
The other changes are reasonable, so I'm ok with the spkg modulo the useless changes w.r.t. extracting CFLAGS
(and as already mentioned, I'd just change the .asm
files on the fly, not add a large patch which is under revision control for that*).
Don't know whether we should cat
the whole config.log
in case configure
failed in the first place; I'd just print its full filename (and we don't have to "rename" it), like we do in some other spkgs.
[* The "autotools way" would by the way be to check whether m4 bails out on define(,1)
, and only if that's the case, modify the offending source files.]
comment:64 in reply to: ↑ 63 Changed 10 years ago by
Replying to leif:
Why do you refuse to use the documented, supposed (and hence safest) way (or location) to get the flags from?
Is it really documented as such? I don't find any mention of GMP_CC
or MPIR_CC
in the MPIR manual.
comment:65 Changed 10 years ago by
- Description modified (diff)
comment:66 in reply to: ↑ 63 Changed 10 years ago by
Replying to leif:
You should also add
-mno-power*
toget_processor_specific_cflags()
, since at least a user might use such.
Done.
comment:67 in reply to: ↑ 63 Changed 10 years ago by
Replying to leif:
Don't know whether we should
cat
the wholeconfig.log
in caseconfigure
failed in the first place; I'd just print its full filename (and we don't have to "rename" it), like we do in some other spkgs.
I see, we don't cat config.log
, but the otherwise normal output. (Must have been late...)
Still, I'd print the pathname of config.log
, which usually reveals what really went wrong, if configure
failed.
comment:68 Changed 10 years ago by
This builds successfully (and passes self-tests) for me on various platforms. I don't think I'm competent to review all of the patches; what can we do to make some progress on this ticket? (One very minor comment: in the last version, I would have used "hg rename" to change gmp-h.in.diff
to gmp-h.in.patch
.)
comment:69 Changed 10 years ago by
I think this ticket shows that there is a need for a better management of C(XX)FLAGS in Sage. Ideally via some shell script that is part of Sage, so that not every spkg has to reinvent the wheel. The whole approach of parsing the install of another spkg to recreate its CFLAGS is just wrong, and arguing how to make this bandaid less likely to break in the future is not particularly helpful.
But now is not the time to create some CFLAGS framework. The spkg here seems to work pretty well, is better what we currently ship, is easy enough to read to see what it does, and is unlikely to be improved upon with some minor patches. So lets ship it!
comment:70 Changed 10 years ago by
- Reviewers changed from Jeroen Demeyer, Leif Leonhardy to Jeroen Demeyer, Leif Leonhardy, Volker Braun
comment:71 Changed 10 years ago by
For the record:
There seems to be some progress on the GCC 4.7 on ia64 issue, but assuming that
- GCC 4.7.1 will be released in July, and
- there'll (hopefully) be some Sage 5.1 or whatever final release before that,
I think we can leave the GCC 4.7.x (rather than 4.7.0) work-around as is here. (Same for the MPFR, GMP-ECM and some other, not yet merged / positively reviewed tickets.)
comment:72 follow-up: ↓ 73 Changed 10 years ago by
Leif, do you agree with giving this ticket positive_review?
comment:73 in reply to: ↑ 72 ; follow-ups: ↓ 74 ↓ 75 Changed 10 years ago by
Replying to jdemeyer:
Leif, do you agree with giving this ticket positive_review?
Well, printing the path to config.log
in case the first configure
run failed is certainly a minor issue (although adding it is a minor change which wouldn't need large-scale testing as well).
Volker said he was waiting for the changes to be committed, and I haven't looked at the latest spkg (w.r.t. usual spkg sanity checks) myself, otherwise I would have set it to positive review already.
So I just don't know whether you were planning to make some further changes, and/or commit them.
comment:74 in reply to: ↑ 73 Changed 10 years ago by
Replying to leif:
So I just don't know whether you were planning to make some further changes, and/or commit them.
Also the description still states the p3 was preliminary.
comment:75 in reply to: ↑ 73 Changed 10 years ago by
Replying to leif:
Volker said he was waiting for the changes to be committed, and I haven't looked at the latest spkg (w.r.t. usual spkg sanity checks) myself, otherwise I would have set it to positive review already.
FWIW, the current(?) p3 spkg (md5sum 45bd5544cfa568e1d7752f335495c336
) looks technically sane to me (except that the changes aren't committed yet).
(I guess it's unnecessary to mention that I of course don't agree on parts of the changelog entry in SPKG.txt
.)
comment:76 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_review to positive_review
Added path to config.log
in case of failure, committed changes.
Note that some changes between 2.1.1 and 2.1.4 may be missing (documented elsewhere, i.e. at least on
mpir-devel
I think). The segfault was fixed in 2.1.3, the race condition in 2.1.4.