ecm does not compile on some 32-bit Linux systems

[jdemeyer]

On the Skynet machine cicero and on a 32-bit Gentoo Linux Pentium D machine of my own, the ecm package from #5847 does not install in Sage.

The problem is with Sage's spkg-install as the pure upstream sources in src/ work fine.

The following works:

$ ./configure
$ make

The following (emulating spkg-install) does NOT work:

$ export CFLAGS="-g -O3 -fPIC"
$ ./configure
$ make

It fails with the error

spv.c: In function 'spv_pwmul':
spv.c:157: error: unknown register name '%xmm7' in 'asm'
spv.c:157: error: unknown register name '%xmm6' in 'asm'
spv.c:157: error: unknown register name '%xmm5' in 'asm'
spv.c:157: error: unknown register name '%xmm3' in 'asm'
spv.c:157: error: unknown register name '%xmm2' in 'asm'
spv.c:157: error: unknown register name '%xmm1' in 'asm'
spv.c:157: error: unknown register name '%xmm0' in 'asm'

It doesn't actually matter what CFLAGS is set to, it fails even with

$ export CFLAGS=" "

This is probably because Sage's CFLAGS overrides ecm's CFLAGS. I think the easiest solution is simply not set any CFLAGS in ecm's spkg-install. Since ecm really wants to optimize for a particular machine, I think the fact that ecm's CFLAGS are not used has also bad consequences for speed.

---

