Ticket #12830 (closed enhancement: fixed)
Work around GCC 4.7.0 bug on ia64 and improve the GMP-ECM spkg
| Reported by: | leif | Owned by: | leif |
|---|---|---|---|
| Priority: | blocker | Milestone: | sage-5.0 |
| Component: | packages: standard | Keywords: | spkg -march=native assembler error Darwin MacOS __GMP_CFLAGS __MPIR_CFLAGS gmp.h GCC 4.7.0 ia64 Itanium bug impossible reload |
| Cc: | hedtke, justin | Work issues: | |
| Report Upstream: | N/A | Reviewers: | Jeroen Demeyer |
| Authors: | Leif Leonhardy | Merged in: | sage-5.0.rc0 |
| Dependencies: | Stopgaps: |
Description (last modified by leif) (diff)
GCC 4.7.0 is broken on ia64 (Itanium), which also affects the GMP-ECM spkg (cf. #12751).
Adding -march=native to CFLAGS may lead to assembler errors, e.g. on MacOS X with newer GCCs on newer CPUs (e.g. such supporting AVX, which Apple's assembler currently doesn't).
Newer versions of MPIR don't define __GMP_CFLAGS (in gmp.h) to a string literal, but instead to a preprocessor macro (__MPIR_CFLAGS), which in turn is defined to the string we want.
New spkg: http://boxen.math.washington.edu/home/leif/Sage/spkgs/ecm-6.3.p7.spkg
md5sum: a5e9a4b47d30c4aba2c77ada47579e1a ecm-6.3.p7.spkg
ecm-6.3.p7 (Leif Leonhardy, April 19th 2012)
- #12830: Add -m[no-]power* to the processor-specific options recognized; these are used to enable/disable specific instruction set extensions of the POWER and PowerPC (PPC) CPUs.
ecm-6.3.p6 (Leif Leonhardy, April 16th 2012)
- #12830: Add a work-around for GCC 4.7.x on ia64 (Itanium), since GMP-ECM currently won't build with that and anything but -O0 on that platform.
- Use \{1,\} instead of \+ in sed patterns, which is more portable.
- Also support newer system-wide MPIR installations for printing their settings.
- Use patch to apply patches. Since the pre-patched configure in patches/ was created with a newer version of autotools (or, rather, the original configure was created with an outdated version), the patch would have been almost as large as the patched configure file itself. Hence I autoreconfed the source tree with a patched configure.in (and almost the latest versions of autotools), then created a patch to configure from the resulting file(s). Note that therefore src/ isn't really vanilla any more, although just the auto-generated files differ (which are still made from vanilla upstream sources, including configure.in). Add a "Patches" subsection and update "Special Update/Build? Instructions". Remove files in patches/ from .hgignore (and also remove pre-patched files from that directory); the patch to configure and the diff of configure.in are [now] under revision control, which IMHO makes sense.
- Beautify (and simplify) the output with respect to options passed to configure; print the settings of a few more environment variables that GMP-ECM uses (in case they're set); add some messages, also mention --enable-assert etc. if SAGE_DEBUG=yes.
- Remove unused test for GCC.
ecm-6.3.p5 (Leif Leonhardy, April 11th 2012)
- #12830: Don't add -march=native if the assembler doesn't understand the instructions the compiler emits with that. (E.g. the Apple/XCode assembler on Darwin doesn't yet support AVX.)
- Fix extraction of __GMP_CC and __GMP_CFLAGS (from gmp.h) for newer versions of MPIR, which define these to preprocessor macros rather than strings.
- Redirect warnings and error messages to stderr; add some messages.
- Correct SPKG.txt w.r.t. applied patches.
Attachments
Change History
Changed 13 months ago by leif
-
attachment
ecm-6.3.p4-p5.diff
added
comment:1 Changed 13 months ago by leif
- Cc hedtke, justin added
- Status changed from new to needs_review
- Description modified (diff)
- Authors set to Leif Leonhardy
This new spkg (also) fixes issues (i.e., assembler errors) on MacOS X with the GCC spkg.
Please test and review!
comment:2 follow-up: ↓ 5 Changed 13 months ago by jdemeyer
Instead of copying all this complicated "detect CFLAGS from MPIR" stuff, I propose the following: make the MPIR spkg store its $CC and $CFLAGS somewhere (say, in $SAGE_LOCAL/etc/mpirflags) and read that? That would replace the horrible kludge we have now in this ecm spkg.
comment:3 follow-up: ↓ 6 Changed 13 months ago by jdemeyer
- Remove the reading of the system GMP header, as they aren't used anyway (there is no good reason to use them anyway).
- Remove unset RM which isn't needed anymore.
comment:4 follow-up: ↓ 7 Changed 13 months ago by jdemeyer
Concerning march=native: what do you think about using testcflags.sh for this? Let's add
unsigned long fancy_insns() { return (unsigned long) d; }
to the program that testcflags.sh uses to check flags.
Then we could do something like
if testcflags.sh -march=native >/dev/null; then
CFLAGS="-march=native $CFLAGS"
elif testcflags.sh "-march=native -mno-avx" >/dev/null; then
echo >&2 "Warning bla bla"
CFLAGS="-march=native -mno-avx $CFLAGS"
fi
comment:5 in reply to: ↑ 2 ; follow-up: ↓ 9 Changed 13 months ago by leif
Replying to jdemeyer:
Instead of copying all this complicated "detect CFLAGS from MPIR" stuff, I propose the following: make the MPIR spkg store its $CC and $CFLAGS somewhere (say, in $SAGE_LOCAL/etc/mpirflags) and read that? That would replace the horrible kludge we have now in this ecm spkg.
Well, using gmp.h is the documented, supposed way to do this, and compatible to GMP.
Extracting the flags isn't complicated (nor a kludge); complain to the MPIR developers that they break GMP compatibility even with --enable-gmpcompat (as I already did). The extra code working around this is six lines IIRC, and not really complicated.
If you want MPIR and GMP to store the flags elsewhere, suggest them to create a pkg-config file (although "standard" .pc files don't have a variable for the compiler used).
We could of course create our own in MPIR's (and GMP's) spkg-install, but that doesn't make much sense to me, since then again our other spkgs using the .pc file would depend on Sage's versions (and a recent version of Sage itself).
comment:6 in reply to: ↑ 3 Changed 13 months ago by leif
Replying to jdemeyer:
- Remove the reading of the system GMP header, as they aren't used anyway (there is no good reason to use them anyway).
It's for informational purposes; it's not bad to compare (potentially) MPIR's flags to those of a system-wide GMP (or MPIR) installation.
- Remove unset RM which isn't needed anymore.
It doesn't hurt, and doesn't break anything.
On the contrary, removing it may break the installation of the spkg on older versions of Sage.
comment:7 in reply to: ↑ 4 ; follow-up: ↓ 8 Changed 13 months ago by leif
Replying to jdemeyer:
Concerning march=native: what do you think about using testcflags.sh for this? Let's add
unsigned long fancy_insns() { return (unsigned long) d; }to the program that testcflags.sh uses to check flags.
Then we could do something like
if testcflags.sh -march=native >/dev/null; then CFLAGS="-march=native $CFLAGS" elif testcflags.sh "-march=native -mno-avx" >/dev/null; then echo >&2 "Warning bla bla" CFLAGS="-march=native -mno-avx $CFLAGS" fi
Probably. Although testcflags.sh is currently broken... ;-)
I'm not sure if we should do such a complicated test whenever some arbitrary compiler flag is to be tested. We'd never know whether the compiler, the assembler or the linker caused an error.
Using it again requires newer versions of Sage for little reason, which I don't like in general.
testcflags.sh might have an option to not use CFLAGS btw.; otherwise one would have to (unset CFLAGS; testcflags.sh ...) or specifically construct CFLAGS for the desired purpose.
comment:8 in reply to: ↑ 7 Changed 13 months ago by jdemeyer
Replying to leif:
Probably. Although testcflags.sh is currently broken... ;-)
In which sense is it broken???
I'm not sure if we should do such a complicated test whenever some arbitrary compiler flag is to be tested. We'd never know whether the compiler, the assembler or the linker caused an error.
Why does it matter whether the compiler, assembler or linker caused the problem? What matters is that it doesn't work.
Using it again requires newer versions of Sage for little reason, which I don't like in general.
That I noticed :-)
Using testcflags.sh looks like the cleaner and simpler solution to me.
testcflags.sh might have an option to not use CFLAGS btw.; otherwise one would have to (unset CFLAGS; testcflags.sh ...) or specifically construct CFLAGS for the desired purpose.
I think
CFLAGS= testcflags.sh -flag
is simple enough.
comment:9 in reply to: ↑ 5 ; follow-up: ↓ 10 Changed 13 months ago by jdemeyer
What's wrong with simply storing MPIR's flags in some easy-to-read file? That would be the most simple solution. Moreover, it would ensure that we don't need to change the ECM spkg (or any other) if either Sage or MPIR changes the CFLAGS handling for MPIR. We would only need to change the MPIR spkg.
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 11 Changed 13 months ago by leif
Replying to jdemeyer:
What's wrong with simply storing MPIR's flags in some easy-to-read file? That would be the most simple solution.
Why reinvent the wheel?
(And reading gmp.h isn't complicated at all, presumably because doing so is intended...)
Moreover, it would ensure that we don't need to change the ECM spkg (or any other) if either Sage or MPIR changes the CFLAGS handling for MPIR. We would only need to change the MPIR spkg.
I don't see the point. First of all, that would be incompatible to any other MPIR or GMP installation / package, and we'd certainly have to change all packages using such a file whenever we change "Sage's format" (or handling) of MPIR's flags / settings.
comment:11 in reply to: ↑ 10 Changed 13 months ago by jdemeyer
Replying to leif:
Replying to jdemeyer:
What's wrong with simply storing MPIR's flags in some easy-to-read file? That would be the most simple solution.
Why reinvent the wheel?
You are already reinventing the wheel by manually sedding the flags out of the header file, as opposed to using autoconf. So if we reinvent the wheel, let's reinvent it as simple as possible.
comment:12 Changed 13 months ago by jdemeyer
Besides, your wheel is broken as \+ isn't portable.
Changed 13 months ago by leif
-
attachment
ecm-6.3.p5-p6.diff
added
Diff between my p5 and p6 spkgs. For reference / review only.
Changed 13 months ago by leif
-
attachment
ecm-6.3.p4-p6.diff
added
Diff between the previous spkg in Sage and my new p6 spkg. For reference / review only.
comment:14 Changed 13 months ago by leif
- Keywords GCC 4.7.0 ia64 Itanium bug impossible reload added
- Reviewers set to Jeroen Demeyer
- Status changed from needs_work to needs_review
- Description modified (diff)
- Summary changed from Improve handling of CFLAGS in GMP-ECM's spkg-install to Work around GCC 4.7.0 bug on ia64 and improve the GMP-ECM spkg
I've made a p6 spkg with further fixes and improvements, now also working around the GCC 4.7.0 bug on ia64 (Itanium).
(See attached diffs and the spkg changelog entry in the description for details.)
comment:16 follow-up: ↓ 18 Changed 13 months ago by jdemeyer
In your case statement:
-march=*|-mcpu=*|-mtune*)
add -mpower* which MPIR also uses.
Of course, I still think the spkg-install is overly complicated and re-invents too many wheels. But since you don't want to give in, I guess I'll have to, otherwise this will never get merged.
comment:18 in reply to: ↑ 16 Changed 13 months ago by leif
- Status changed from needs_work to needs_review
- Description modified (diff)
Replying to jdemeyer:
In your case statement:
-march=*|-mcpu=*|-mtune*)add -mpower* which MPIR also uses.
Fixed in a p7. (I've added -mpower* and -mno-power*.)
Changed 13 months ago by leif
-
attachment
ecm-6.3.p6-p7.diff
added
Diff between my p6 and p7 spkgs. For reference / review only.
comment:20 follow-up: ↓ 21 Changed 13 months ago by ppurka
Is egrep available on all systems? Otherwise one can use grep -E.
comment:21 in reply to: ↑ 20 ; follow-up: ↓ 22 Changed 13 months ago by jdemeyer
Replying to ppurka:
Is egrep available on all systems? Otherwise one can use grep -E.
egrep is the more portable of these two: Solaris (at least some versions) does support egrep but not grep -E.
comment:22 in reply to: ↑ 21 Changed 13 months ago by leif
Replying to jdemeyer:
Replying to ppurka:
Is egrep available on all systems? Otherwise one can use grep -E.
egrep is the more portable of these two: Solaris (at least some versions) does support egrep but not grep -E.
And we do use egrep in a couple of other spkgs (and probably scripts, too) since quite a while by the way.
In general, the safest way would be to use some $EGREP, determined by Sage's configure, but that's a different story...
comment:23 Changed 13 months ago by vbraun
- Status changed from needs_review to positive_review
Looks fine.
In the long run it is probably unavoidable to disable optimizations on ia64 as nobody is going to fix bugs in the itanium optimizer at this point in time.
comment:24 follow-up: ↓ 26 Changed 13 months ago by vbraun
- Status changed from positive_review to needs_work
As reported on https://groups.google.com/d/msg/sage-devel/F8Y6DP3eSLM/zQM7HKzLt68J, this fails on SLES10 x86_64 with
libtool: compile: gcc -DHAVE_CONFIG_H -I. -I./x86_64 -I/home/rajeev/bin/sage-5.0.beta14/local/include -I/home/rajeev/bin/sage-5.0.beta14/local/include -march=native -g -O3 -fPIC -MT libecm_la-mul_fft.lo -MD -MP -MF .deps/libecm_la-mul_fft.Tpo -c mul_fft.c -o libecm_la-mul_fft.o : Assembler messages: :16797: Error: no such instruction: `pmulld %xmm2,%xmm0' :1023: Error: no such instruction: `pmulld %xmm2,%xmm0' :608: Error: no such instruction: `pmulld %xmm2,%xmm0' make[4]: *** [libecm_la-mul_fft.lo] Error 1
This seems to be a known feature of gcc -march=native sometimes emitting SSE4 asm that is not supported by the installed binutils. See also http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32062
comment:25 Changed 13 months ago by jdemeyer
It seems that MPIR is quite clever about choosing CFLAGS. So, as I have said before, I would simply use MPIR's CFLAGS. My feeling is that we should still merge this spkg as-is in sage-5.0 and continue thinking about it for the next Sage release.
comment:26 in reply to: ↑ 24 Changed 13 months ago by jdemeyer
Replying to vbraun:
This seems to be a known feature of gcc -march=native sometimes emitting SSE4 asm that is not supported by the installed binutils.
I don't think that gcc ever checks what the assembler supports. It just emits instructions and assumes that the assembler knows about them.
comment:27 follow-up: ↓ 28 Changed 13 months ago by vbraun
I agree, gcc just assumes that sufficiently recent binutils are installed. So we really would need to also bundle binutils and its required BFD and opcodes libraries, plus whatever else they use. It was probably a mistake to use a cutting-edge gcc binary, we should use one that is as old as possible while having all critical bugs fixed. Or have some framework for CFLAGS that is smart enough to not turn on SSE levels that the assembler does not support.
I'm fine with closing this ticket and fixing the SLES problems in the next release (and on another ticket).
comment:28 in reply to: ↑ 27 Changed 13 months ago by jdemeyer
Replying to vbraun:
we should use one that is as old as possible while having all critical bugs fixed.
That's exactly what I did :-)
comment:29 Changed 13 months ago by jdemeyer
- Status changed from needs_work to closed
- Resolution set to fixed
- Merged in set to sage-5.0.rc0

Diff between the previous spkg in Sage and my new p5 spkg. For reference / review only.