Opened 10 years ago
Closed 10 years ago
#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 | Merged in: | sage-5.0.rc0 |
Authors: | Leif Leonhardy | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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\+
insed
patterns, which is more portable. - Also support newer system-wide MPIR installations for printing their settings.
- Use
patch
to apply patches. Since the pre-patchedconfigure
inpatches/
was created with a newer version of autotools (or, rather, the originalconfigure
was created with an outdated version), the patch would have been almost as large as the patchedconfigure
file itself. Hence Iautoreconf
ed the source tree with a patchedconfigure.in
(and almost the latest versions of autotools), then created a patch toconfigure
from the resulting file(s). Note that thereforesrc/
isn't really vanilla any more, although just the auto-generated files differ (which are still made from vanilla upstream sources, includingconfigure.in
). Add a "Patches" subsection and update "Special Update/Build? Instructions". Remove files inpatches/
from.hgignore
(and also remove pre-patched files from that directory); the patch toconfigure
and the diff ofconfigure.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. ifSAGE_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
(fromgmp.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 (4)
Change History (33)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Cc hedtke justin added
- Description modified (diff)
- Status changed from new to needs_review
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 10 years ago by
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 10 years ago by
- 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 10 years ago by
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 10 years ago by
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 10 years ago by
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 10 years ago by
Replying to jdemeyer:
Concerning
march=native
: what do you think about usingtestcflags.sh
for this? Let's addunsigned 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 10 years ago by
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 useCFLAGS
btw.; otherwise one would have to(unset CFLAGS; testcflags.sh ...)
or specifically constructCFLAGS
for the desired purpose.
I think
CFLAGS= testcflags.sh -flag
is simple enough.
comment:9 in reply to: ↑ 5 ; follow-up: ↓ 10 Changed 10 years ago by
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 10 years ago by
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 10 years ago by
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 sed
ding 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 10 years ago by
Besides, your wheel is broken as \+
in sed
scripts isn't portable.
comment:13 Changed 10 years ago by
- Status changed from needs_review to needs_work
Changed 10 years ago by
Diff between the previous spkg in Sage and my new p6 spkg. For reference / review only.
comment:14 Changed 10 years ago by
- Description modified (diff)
- Keywords GCC 4.7.0 ia64 Itanium bug impossible reload added
- Reviewers set to Jeroen Demeyer
- Status changed from needs_work to needs_review
- 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:15 Changed 10 years ago by
- Priority changed from major to blocker
comment:16 follow-up: ↓ 18 Changed 10 years ago by
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:17 Changed 10 years ago by
- Status changed from needs_review to needs_work
comment:18 in reply to: ↑ 16 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
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*
.)
comment:19 Changed 10 years ago by
- Description modified (diff)
comment:20 follow-up: ↓ 21 Changed 10 years ago by
Is egrep
available on all systems? Otherwise one can use grep -E
.
comment:21 in reply to: ↑ 20 ; follow-up: ↓ 22 Changed 10 years ago by
Replying to ppurka:
Is
egrep
available on all systems? Otherwise one can usegrep -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 10 years ago by
Replying to jdemeyer:
Replying to ppurka:
Is
egrep
available on all systems? Otherwise one can usegrep -E
.
egrep
is the more portable of these two: Solaris (at least some versions) does supportegrep
but notgrep -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 10 years ago by
- 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 10 years ago by
- 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 10 years ago by
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 10 years ago by
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 10 years ago by
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 10 years ago by
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 10 years ago by
- Merged in set to sage-5.0.rc0
- Resolution set to fixed
- Status changed from needs_work to closed
Diff between the previous spkg in Sage and my new p5 spkg. For reference / review only.