Opened 9 years ago
Closed 9 years ago
#11574 closed enhancement (fixed)
update M4RI to newest upstream release
Reported by: | malb | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-4.7.2 |
Component: | packages: standard | Keywords: | sd32 |
Cc: | strogdon, leif, AlexanderDreyer | Merged in: | sage-4.7.2.alpha3 |
Authors: | Martin Albrecht | Reviewers: | Simon King, Alexander Dreyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #11261 #11756 | Stopgaps: |
Description (last modified by )
M4RI 20110601 changed the API, so we need to apply a patch as well,
cf. https://bitbucket.org/malb/m4ri/wiki/M4RI-20110601 cf. https://bitbucket.org/malb/m4ri/wiki/M4RI-20110715
Attachments (2)
Change History (82)
comment:1 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
- Dependencies set to #11261
comment:3 Changed 9 years ago by
comment:4 Changed 9 years ago by
Okay, I think I figured it out. Sorry for the noise.
comment:5 Changed 9 years ago by
I don't know what I did wrong, but I still get the problem with the Symbol not found: _m4ri_swap_bits
while trying to start, even after this patch. I may have done something wrong, but at any rate I can't spend more time on this now, sorry.
comment:6 Changed 9 years ago by
- Description modified (diff)
comment:7 Changed 9 years ago by
For the record, I tried spkg + patch on t2 and doctests pass.
comment:8 Changed 9 years ago by
- Description modified (diff)
comment:9 Changed 9 years ago by
applies & passes doctests with 4.7.1.rc2.
comment:10 Changed 9 years ago by
- Reviewers set to Simon King
- Status changed from needs_review to needs_work
Why is the documentation for matrices over GF(2) not part of the reference manual? I think you should use the upgrade to expose the documentation to the audience.
comment:11 Changed 9 years ago by
I am not sure this documentation is much use in the reference manual since most of the interesting functions won't show up. Also, these matrices are expected to behave just as other matrices. On the other hand, there are a few - not many - special functions which are only available for GF(2). I'll take a look.
comment:12 Changed 9 years ago by
- Status changed from needs_work to needs_review
It seems that you agreed in a different ticket (#9562) that this class doesn't need to be in the reference manual? Hence, returning to needs review. Feel free to change it back, though.
comment:13 Changed 9 years ago by
I think I can live with not including it into the references. However, currently I am getting segment faults when doctesting on mark (one of the skynet machines). I try to make sure that I properly install everything from scratch again, before reporting details.
comment:14 Changed 9 years ago by
I did a forced new installation of the polybori spkg, then of the M4RI spkg, and I also installed the patches. I get on mark:
sage: P.<a,b,c,d,e> = PolynomialRing(GF(2), 5, order='lex') sage: I1 = ideal([a*b + c*d + 1, a*c*e + d*e, a*b*e + c*e, b*c + c*d*e + 1]) sage: B.<a,b,c,d,e> = BooleanPolynomialRing(5, order='lex') sage: I2 = ideal([B(f) for f in I1.gens()]) sage: I2.groebner_basis() --------------------------------------------------------------------------- RuntimeError Traceback (most recent call last) /home/simonking/SAGE/sage-4.7.1.rc1mark/devel/sage-main/<ipython console> in <module>() /home/simonking/SAGE/sage-4.7.1.rc1mark/local/lib/python2.6/site-packages/sage/rings/polynomial/pbori.so in sage.rings.polynomial.pbori.BooleanPolynomialIdeal.groebner_basis (sage/rings/polynomial/pbori.cpp:26221)() RuntimeError: Segmentation fault
Of course, it could be that the problem comes from the polybori spkg. That would be unfortunate, since it is already merged.
comment:15 Changed 9 years ago by
Now, I tried to force re-installation of the original polybori spkg. However, that failed as well. I hope that it will not be needed to rebuild all of sage, which takes ages on mark.
comment:16 Changed 9 years ago by
I think the issue is that one needs to install stuff in the right order:
- install new M4RI
- install new PolyBoRi?
- apply the M4RI patch
- touch
sage/libs/polybori/decl.pxd
sage -b
comment:17 follow-up: ↓ 20 Changed 9 years ago by
Meanwhile I managed to revert to the original state, and can confirm that, with vanilla sage-4.7.1.rc2 on 32-bit solaris, the segfault above does not occur.
I will now try to do things in the order you suggested: In fact, originally I had installed polybori, then m4ri, then the m4ri patch, and then sage -b (without touching decl.pxd).
But how can the new polybori spkg be merged if apparently it depends on the new m4ri?
comment:18 Changed 9 years ago by
The new installation order did the trick, the segfault has gone. I am now repeating the failing doctests.
comment:19 Changed 9 years ago by
Or actually I am repeating sage -testall.
comment:20 in reply to: ↑ 17 Changed 9 years ago by
Replying to SimonKing:
But how can the new polybori spkg be merged if apparently it depends on the new m4ri?
Hi, the PolyBoRi? does not depend on the new M4RI. This version of M4RI breaks binary compatibility with previous M4RI, i.e. we changed how matrices look internally. Hence, for many pieces of software linking against M4RI one needs to rebuild them to make sure they are talking to the right M4RI library in the right way. That's why M4RI and the PolyBoRi? is necessary: to tell PolyBoRi? to use this new version. I think we need to updated PolyBoRi? as well because some macro names changed and PolyBoRi? 0.7.1.p4 contains the necessary code to deal with it.
comment:21 Changed 9 years ago by
- Status changed from needs_review to positive_review
The tests took 84373.7 seconds on mark, and there were a couple of timeouts. However, there was no failure. The segfault that I reported above vanished by installing everything in the right order. On my computer, all tests pass.
Moreover, it is a prerequisite for working with M4RIE (#9562), which I think is a very important contribution - currently, matrices over GF(2^{e}) are painfully slow in Sage.
I think it is ok that the matrices over GF(2) do not have their own section in the reference manual, since there seems that all non-underscore methods are already present for general matrices.
Hence, it is a positive review.
comment:22 Changed 9 years ago by
Woot!
comment:23 Changed 9 years ago by
- Priority changed from major to critical
I'm upping the priority, since this fixes a serious mathematical bug reported on sage-support.
comment:24 Changed 9 years ago by
- Status changed from positive_review to needs_work
The directory m4ri
should be added to .hgignore
comment:25 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to positive_review
the updated SPKG fixes this issue.
comment:26 Changed 9 years ago by
- Merged in set to sage-4.7.2.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
comment:27 follow-up: ↓ 33 Changed 9 years ago by
- Merged in sage-4.7.2.alpha2 deleted
- Resolution fixed deleted
- Status changed from closed to new
On some 32-bit Linux systems (for example, the buildbot machine cicero
), this causes polybori to fail to install:
g++ -o groebner/src/randomset.o -c -O3 -Wno-long-long -Wreturn-type -g -fPIC -ftemplate-depth-100 -O3 -Wno-long-long -Wreturn-type -g -fPIC -DNDEBUG -DHAVE_GD -DHAVE_TR1_UNORDERED_MAP -DPACKED -DHAVE_M4RI -DHAVE_GD -DHAVE_IEEE_754 -DBSD -I/home/jdemeyer/sage-4.7.2.alpha1/local/include -I/home/jdemeyer/sage-4.7.2.alpha1/local/include/python2.6 -Ipolybori/include -ICudd/obj -ICudd/util -ICudd/cudd -ICudd/mtr -ICudd/st -ICudd/epd groebner/src/randomset.cc In file included from /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:42, from /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/permutation.h:31, from /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/m4ri.h:50, from groebner/src/nf.h:15, from groebner/src/randomset.cc:11: /usr/lib/gcc/i686-pc-linux-gnu/4.4.3/include/emmintrin.h:32:3: error: #error "SSE2 instruction set not enabled" In file included from /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/permutation.h:31, from /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/m4ri.h:50, from groebner/src/nf.h:15, from groebner/src/randomset.cc:11: /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h: In function 'void mzd_row_add_offset(mzd_t*, rci_t, rci_t, rci_t)': /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741: error: '__m128i' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741: error: '__src' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741: error: expected primary-expression before ')' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741: error: expected ';' before 'src' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:742: error: '__dst' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:742: error: expected primary-expression before ')' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:742: error: expected ';' before 'dst' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:743: error: expected primary-expression before 'const' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:743: error: expected ';' before 'const' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:746: error: expected ';' before 'xmm1' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:747: error: 'xmm1' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:749: error: 'eof' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h: In function 'void mzd_combine_even_in_place(mzd_t*, rci_t, wi_t, const mzd_t*, rci_t, wi_t)': /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1190: error: '__m128i' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1190: error: 'a128' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1190: error: expected primary-expression before ')' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1190: error: expected ';' before 'a' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1191: error: 'b128' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1191: error: expected primary-expression before ')' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1191: error: expected ';' before 'b' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1192: error: expected initializer before '*' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1195: error: '_mm_xor_si128' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1198: error: 'eof' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h: In function 'void mzd_combine_even(mzd_t*, rci_t, wi_t, const mzd_t*, rci_t, wi_t, const mzd_t*, rci_t, wi_t)': /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1274: error: '__m128i' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1274: error: 'a128' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1274: error: expected primary-expression before ')' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1274: error: expected ';' before 'a' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1275: error: 'b128' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1275: error: expected primary-expression before ')' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1275: error: expected ';' before 'b' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1276: error: 'c128' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1276: error: expected primary-expression before ')' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1276: error: expected ';' before 'c' /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1277: error: expected initializer before '*' token /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1280: error: '_mm_xor_si128' was not declared in this scope /home/jdemeyer/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1284: error: 'eof' was not declared in this scope scons: *** [groebner/src/randomset.o] Error 1 scons: building terminated because of errors.
comment:28 Changed 9 years ago by
- Status changed from new to needs_review
comment:29 Changed 9 years ago by
- Status changed from needs_review to needs_work
comment:30 follow-up: ↓ 32 Changed 9 years ago by
Can you allow me to read your build on cicero (my username is malb). I don't have a copy of Sage on cicero and it would save me some time.
comment:31 Changed 9 years ago by
- Keywords sd32 added
comment:32 in reply to: ↑ 30 Changed 9 years ago by
Replying to malb:
Can you allow me to read your build on cicero (my username is malb). I don't have a copy of Sage on cicero and it would save me some time.
It's in /home/buildbot/build/sage/cicero-1/cicero_full/build/sage-4.7.2.alpha2
(should be accessible through NFS on any Skynet machine).
comment:33 in reply to: ↑ 27 ; follow-up: ↓ 34 Changed 9 years ago by
Replying to jdemeyer:
On some 32-bit Linux systems (for example, the buildbot machine
cicero
), this causes polybori to fail to install:
[...]
Here on my system the old m4ri/config.h
stays when the new libm4ri spkg is installed. Maybe something around polybori picks the outdated file?
Regards,
Alexander
comment:34 in reply to: ↑ 33 ; follow-up: ↓ 35 Changed 9 years ago by
Replying to AlexanderDreyer:
Here on my system the old
m4ri/config.h
stays when the new libm4ri spkg is installed. Maybe something around polybori picks the outdated file?
As Jeroen pointed out this is not related. Unfortunately I cannot reproduce this behaviour. Is there some machine on which this occurs, which I may be granted access to?
comment:35 in reply to: ↑ 34 ; follow-up: ↓ 36 Changed 9 years ago by
Replying to AlexanderDreyer:
As Jeroen pointed out this is not related. Unfortunately I cannot reproduce this behaviour. Is there some machine on which this occurs, which I may be granted access to?
The only machine which I know of is 'cicero' on skynet. We could try to get you an account there but it would take a while I guess. Right now, I cannot even log into the machine.
comment:36 in reply to: ↑ 35 Changed 9 years ago by
Replying to malb:
The only machine which I know of is 'cicero' on skynet. We could try to get you an account there but it would take a while I guess. Right now, I cannot even log into the machine.
This would be nice. I'm about to prepare a qemu image for Gentoo x86. But that will last for some time since I have to compile everything from scratch (It's Gentoo.)
comment:37 follow-up: ↓ 38 Changed 9 years ago by
- Cc strogdon added
I think thing go south from there
/usr/lib/gcc/i686-pc-linux-gnu/4.4.3/include/emmintrin.h:32:3: error: #error "SSE2 instruction set not enabled"
I had a x86 box without sse2 but it is now collecting dust without an OS installed. I will take a look on the closest thing I can get. Actually Steve Trogdon could have a box where this could be tested, I am copying him.
comment:38 in reply to: ↑ 37 Changed 9 years ago by
Replying to fbissey:
I think thing go south from there
/usr/lib/gcc/i686-pc-linux-gnu/4.4.3/include/emmintrin.h:32:3: error: #error "SSE2 instruction set not enabled"I had a x86 box without sse2 but it is now collecting dust without an OS installed. I will take a look on the closest thing I can get. Actually Steve Trogdon could have a box where this could be tested, I am copying him.
I get the same failure as comment:33
/storage/strogdon/gentoo/usr/lib/gcc/i686-pc-linux-gnu/4.5.2/include/emmintrin.h:32:3: error: #error "SSE2 instruction set not enabled" In file included from /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/permutation.h:31:0, from /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/m4ri.h:50, from groebner/src/nf.h:15, from groebner/src/randomset.cc:11: ... scons: *** [groebner/src/randomset.o] Error 1 scons: building terminated because of errors. Error building PolyBoRi.
comment:39 Changed 9 years ago by
M4RI *should* test for SSE2 and only include that header if it is present. Since it worked so far, I assume it still does that. However, we recently switched from HAVE_SSE2
to __M4RI_HAVE_SSE2
and this might be a change that PolyBoRi? didn't pick up? However, it shouldn't be PolyBoRi?'s responsibility to check for SSE2 and hence something goes wrong when including m4ri.h
.
comment:40 Changed 9 years ago by
For some reason m4ri_config.h
on cicero has
#define __M4RI_HAVE_SSE2 1
which is totally wrong! I'll investigate.
comment:41 Changed 9 years ago by
More data on cicero:
cat /proc/cpuinfo
reports SSE2
flags : fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe up pebs bts
but /usr/lib/gcc/i686-pc-linux-gnu/4.4.3/include/emmintrin.h:32:3
complains. I assume this is because neither PolyBoRi nor Sage pass -msse2
when linking against M4RI but -msse2
defines __SSE2__
which is required by emmintrin.h
. But I don't understand yet why the previous version works.
comment:42 follow-up: ↓ 43 Changed 9 years ago by
I can confirm that passing -msse2
fixes the issue. I guess I'll have to find a way to save the flags (to disc?) such that third parties (PolyBoRi, Sage) can re-use them? Any suggestions on how to do that?
comment:43 in reply to: ↑ 42 Changed 9 years ago by
Replying to malb:
I can confirm that passing
-msse2
fixes the issue. I guess I'll have to find a way to save the flags (to disc?) such that third parties (PolyBoRi, Sage) can re-use them? Any suggestions on how to do that?
For scons-bases builds (PolyBoRi as well as Sage) I should be able to provide a suitable code snipplet which checks whether M4RI_HAVE_SSE2 is defined in m4ri/m4ri_config.h.
comment:44 Changed 9 years ago by
- Owner changed from tbd to (none)
We have added some configuration for this here: https://bitbucket.org/brickenstein/polybori/changeset/e5ecbbe5a904 I'll backport this to 0.7.1 and bundle a new spkg asap.
comment:45 Changed 9 years ago by
PS: I'm also checking things like sse3 which m4ri might or might not use in the future.
comment:46 follow-up: ↓ 48 Changed 9 years ago by
Hi Alexander,
so your plan is to parse for __M4RI_HAVE_SSE2=1
and then pass -msse2
manually? The only caveat I could see with that approach is that it limits us to use !GCC. For Sage this isn't really a problem. M4RI supports more compilers. How's that for PolyBoRi?
If we want to support more compilers I was thinking about dumping the CFLAGS to disk in M4RI which third party tools then can re-use. I was thinking of using the pkg-config.rc file which one can then either use via pkg-config or parse oneself.
PS: Sage's main build scripts are not !Scons based, but I can certainly take care of those.
comment:47 Changed 9 years ago by
- Cc leif added
comment:48 in reply to: ↑ 46 Changed 9 years ago by
Replying to malb:
The only caveat I could see with that approach is that it limits us to use !GCC. For Sage this isn't really a problem. M4RI supports more compilers. How's that for PolyBoRi?
We support only gcc yet. In the case that we would add additional ones we'll add a mapping of non-default compiler options anyway. (e.g. -msse2 -> -xarch-sse2 for sun studio). But if you could export the original parameters, this would of course be better.
If we want to support more compilers I was thinking about dumping the CFLAGS to disk in M4RI which third party tools then can re-use. I was thinking of using the pkg-config.rc file which one can then either use via pkg-config or parse oneself.
I would prefer if you could add a macro defining the options as a string literal to m4ri_config.h. But if you need to setup the rc file anyway, you do not have to duplicate work.
comment:49 Changed 9 years ago by
Replying to malb:
I can confirm that passing
-msse2
fixes the issue. I guess I'll have to find a way to save the flags (to disc?) such that third parties (PolyBoRi, Sage) can re-use them? Any suggestions on how to do that?
Take a look at GMP's / MPIR's gmp.h
and the various libs (like e.g. MPFR) that use it:
$ egrep -B1 "__GMP_(CC|CFLAGS)" /usr/include/gmp.h /* Define CC and CFLAGS which were used to build this version of GMP */ #define __GMP_CC "gcc -std=gnu99" #define __GMP_CFLAGS "-march=native -g -O3"
I for example use (slightly OT):
# We eventually use / add these settings (or parts of them) to "our" CFLAGS, # e.g. "-march=..." or "-mcpu=..." and "-mtune=..." to let gcc generate better, # processor-specific code, since ECM doesn't use them if we set our own: gmp_cc_pat='/^[ ]*#[ ]*define[ ]\+__GMP_CC[ ]\+/s/.*"\([^"]*\)"/\1/p' gmp_cflags_pat='/^[ ]*#[ ]*define[ ]\+__GMP_CFLAGS[ ]\+/s/.*"\([^"]*\)"/\1/p' gmp_cc=`sed -n -e "$gmp_cc_pat" "$SAGE_LOCAL"/include/gmp.h` gmp_cflags=`sed -n -e "$gmp_cflags_pat" "$SAGE_LOCAL"/include/gmp.h` system_gmp_h="" for incdir in /usr/include /usr/local/include; do if [ -f $incdir/gmp.h ]; then system_gmp_h=$incdir/gmp.h fi done if [ -n "$system_gmp_h" ]; then system_gmp_cc=`sed -n -e "$gmp_cc_pat" $system_gmp_h` system_gmp_cflags=`sed -n -e "$gmp_cflags_pat" $system_gmp_h` fi ... else # 'native' not supported, see if GMP / MPIR provides us some CPU type: for opt in $gmp_cflags; do case $opt in -march=*|-mcpu=*|-mtune*) echo "Found CPU parameter in gmp.h: $opt" cpu_params="$cpu_params $opt" ;; # perhaps add other options, too (e.g. for different compilers) *) other_gmp_cflags="$other_gmp_cflags $opt" esac done fi # Only add them if CFLAGS do not already contain similar: if [ -n "$cpu_params" ] && ! (echo "$CFLAGS" | egrep -- '-march=|-mcpu=|-mtune=' >/dev/null); then echo "Using additional host-specific CFLAGS: $cpu_params" CFLAGS="$cpu_params $CFLAGS" fi if [ -n "$other_gmp_cflags" ]; then echo "Not using other CFLAGS provided by gmp.h: $other_gmp_cflags" fi ...
Putting at least the necessary flags into libm4ri.pc
is of course another, non-exclusive option, and even on systems that lack pkg-config
, you could use the .pc
file by adding a few lines of shell, Python or whatever code.
You can also test whether adding -msse2
(if using SSE2 is desired) is necessary at all, or supported (for simplicity, here gcc
-specific):
# Test whether '-msse2' is supported at all: if $CC -msse2 -c -x c /dev/null -o /dev/null &>/dev/null; then # Ok, supported. Do we need it? # [Should perhaps check whether __SSE2__ is really non-zero.] if ($CC -E -dM -x c /dev/null | grep -w __SSE2__) &>/dev/null; then # Fine, we don't need to pass any options to enable SSE2. required_cflag="" else required_cflag="-msse2" fi # Check whether the flag conflicts with others, perhaps specified by the user: if $CC $CFLAGS $required_cflag -c -x c /dev/null -o /dev/null &>/dev/null; then # Ok, compatible and sufficient. else # Conflicting flags, issue error message or try something else... fi else # Not supported, perhaps disable use of SSE2. fi
(The same tests can easily be performed from Python as well, e.g. in [Sage's] setup.py
or module_list.py
. You could also steal macros from autotools.)
comment:50 follow-up: ↓ 53 Changed 9 years ago by
The attached patch manually adds -msse -msse2
in module_list.py if __M4RI_HAVE_SSE2
. I fully agree that M4RI should export its flags both as an .rc file and as a define like GMP and I will implement this as well.
comment:51 Changed 9 years ago by
Leif, note that M4RI checks for -msse2
like this
https://bitbucket.org/malb/m4ri/src/58007c8c739a/m4/ax_ext.m4
comment:52 Changed 9 years ago by
Replying to AlexanderDreyer:
Replying to malb:
The only caveat I could see with that approach is that it limits us to use !GCC. For Sage this isn't really a problem. M4RI supports more compilers. How's that for PolyBoRi?
We support only gcc yet. In the case that we would add additional ones we'll add a mapping of non-default compiler options anyway.
Yep. Adding such is almost trivial, at least for a fixed set of compilers (and platforms).
(e.g. -msse2 -> -xarch-sse2 for sun studio). But if you could export the original parameters, this would of course be better.
I'd do both, and resort to the SCons method in case no (suitable) parameters are available.
If we want to support more compilers I was thinking about dumping the CFLAGS to disk in M4RI which third party tools then can re-use. I was thinking of using the pkg-config.rc file which one can then either use via pkg-config or parse oneself.
I would prefer if you could add a macro defining the options as a string literal to m4ri_config.h. But if you need to setup the rc file anyway, you do not have to duplicate work.
I'd do both, but preferably use the header method. Using the .pc
file is limited and to some extent dangerous, since pkg-config
files lack (by default) a compiler command entry such that the flags may be invalid or insufficient for the actual compiler used.
(Note that some people even put flags into CC
rather than CFLAGS
, and different installed GCC versions might default to different architectures as well.)
You could either define other variables there, to get parsed / interpreted "manually", or simply put exactly the same lines you put into your header file into the .pc
file, since incidentally any #define
in a .pc
file happens to be just a comment. :-)
(You'll have to use e.g. grep
to extract them though, rather than pkg-config
.)
comment:53 in reply to: ↑ 50 ; follow-up: ↓ 54 Changed 9 years ago by
Replying to malb:
The attached patch manually adds
-msse -msse2
in module_list.py if__M4RI_HAVE_SSE2
. I fully agree that M4RI should export its flags both as an .rc file and as a define like GMP and I will implement this as well.
Shouldn't the modules also depend on SAGE_LOCAL + "/include/m4ri/m4ri_config.h"
? (You should use SAGE_INC
there, too, btw. ;-) )
This should be done in a more general or automatic way, i.e., m4ri_extra_compile_args
(and probably -std=c99
) should be added in setup.py
for every module that links against libm4ri
.
(I'm adding similar for dependencies, e.g. at #8664, and other stuff, in order to make module_list.py
more modular and less error-prone.)
P.S.: SSE2 is a superset of SSE, just as -msse2
implies -msse
.
The patch doesn't yet check whether adding these flags is sufficient. Haven't looked at your M4RI code, but if you add something like
#if defined(__M4RI_HAVE_SSE2) && __M4RI_HAVE_SSE2
# if !defined(__SSE2__) || !__SSE2__
# error Your current compiler and / or CFLAGS setting doesn't allow SSE2 code. Please change that or these to the setting(s) you used when compiling M4RI.
# endif
#endif
(I wanted to post that to sage-devel a few hours ago, in response to:
> > If 'configure' determined the machine supports SSE2, __M4RI_HAVE_SSE2 > > is set to 1, but even if that's the case, you should still also test > > __SSE2__ when compiling code later (including code *using* the > > library). > Why? I want that whoever links against M4RI lives in the same world as M4RI.
for the reasons given above.)
comment:54 in reply to: ↑ 53 ; follow-up: ↓ 56 Changed 9 years ago by
Replying to leif:
This should be done in a more general or automatic way, i.e.,
m4ri_extra_compile_args
(and probably-std=c99
) should be added insetup.py
for every module that links againstlibm4ri
.
P.S.: I don't mean you have to do / change that here, as e.g. Francois is also messing around in that area of setup.py
;-) , such that patches were likely to conflict, so I could also change that later on another ticket, perhaps unrelated to M4RI, but dealing with setup.py
.
comment:55 Changed 9 years ago by
A new PolyBoRi? 0.7.1 spkg (with activated -msse2 option, when necessary) can be found here: http://sage.math.washington.edu/home/dreyer/spkg/polybori-0.7.1.p5.spkg
comment:56 in reply to: ↑ 54 ; follow-up: ↓ 57 Changed 9 years ago by
Replying to leif:
Replying to leif:
This should be done in a more general or automatic way, i.e.,
m4ri_extra_compile_args
(and probably-std=c99
) should be added insetup.py
for every module that links againstlibm4ri
.P.S.: I don't mean you have to do / change that here, as e.g. Francois is also messing around in that area of
setup.py
;-) , such that patches were likely to conflict, so I could also change that later on another ticket, perhaps unrelated to M4RI, but dealing withsetup.py
.
All my changes have been merged in 4.7.2.alpha1 I don't see them getting out again. So I would say it is safe to base your patch on that.
comment:57 in reply to: ↑ 56 Changed 9 years ago by
Replying to fbissey:
Replying to leif:
P.S.: I don't mean you have to do / change that here, as e.g. Francois is also messing around in that area of
setup.py
;-) , such that patches were likely to conflict, so I could also change that later on another ticket, perhaps unrelated to M4RI, but dealing withsetup.py
.All my changes have been merged in 4.7.2.alpha1 I don't see them getting out again. So I would say it is safe to base your patch on that.
But I've to again rebase my patch from #8664 on yours... :D
P.S.: Some good and bad news for #9958 coming soon...
comment:58 Changed 9 years ago by
Indeed, I mothballed SAGE_DEBIAN ! I didn't notice you had a patch touching that area in #8664, it took too long to merge that one really. It has to go in.
I am bracing myself for #9958, you know we already ship sage-on-gentoo with python-2.7 and it works well (#11339 excluded) on x86/amd64 linux and OS X in a prefix. But we are getting off-track.
comment:59 Changed 9 years ago by
The PolyBoRi spkg has now its own ticket: #11756 It it solved the problem (it does for me on a Gentoo VM), please review.
comment:60 follow-up: ↓ 63 Changed 9 years ago by
Hi,
so how's this for a strategy:
- we do the fixes (or something similar) that Alexander and I proposed for PolyBoRi and !Sage respectively now
- I provide (in a new ticket) an updated M4RI which exports its !CLFAGS both in the headers and in the .rc file and also checks for SSE2 to fail gracefully.
- We then update !Sage & PolyBoRi again to use these !CFLAGS?
comment:61 Changed 9 years ago by
polybori-0.7.1.p5.spkg does build. I applied the two indicated patches but building of sage itself fails:
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC -I/storage/sage-python2.6/sage-4.7.2.alpha1/local/include -I/storage/sage-python2.6/sage-4.7.2.alpha1/local/include/csage -I/storage/sage-python2.6/sage-4.7 .2.alpha1/devel/sage/sage/ext -I/storage/sage-python2.6/sage-4.7.2.alpha1/local/include/python2.6 -c sage/matrix/matrix_integer_dense.c -o build/temp.linux-i686-2.6/sage/matrix/matrix_integer_dense.o -std=c99 -w In file included from /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:42:0, from /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/permutation.h:31, from /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/m4ri.h:50, from sage/matrix/matrix_integer_dense.c:240: /storage/strogdon/gentoo/usr/lib/gcc/i686-pc-linux-gnu/4.5.2/include/emmintrin.h:32:3: error: #error "SSE2 instruction set not enabled" In file included from /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/permutation.h:31:0, from /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/m4ri.h:50, from sage/matrix/matrix_integer_dense.c:240: /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h: In function mzd_row_add_offset: /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741:5: error: __m128i undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741:5: note: each undeclared identifier is reported only once for each function it appears in /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741:14: error: __src undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:741:31: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:742:14: error: __dst undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:742:31: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:743:14: error: expected expression before const /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:746:15: error: expected ; before xmm1 /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:747:18: error: xmm1 undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:749:21: error: eof undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h: In function mzd_combine_even_in_place: /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1190:7: error: __m128i undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1190:16: error: a128 undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1190:32: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1191:16: error: b128 undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1191:32: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1192:21: error: expected =, ,, ;, asm or __attribute__ before * token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1192:22: error: eof undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1192:37: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h: In function mzd_combine_even: /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1274:7: error: __m128i undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1274:16: error: a128 undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1274:32: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1275:16: error: b128 undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1275:32: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1276:16: error: c128 undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1276:32: error: expected expression before ) token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1277:21: error: expected =, ,, ;, asm or __attribute__ before * token /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1277:22: error: eof undeclared (first use in this function) /storage/sage-python2.6/sage-4.7.2.alpha1/local/include/m4ri/packedmatrix.h:1277:37: error: expected expression before ) token
comment:62 Changed 9 years ago by
Okay, I know what the issue is and will update the patch accordingly!
comment:63 in reply to: ↑ 60 Changed 9 years ago by
Replying to malb:
so how's this for a strategy:
- we do the fixes (or something similar) that Alexander and I proposed for PolyBoRi and !Sage respectively now
- I provide (in a new ticket) an updated M4RI which exports its !CLFAGS both in the headers and in the .rc file and also checks for SSE2 to fail gracefully.
- We then update !Sage & PolyBoRi again to use these !CFLAGS?
!That ?sounds !reasonable!!
And also avoids circular ticket dependencies for the follow-up(s)...
comment:64 Changed 9 years ago by
... though you could easily add the graceful dead here' as well.
comment:65 Changed 9 years ago by
I wanted to add it to M4RI and so far the update to this ticket does not require a new M4RI SPKG. I'm in the process of patching M4RI and will open a new ticket for it shortly. Once that's done we can easily make that a dependency for this ticket?
Changed 9 years ago by
comment:66 follow-up: ↓ 69 Changed 9 years ago by
- Status changed from needs_work to needs_review
The updated patch should allow the Sage build to go through. Testing on cicero now. I also added some depends
fields as suggested by Leif.
comment:67 follow-up: ↓ 68 Changed 9 years ago by
The new M4RI SPKG is at #11757
comment:68 in reply to: ↑ 67 ; follow-up: ↓ 70 Changed 9 years ago by
- Status changed from needs_review to needs_info
comment:69 in reply to: ↑ 66 Changed 9 years ago by
Replying to malb:
The updated patch should allow the Sage build to go through. Testing on cicero now. I also added some
depends
fields as suggested by Leif.
Yes, the updated patch does allow Sage to build and there are no failing tests.
comment:70 in reply to: ↑ 68 Changed 9 years ago by
Replying to SimonKing:
Replying to malb:
The new M4RI SPKG is at #11757
Does that mean that #11757 makes this ticket a duplicate?
I guess one could look at it this way. I'd suggest: We deal with this ticket here first and if people then still have energy and are not sick of M4RI yet we can also include #11757.
comment:71 Changed 9 years ago by
- Status changed from needs_info to needs_review
comment:72 follow-up: ↓ 73 Changed 9 years ago by
So, anybody up for reviewing this? Or are people waiting for trac_11574_m4ri_sse2.patch being ported to #11757?
comment:73 in reply to: ↑ 72 ; follow-ups: ↓ 74 ↓ 75 Changed 9 years ago by
Replying to malb:
So, anybody up for reviewing this? Or are people waiting for trac_11574_m4ri_sse2.patch being ported to #11757?
I should probably defer review to Simon since I got in through the backdoor. However, since Sage builds here and there are no doctest failures I would give it a positive review. I do have one question. Shouldn't this ticket depend upon ticket #11756, even though that one has a positive review?
comment:74 in reply to: ↑ 73 Changed 9 years ago by
- Dependencies changed from #11261 to #11261 #11756
comment:75 in reply to: ↑ 73 Changed 9 years ago by
Replying to strogdon:
Replying to malb:
So, anybody up for reviewing this? Or are people waiting for trac_11574_m4ri_sse2.patch being ported to #11757?
I should probably defer review to Simon since I got in through the backdoor.
Sorry, I will probably not be able to review it in the next couple of days. So, iI'd appreciate if someone was quicker than I.
comment:77 Changed 9 years ago by
- Cc AlexanderDreyer added
- Reviewers changed from Simon King to Simon King, Alexander Dreyer
- Status changed from needs_review to positive_review
The spkg builds and and installs fine. Together with the patches sage -testall
was run successfully on a SuSE Linux Enterprise 11 x86_64, Gentoo i486, and Mac OS X 10.5 ppc.
So, I can cordially give a positive review!
comment:78 Changed 9 years ago by
m4ri_20110601.patch has no ticket number in its commit message.
comment:79 Changed 9 years ago by
fixed.
comment:80 Changed 9 years ago by
- Merged in set to sage-4.7.2.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
I'm a little confused. The spkgs here and at #9562 are both libm4ri, and seem to be numbered backwards (the "earlier" one in the dependency is from 2011).