update M4RI to newest upstream release

malb commented 13 years ago

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

Install http://sage.math.washington.edu/home/malb/spkgs/libm4ri-20110715.spkg
Apply attachment: m4ri_20110601.patch
Apply attachment: trac_11574_m4ri_sse2.patch

Depends on #11261 Depends on #11756

CC: @strogdon @nexttime @alexanderdreyer

Component: packages: standard

Keywords: sd32

Author: Martin Albrecht

Reviewer: Simon King, Alexander Dreyer

Merged: sage-4.7.2.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/11574

malb commented 13 years ago

comment:50

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.

malb commented 13 years ago

comment:51

Leif, note that M4RI checks for -msse2 like this

https://bitbucket.org/malb/m4ri/src/58007c8c739a/m4/ax_ext.m4

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:52

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.)

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:53

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.)

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:54

Replying to @nexttime:

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.

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.

d46d2cb1-860f-484d-8329-8ebfc9f5d004 commented 13 years ago

comment:55

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

kiwifb commented 13 years ago

comment:56

Replying to @nexttime:

Replying to @nexttime:

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.

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.

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.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:57

Replying to @kiwifb:

Replying to @nexttime:

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.

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...

kiwifb commented 13 years ago

comment:58

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.

d46d2cb1-860f-484d-8329-8ebfc9f5d004 commented 13 years ago

comment:59

The PolyBoRi spkg has now its own ticket: #11756 It it solved the problem (it does for me on a Gentoo VM), please review.

malb commented 13 years ago

comment:60

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?

strogdon commented 13 years ago

comment:61

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

malb commented 13 years ago

comment:62

Okay, I know what the issue is and will update the patch accordingly!

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:63

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)...

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:64

... though you could easily add the graceful dead ''here' as well.

malb commented 13 years ago

comment:65

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?

malb commented 13 years ago

comment:66

Attachment: trac_11574_m4ri_sse2.patch.gz

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.

malb commented 13 years ago

comment:67

The new M4RI SPKG is at #11757

simon-king-jena commented 13 years ago

comment:68

Replying to @malb:

The new M4RI SPKG is at #11757

Does that mean that #11757 makes this ticket a duplicate?

strogdon commented 13 years ago

comment:69

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.

malb commented 13 years ago

comment:70

Replying to @simon-king-jena:

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.

malb commented 13 years ago

comment:72

So, anybody up for reviewing this? Or are people waiting for attachment: trac_11574_m4ri_sse2.patch being ported to #11757?

strogdon commented 13 years ago

comment:73

Replying to @malb:

So, anybody up for reviewing this? Or are people waiting for attachment: 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?

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:74

Replying to @strogdon:

Shouldn't this ticket depend upon ticket #11756, even though that one has a positive review?

Yep, IMHO even if #11756 had already been merged into some devel release.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

Changed dependencies from #11261 to #11261 #11756

simon-king-jena commented 13 years ago

comment:75

Replying to @strogdon:

Replying to @malb:

So, anybody up for reviewing this? Or are people waiting for attachment: 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.

malb commented 13 years ago

comment:76

anyone?

malb commented 13 years ago

Description changed:

--- 
+++ 
@@ -5,3 +5,4 @@

 * **Install** http://sage.math.washington.edu/home/malb/spkgs/libm4ri-20110715.spkg
 * **Apply** [attachment: m4ri_20110601.patch](https://github.com/sagemath/sage-prod/files/10653267/m4ri_20110601.patch.gz)
+* **Apply** [attachment: trac_11574_m4ri_sse2.patch](https://github.com/sagemath/sage-prod/files/10653266/trac_11574_m4ri_sse2.patch.gz)

d46d2cb1-860f-484d-8329-8ebfc9f5d004 commented 13 years ago

comment:77

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!

d46d2cb1-860f-484d-8329-8ebfc9f5d004 commented 13 years ago

Changed reviewer from Simon King to Simon King, Alexander Dreyer

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

comment:78

attachment: m4ri_20110601.patch has no ticket number in its commit message.

malb commented 13 years ago

rebased to 4.7.1.alpha4

malb commented 13 years ago

comment:79

Attachment: m4ri_20110601.patch.gz

fixed.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

Merged: sage-4.7.2.alpha3

sagemath / sage

update M4RI to newest upstream release #11574