Actually an upstream bug... (And not setting CFLAGS or CC is not an option. There are different ways to let the compiler produce processor-specific optimized code. Note that this anyway doesn't affect optimized assembly code, which ECM also comes with.)

New spkg (ecm-6.3.p2) fixing this (including the upstream patch) at #5847.

---

This ticket can be closed when #5847 got merged.

Depends on #5847

Upstream: Fixed upstream, but not in a stable release.

CC: @zimmermann6 @dimpase @nexttime @mwhansen

Component: packages: standard

Keywords: ecm spkg-install SSE2

Reviewer: Leif Leonhardy

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

[zimmermann6]

comment:2

note that GMP-ECM uses CC and CFLAGS from GMP, which ensures both portability and performance.
Thus overriding the choice of CFLAGS by GMP-ECM is a bad idea.

What does grep CFLAGS gmp.h give on that machine?

Paul

[jdemeyer]

comment:3

Replying to @zimmermann6:

What does grep CFLAGS gmp.h give on that machine?

On my machine (32-bit Gentoo Linux Pentium D), I get with the mpir header installed by Sage:

#define __GMP_CFLAGS "-g -O3 "
#define __MPIR_CFLAGS "-g -O3 "

With the system-wide /usr/include/gmp.h, I get

#define __GMP_CFLAGS "-pipe -march=nocona -O2 -mfpmath=sse"

[zimmermann6]

comment:4

Replying to @jdemeyer:

Replying to @zimmermann6:

What does grep CFLAGS gmp.h give on that machine?

On my machine (32-bit Gentoo Linux Pentium D), I get with the mpir header installed by Sage:

#define __GMP_CFLAGS "-g -O3 "
#define __MPIR_CFLAGS "-g -O3 "

With the system-wide /usr/include/gmp.h, I get

#define __GMP_CFLAGS "-pipe -march=nocona -O2 -mfpmath=sse"

the spkg should work with the system-wide CFLAGS value. With -g -O3 it seems HAVE_SSE2 is defined but SSE2 is not supported.

What is strange is that the GMP-ECM configure checks if SSE2 is really supported,
and if it is not the SSE2 code should not be compiled.

Do you confirm that the upstream package works with CFLAGS=-g -O3?

Paul

[jdemeyer]

comment:5

Replying to @zimmermann6:

Do you confirm that the upstream package works with CFLAGS=-g -O3?

No, the upstream package does not work with those CFLAGS. The reports in this bug report concerned the upstream sources (built independently of Sage).

[zimmermann6]

comment:6

I can reproduce the problem on a 32-bit Pentium 4, both with ecm-6.3 and the svn version. I will
try to find a fix.

Paul

[zimmermann6]

comment:7

I will try to find a fix.

with Pierrick Gaudry, we have analyzed and fixed the problem.

Long story: the asm volatile instructions without constraints in the configure script compile fine without -msse2, but those from the spv.c file, which have constraints, do not compile
without -msse2. The fix was to put instructions with constraints in configure.

Short story: apply the following patch

--- configure.in        (revision 1545)
+++ configure.in        (revision 1546)
@@ -296,9 +296,9 @@
   AC_MSG_CHECKING([for SSE2 support])
   m4_define([SSE2_TEST_PROG], [AC_LANG_PROGRAM([], dnl
 [#if (defined(__GNUC__) || defined(__ICL)) && defined(__i386__)
-/* When there are no constraints, registers are referred to by
-   single % sign, not double. Argh */
-asm volatile ("pmuludq %xmm2, %xmm0");
+/* On some machines, a program without constraints may pass without -msse2 but
+   those with constraints in spv.c fail, thus we test with constraints here. */
+asm volatile ("pmuludq %%xmm2, %%xmm0" : : :"%xmm0");
 #else
 #error
 #IRIXdoesnotexitaterrordirective

[jdemeyer]

comment:8

I can imagine that this bug-fix will make the ecm package work, but I believe that the deeper issue in Sage still needs to be addressed: namely that spkg-install (both for mpir and for ecm) is providing CFLAGS when it should leave that to the packages themselves.

For example, on sage.math.washington.edu, the upstream mpir package uses the flags -O2 -m64 -march=core2 -mtune=core2 (i.e.: optimized for the particular machine) while within Sage, the flags -g -O3 are used.

[dimpase]

comment:9

Replying to @jdemeyer:

I can imagine that this bug-fix will make the ecm package work, but I believe that the deeper issue in Sage still needs to be addressed: namely that spkg-install (both for mpir and for ecm) is providing CFLAGS when it should leave that to the packages themselves.

For example, on sage.math.washington.edu, the upstream mpir package uses the flags -O2 -m64 -march=core2 -mtune=core2 (i.e.: optimized for the particular machine) while within Sage, the flags -g -O3 are used.

How does the upstream mpir find these optimized flags?

[nexttime]

comment:11

Replying to @jdemeyer:

I can imagine that this bug-fix will make the ecm package work, but I believe that the deeper issue in Sage still needs to be addressed: namely that spkg-install (both for mpir and for ecm) is providing CFLAGS when it should leave that to the packages themselves.

I consider such upstream issues, i.e., upstream should IMHO "modify" CFLAGS et al. if appropriate or necessary, rather than either ignoring them or (solely) using user-specified settings "as is".

(Try e.g. env CFLAGS="" ./sage -ba-force && ./sage -c "quit"... which breaks the Cython-generated C code, or at least used to do so.)

For example, on sage.math.washington.edu, the upstream mpir package uses the flags -O2 -m64 -march=core2 -mtune=core2 (i.e.: optimized for the particular machine) while within Sage, the flags -g -O3 are used.

Just for the record: -march=hobel implies -mtune=hobel (and -mcpu=hobel).

[zimmermann6]

comment:12

I consider such upstream issues, i.e., upstream should IMHO "modify" CFLAGS et al. if appropriate or necessary

I disagree. If the user requests CFLAGS=..., upstream should use those flags.

Paul Zimmermann

[nexttime]

comment:13

Replying to @zimmermann6:

I consider such upstream issues, i.e., upstream should IMHO "modify" CFLAGS et al. if appropriate or necessary

I disagree. If the user requests CFLAGS=..., upstream should use those flags.

Yes, but not (necessarily) exclusively. ;-)

(Almost all build scripts append or prepend to CFLAGS etc., which is convenient if one only wants to add e.g. "-g", "-march=native" or "-mcpu=i486". Here we want the former...)

[nexttime]

comment:14

So we at least still have the problem of adding -m64 if SAGE64=yes (i.e., if we want a 64-bit build on systems that default to 32-bit builds):

...
# Note that GMP-ECM is written in C (and assembler) - no C++ sources.

if [ "$SAGE64" = yes ]; then
    echo "Building a 64-bit version of GMP-ECM"
    if [ -z "$CFLAG64" ]; then
        CFLAG64=-m64
    fi
    CFLAGS="$CFLAGS $CFLAG64 -fPIC"
    LDFLAGS="$CFLAG64"; export LDFLAGS
else
    CFLAGS="$CFLAGS -fPIC"
fi

# We add debug symbols by default;
if [ "$SAGE_DEBUG" = yes ]; then
    # Disable optimization:
    CFLAGS="$CFLAGS -g -O0"
else
    # Enable optimization, may be overridden by user settings:
    CFLAGS="-g -O3 $CFLAGS"
fi

export CFLAGS # usually redundant, but safe(r)
...

Of course I could add "our" (or worse, also any user-specified) CFLAGS to CC, i.e. the C compiler command, and unset CFLAGS to make ECM's effective, but I wouldn't like that. (And some settings then would get overridden by ECM's CFLAGS anyway.)

---

An IMHO better solution is to also add -march=native to CFLAGS, since this enables SSE (to be precise, allows SSE2 instructions) if supported by the processor (when configure defines HAVE_SSE2), ignoring SAGE_FAT_BINARY at least for the moment (unless ECM's configure has an option like --disable-sse or similar; I haven't really checked that yet, perhaps just --enable-sse2=no on 32-bit x86).

I'm not sure what happens if we configure with CFLAGS set and --enable-gmp-cflags=no (which seems to be more appropriate when using ECM with MPIR).

[nexttime]

comment:15

Just for the record: I'll upload a new spkg that also fixes this issue at #5847 (hopefully) soon...

[zimmermann6]

comment:16

Replying to @nexttime:

So we at least still have the problem of adding -m64 if SAGE64=yes [...]

not sure. GMP uses 64-bit words if available (even if the system is 32 bit), and GMP-ECM uses
GMP CFLAGS.

Paul

[jdemeyer]

comment:17

So the solution is easy: don't set any CFLAGS in the mpir and ecm packages. Right?

[zimmermann6]

comment:18

Replying to @jdemeyer:

So the solution is easy: don't set any CFLAGS in the mpir and ecm packages. Right?

that is my advice. Same for MPFR, which also copies CFLAGS from GMP.

Paul

[kiwifb]

comment:19

Replying to @nexttime:

An IMHO better solution is to also add -march=native to CFLAGS, since this enables SSE (to be precise, allows SSE2 instructions) if supported by the processor (when configure defines HAVE_SSE2), ignoring SAGE_FAT_BINARY at least for the moment (unless ECM's configure has an option like --disable-sse or similar; I haven't really checked that yet, perhaps just --enable-sse2=no on 32-bit x86).

You know that -march=native is only supported on x86 and amd64 cpus right?
I can tell you first hand that the compilation will stop straight away on ppc.
And then you will get Dave telling you it is not supported by other compilers.

[nexttime]

comment:20

Replying to @kiwifb:

You know that -march=native is only supported on x86 and amd64 cpus right?

Not only, but ...

I can tell you first hand that the compilation will stop straight away on ppc.

I was not going to try to enable SSE2 on PowerPCs (nor SPARC processors). ;-)

And then you will get Dave telling you it is not supported by other compilers.

There's very little support for other compilers in Sage, and it's easy to add a distinction when the day it gets necessary comes, though I could add it now.

[kiwifb]

comment:21

Replying to @nexttime:

Replying to @kiwifb:

You know that -march=native is only supported on x86 and amd64 cpus right?

Not only, but ...

I can tell you first hand that the compilation will stop straight away on ppc.

I was not going to try to enable SSE2 on PowerPCs (nor SPARC processors). ;-)

And then you will get Dave telling you it is not supported by other compilers.

There's very little support for other compilers in Sage, and it's easy to add a distinction when the day it gets necessary comes, though I could add it now.

Ok, I missed a little bit of context. I guess I didn't pay full attention to the whole thread. Sorry for the noise.

[nexttime]

comment:22

Replying to @nexttime:

Just for the record: I'll upload a new spkg that also fixes this issue at #5847 (hopefully) soon...

Did so...

Setting this ticket to "needs review" too, can be closed as duplicate when #5847 gets a positive review / merged.

[nexttime]

Upstream: Fixed upstream, but not in a stable release.

[nexttime]

comment:23

Replying to @nexttime:

Replying to @nexttime:

Just for the record: I'll upload a new spkg that also fixes this issue at #5847 (hopefully) soon...

Did so...

Setting this ticket to "needs review" too, can be closed as duplicate when #5847 gets a positive review / merged.

I've updated the p2 package at #5847 (some corrections, some improvements); same place, different md5sum, still or again needing testing (especially on PowerPCs) / review.

See #5847 comment:86 .

[dimpase]

comment:24

Replying to @nexttime:

Replying to @nexttime:

Replying to @nexttime:

Just for the record: I'll upload a new spkg that also fixes this issue at #5847 (hopefully) soon...

Did so...

Setting this ticket to "needs review" too, can be closed as duplicate when #5847 gets a positive review / merged.

I've updated the p2 package at #5847 (some corrections, some improvements); same place, different md5sum, still or again needing testing (especially on PowerPCs) / review.

See #5847 comment:86 .

duly tested on PPC, see the other ticket.

[robertwb]

comment:25

Is there any reason this shouldn't be closed as a dup now?

[nexttime]

comment:26

Replying to @robertwb:

Is there any reason this shouldn't be closed as a dup now?

Well, it's a symbolic link to #5847, which still needs review, delaying the MPIR upgrade (#8664) further.

We should close it when #5847 got merged.

[nexttime]

Changed keywords from ecm spkg-install to ecm spkg-install SSE2

[nexttime]

Dependencies: #5847

[nexttime]

Reviewer: Leif Leonhardy

[zimmermann6]

comment:30

for your information, this bug is fixed upstream, but we have no new release yet. The fix
consists in adding -msse2 when required to compile the corresponding assembly files.
See around lines 297-326 of the configure.in file in the svn version upstream.

Paul Zimmermann

[nexttime]

comment:31

Replying to @zimmermann6:

for your information, this bug is fixed upstream, but we have no new release yet.

Thanks. We already apply that patch (I think; from revision 1546) in the .p2 spkg at #5847.

[nexttime]

Merged: sage-4.7.2.alpha3

[jdemeyer]

Changed merged from sage-4.7.2.alpha3 to none

[zimmermann6]

comment:34

this problem is fixed in GMP-ECM 6.4, which has just been released.

Paul Zimmermann