sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.39k stars 473 forks source link

Update GMP-ECM to 6.4.4 #14151

Closed zimmermann6 closed 11 years ago

zimmermann6 commented 11 years ago

GMP-ECM 6.4.4 is available at https://gforge.inria.fr/frs/download.php/32159/ecm-6.4.4.tar.gz.

With respect to 6.3 currently used in Sage:

Changes between GMP-ECM 6.4.3 and GMP-ECM 6.4.4:
* Fixed PowerPC64 assembly code with --enable-shared (thanks Leif Leonhardy)
* Fix to deal with change of semantics of internal GMP functions in GMP 5.1
* Fixed small memory leak in non-NTT P-1 stage 2
* Fixed segfaults with large non-NTT P+-1 stage 2
* Removed defunct -t command line option

Changes between GMP-ECM 6.4.2 and GMP-ECM 6.4.3:
* Fixed bug reported by user "lorgix" on mersenneforum
  (http://www.mersenneforum.org/showpost.php?p=286385&postcount=280)
* Use 64-bit value for random seed under Windows

Changes between GMP-ECM 6.4.1 and GMP-ECM 6.4.2:
* Corrected the copyright headers
* Reduced memory usage in stage 1 with -batch={1,2} mode.
* Fixed bug in modular reduction (could occur only for numbers larger than
  386 digits on 64-bit computers and 193 digits on 32-bit computers).
* Speedup in stage 2 with the NTT default mode

Changes between GMP-ECM 6.4 and GMP-ECM 6.4.1:
* GMP-ECM is now distributed under the GPL version 3 or later for the binary,
  and under the LGPL version 3 or later for the library
* Fixed a speed regression with respect to GMP-ECM 6.3
  http://lists.gforge.inria.fr/pipermail/ecm-discuss/2012-February/004103.html
* Fixed a bug with the -treefile option which had been present for a long time
* Several fixes for the Visual Studio 2010 build
* New experimental option -batch=2, and speedup for -batch (i.e., -batch=1)
* New tuning mechanism, now --enable-asm-redc is always recommended
* New configure option --enable-mulredc-svoboda, for input numbers whose low
  limbs is congruent to -1
* New tuning parameters for Intel Core i5
* New ecmbench utility

Changes between GMP-ECM 6.3 and GMP-ECM 6.4:
* Fixed configure problem with SSE2
  (https://github.com/sagemath/sage-prod/issues/10252)
* Fixed configure bug on 32-bit PowerPC (tried to use 64-bit assembly)
  https://gforge.inria.fr/tracker/index.php?func=detail&aid=10646
* Fixed dependencies from build directory
  https://gforge.inria.fr/tracker/index.php?func=detail&aid=10648
* Patch from David Cleaver to allow B1 >= 2^32 on machines where
  "unsigned long" has 32 bits only
* Patch from David Cleaver to use GWNUM 26.6 on Windows x64 with MingW64/Msys
* Improved conversion from mpz_t to residue number system in NTT code
* Better asm code for AMD cpus
* Use of GMP's mpn_mullo_n and mpn_redc_2 when available
* New option -batch with faster Stage 1 (but smaller success probability)
* Added Visual Studio 2010 build

spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/ecm-6.4.4.spkg (diff)

CC: @nexttime

Component: packages: standard

Keywords: spkg

Author: Jeroen Demeyer

Reviewer: François Bissey, Paul Zimmermann

Merged: sage-5.10.beta0

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

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

Changed keywords from none to spkg

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:2

Hi Paul,

I noticed a while ago that it is still necessary to explicitly --disable-sse2 (export ECM_CONFIGURE="--disable-sse2" when building Sage) on a Pentium III (which doesn't have SSE2). (In Sage, SSE2 also gets disabled if one uses SAGE_FAT_BINARY=yes for 32-bit builds IIRC, but vanilla GMP-ECM tries to use SSE2 even when configured for pentium3-*-*, implicitly or explicitly.)

Unfortunately my Pentium III meanwhile died, and I haven't tried to revive it yet, but this was true for the latest GMP-ECM 6.4.x at that time as well. I vaguely remember the problem in the configure test was that it only tries to compile with SSE2 instructions, but doesn't try to run the resulting program (if appropriate, i.e., when not cross-compiling). Alternatively, or in addition, SSE2 should simply get disabled if the target is pentium-*-*|pentium2-*-*|pentium3-*-* etc.

zimmermann6 commented 11 years ago
comment:3

Hi Leif,

I wonder how sse2 can be enabled on a Pentium3, since configure.in only enables it (when not requested by the user) for pentium4-*-* | viac7-*-* | i686-*-* | i786-*-*.

Unless we have a concrete example of a computer where it fails, I'm reluctant to add a patch one could not test.

Paul

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:4

Replying to @zimmermann6:

I wonder how sse2 can be enabled on a Pentium3, since configure.in only enables it (when not requested by the user) for pentium4-*-* | viac7-*-* | i686-*-* | i786-*-*.

Hmmm, it seems somehow pentium3-* gets mapped to i686-*, which then triggers the use of SSE2 (if "supported"; the test apparently doesn't try to run a program with SSE2 instructions, as mentioned).

Just tried configuring GMP-ECM 6.4.4-rc2 on a Pentium4 (i.e., a 32-bit machine) with --target=pentium3-linux-gnu, which results in

...
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
...
checking for SSE2 support... yes
...
config.status: linking ../../src/ecm-6.4.4-rc2/ecm-params.h.pentium4 to ecm-params.h
config.status: linking ../../src/ecm-6.4.4-rc2/mul_fft-params.h.default to mul_fft-params.h
config.status: executing depfiles commands
config.status: executing libtool commands
configure: Configuration:
configure: Build for host type i686-pc-linux-gnu
configure: CC=gcc-4.7.2, CFLAGS=-g -O2
configure: Linking GMP with -lgmp
configure: Not using asm redc code
configure: Using SSE2 instructions in NTT code
configure: Assertions disabled
configure: Shell command execution disabled
configure: OpenMP disabled
configure: Memory debugging disabled

(Similar for --build=pentium3-linux-gnu, and configure ... pentium3-linux-gnu.)

Unless we have a concrete example of a computer where it fails, I'm reluctant to add a patch one could not test.

Well, it's a minor issue, and we could probably also work around that in Sage's spkg-install, at least for the moment...

(I guess not many people will use a "stand-alone" GMP-ECM on such old processors. ;-) )

zimmermann6 commented 11 years ago
comment:5

Leif, configure --target=... is not valid, see configure --help, you should use either --build=... or --host=...

Note that your run did choose Pentium4 as target and not Pentium3:

config.status: linking ../../src/ecm-6.4.4-rc2/ecm-params.h.pentium4 to ecm...

Paul

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:6

P.S.:

The GCC default target also matters, but doesn't really make a difference; with (Ubuntu) GCC 4.4.3 I get:

...
checking for SSE2 support... yes, with -msse2
...
config.status: linking ../../src/ecm-6.4.4-rc2/ecm-params.h.pentium4 to ecm-params.h
config.status: linking ../../src/ecm-6.4.4-rc2/mul_fft-params.h.default to mul_fft-params.h
config.status: executing depfiles commands
config.status: executing libtool commands
configure: Configuration:
configure: Build for host type i686-pc-linux-gnu
configure: CC=gcc-4.4.3, CFLAGS=-g -O2 -msse2
configure: Linking GMP with -lgmp
configure: Not using asm redc code
configure: Using SSE2 instructions in NTT code
configure: Assertions disabled
configure: Shell command execution disabled
configure: OpenMP disabled
configure: Memory debugging disabled
83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:7

Replying to @zimmermann6:

Leif, configure --target=... is not valid, see configure --help, you should use either --build=... or --host=...

That doesn't really matter, i.e., same with --host=... or just the (deprecated) pentium3-pc-linux-gnu parameter. (It always gets mapped to "checking host system type... i686-pc-linux-gnu", although configure is also "checking for pentium3-pc-linux-gnu-gcc... gcc-4.4.3".)

Note that your run did choose Pentium4 as target and not Pentium3:

config.status: linking ../../src/ecm-6.4.4-rc2/ecm-params.h.pentium4 to ecm...

Yes, noticed that, which is also wrong (but only affects tuning parameters, so pretty harmless, right?).

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:8

Replying to @nexttime:

Note that your run did choose Pentium4 as target and not Pentium3:

config.status: linking ../../src/ecm-6.4.4-rc2/ecm-params.h.pentium4 to ecm...

Yes, noticed that, which is also wrong (but only affects tuning parameters, so pretty harmless, right?).

Found some old logs from the native build attempts (with GMP-ECM 6.3, 6.4.2 and 6.4.3b), there it was

...
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
...
checking for SSE2 support... yes
...
config.status: linking ../ecm-6.4.3b/ecm-params.h.default to ecm-params.h
config.status: linking ../ecm-6.4.3b/mul_fft-params.h.default to mul_fft-params.h
...
configure: Build for host type i686-pc-linux-gnu
configure: CC=gcc, CFLAGS=-march=native -mtune=native -O3 -fomit-frame-pointer -DHONORS_CFLAGS
configure: Linking GMP with /scratch/local/coppermine//lib/libgmp.a
configure: Not using asm redc code
configure: Using SSE2 instructions in NTT code
...
83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:9

P.S.: There I used --build=... in the second attempt, which still led to SSE2 getting enabled.

kiwifb commented 11 years ago
comment:10

There is a patch to configure in the current ecm spkg, is it still needed? I am preparing a proper spkg so I can test this on power7.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:11

Replying to @kiwifb:

There is a patch to configure in the current ecm spkg, is it still needed?

Haven't looked at it, but that's presumably the PPC64 patch that went upstream so isn't needed anymore.

I am preparing a proper spkg so I can test this on power7.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:12

Replying to @nexttime:

Replying to @kiwifb:

There is a patch to configure in the current ecm spkg, is it still needed?

Haven't looked at it, but that's presumably the PPC64 patch that went upstream so isn't needed anymore.

Nope, but the patches are all from upstream, so should no longer be necessary.

kiwifb commented 11 years ago
comment:13

ppc64 is kind of the main point of the upgrade. I have asked Paul if the patch was added in a later release upstream and he said he would prepare a release.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:14

Replying to @kiwifb:

ppc64 is kind of the main point of the upgrade. I have asked Paul if the patch was added in a later release upstream and he said he would prepare a release.

Yes, that's 6.4.4[-rc2]; cf. the upstream changelog in the ticket's description.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:15

Replying to @zimmermann6:

I wonder how sse2 can be enabled on a Pentium3, since configure.in only enables it (when not requested by the user) for pentium4-*-* | viac7-*-* | i686-*-* | i786-*-*.

Actually, GMP-ECM's config.sub "canonicalizes" pentium3-* (as well as pentium2-* and similar) to i686-*, so i686 is not sufficient to assume SSE2 support. (pentium4-* gets i786-*.)

(i686 then apparently triggers the SSE2 configure test, which doesn't try to execute SSE2 instructions.)

zimmermann6 commented 11 years ago
comment:16

to answer all questions:

1) ecm-6.4.4-rc2 should include all bug fixes reported upstream, including the one for powerpc64

2) for the sse2 issue, as said above we (upstream) won't change anything that we can't test on a real machine

Paul

kiwifb commented 11 years ago
comment:17

I started a build with checks but never had time to go back to it because I have been litteraly buried in paperwork after starting. I will check how it did now.

kiwifb commented 11 years ago
comment:18

OK, ecm test suite passed on power7 as well as the sage/libs/libecm.pyx doctests. Something I have just noticed looking at the build log:

checking for cblas_dgemm in -lgslcblas... yes
checking for gsl_blas_dgemm in -lgsl... yes

Is ecm depending on gsl and specifically for generic cblas functions that could be provided by atlas or openblas?

Francois

zimmermann6 commented 11 years ago
comment:19

Is ecm depending on gsl and specifically for generic cblas functions that could be provided by atlas or openblas?

this is not a mandatory dependency. GSL is only used to compute some probability of success, of some macro is defined...

Paul

kiwifb commented 11 years ago
comment:20

Replying to @zimmermann6:

Is ecm depending on gsl and specifically for generic cblas functions that could be provided by atlas or openblas?

this is not a mandatory dependency. GSL is only used to compute some probability of success, of some macro is defined...

Quick check on configure --help..... This is automagic detection, as a Gentoo packager of ecm I object. Furthermore the test is for a generic cblas function (I know generic detection of blas into autoconf is not trivial). It would be better to provide a --with-cblas switch on which you could pass appropriate linking flags for cblas. I am done with the packaging rant.
83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:21

Replying to @zimmermann6:

2) for the sse2 issue, as said above we (upstream) won't change anything that we can't test on a real machine

FWIW, GMP-ECM's config.guess/config.sub and configure[.in] are currently at least inconsistent (which can be seen without having access to a Pentium III ;-) ).

zimmermann6 commented 11 years ago
comment:22

Leif, does the config.guess file shipped with GMP 5.1.1 give better results?

Does $host_alias give pentium4-... on your Pentium 4?

Paul

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:23

Replying to @zimmermann6:

Leif, does the config.guess file shipped with GMP 5.1.1 give better results?

leif@restless:~/src$ gmp-5.1.1/config.guess 
pentium4-pc-linux-gnu
leif@restless:~/src$ ecm-6.4.4-rc2/config.guess 
i686-pc-linux-gnu
leif@restless:~/src$ gmp-5.1.1/config.sub pentium3-linux-gnu
pentium3-pc-linux-gnu
leif@restless:~/src$ ecm-6.4.4-rc2/config.sub pentium3-linux-gnu
i686-pc-linux-gnu
leif@restless:~/src$ gmp-5.1.1/config.sub pentium4-linux-gnu
pentium4-pc-linux-gnu
leif@restless:~/src$ ecm-6.4.4-rc2/config.sub pentium4-linux-gnu
i786-pc-linux-gnu

So yes, you could probably take both from GMP 5.1.1.

Does $host_alias give pentium4-... on your Pentium 4?

host_alias contains the name specified on the command line, so it contains pentium4-* (only) if I explicitly configure with [--host=]pentium4-linux-gnu. (It's otherwise empty.)

zimmermann6 commented 11 years ago
comment:24

Francois, I've removed the GSL/BLAS dependency in the development version.

Paul

zimmermann6 commented 11 years ago
comment:25

GMP-ECM 6.4.4 has been released.

Paul

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

Description changed:

--- 
+++ 
@@ -1,11 +1,15 @@
-A release candidate of GMP-ECM 6.4.4 is available at
-http://www.loria.fr/~kruppaal/ecm-6.4.4-rc2.tar.gz. With respect to 6.3 currently
-used in Sage:
+GMP-ECM 6.4.4 is available at
+[https://gforge.inria.fr/frs/download.php/32159/ecm-6.4.4.tar.gz](https://gforge.inria.fr/frs/download.php/32159/ecm-6.4.4.tar.gz).
+
+With respect to 6.3 currently used in Sage:

Changes between GMP-ECM 6.4.3 and GMP-ECM 6.4.4:

-Paul

kiwifb commented 11 years ago
comment:27

I will give it a spin when I have time.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:28

Replying to @kiwifb:

I will give it a spin when I have time.

I was just going to ask whether you're going to provide an spkg with the final version...

kiwifb commented 11 years ago
comment:29

Replying to @nexttime:

Replying to @kiwifb:

I will give it a spin when I have time.

I was just going to ask whether you're going to provide an spkg with the final version...

I'd rather leave that to someone else. I have a number of things to do. Chase the cvxopt bug on power7 bump the sage-on-gentoo overlay so there is a 5.8.beta to test and official paid work that's supposed to come first :)

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:30

Just revisited #12472 ... (disabling asm redc when compiling with clang).

jdemeyer commented 11 years ago

Author: Jeroen Demeyer

jdemeyer commented 11 years ago

Description changed:

--- 
+++ 
@@ -54,3 +54,4 @@
 * Added Visual Studio 2010 build

+spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/ecm-6.4.4.spkg (diff)

jdemeyer commented 11 years ago
comment:31

I created a spkg by essentially just putting the new upstream sources in.

jdemeyer commented 11 years ago

Attachment: ecm-6.4.4.diff.gz

jdemeyer commented 11 years ago
comment:33

Seems to work fine on the buildbot.

kiwifb commented 11 years ago

Reviewer: François Bissey

kiwifb commented 11 years ago
comment:34

Compile fine and tests pass on power7 as well. The diff for spkg looks ok to me. I am putting this to positive review.

zimmermann6 commented 11 years ago
comment:35

sorry Francois was quicker than myself. Here are my 2 cents.

In SPKG.txt:

 * We currently work around a linker bug on MacOS X 10.5 PPC (with
   GCC 4.2.1) which breaks 'configure' if debug symbols are enabled.
   This *might* get fixed in later upstream releases.

Is that still true, since there is no patch any more?

I notice you don't run "make check". It is highly recommended (maybe in a future revision of the spkg), especially since spkg-install uses different values of CC/CFLAGS than the upstream package would choose.

Paul

kiwifb commented 11 years ago

Work Issues: SPKG,txt clean up

kiwifb commented 11 years ago
comment:36

Replying to @zimmermann6:

sorry Francois was quicker than myself. Here are my 2 cents.

In SPKG.txt:

 * We currently work around a linker bug on MacOS X 10.5 PPC (with
   GCC 4.2.1) which breaks 'configure' if debug symbols are enabled.
   This *might* get fixed in later upstream releases.

Is that still true, since there is no patch any more?

I believe it is fixed as from the description of the patch:

* configure.patch: 
       - Disable "asm-redc" on 32-bit Darwin PPCs (upstream revision 1516 / bug 
         #10646). 
         (Note that this upstream patch is likely to slow down GMP-ECM on 64-bit 
         PPC CPUs running (32-bit) MacOS X, since the extended instruction set of 
         the CPU is no longer exploitet.  A proper fix would just pass an option 
         to Apple's assembler to allow the use of the extended instruction set.)   
           - Fix compilation error on x86 CPUs supporting SSE2. (Sage trac #10252, 
         upstream revision 1546). 

So there is mention of two revisions in gmp-ecm taking care of the problem from that patch. But good spotting, SPKG.txt needs cleaning to remove that statement.

I notice you don't run "make check". It is highly recommended (maybe in a future revision of the spkg), especially since spkg-install uses different values of CC/CFLAGS than the upstream package would choose.

In this case checks are run if SAGE_CHECK is set during building. There are few spkg where the test suite is systematically run. I guess there would be a call for it if the packge was troublesome on a regular basis.

jdemeyer commented 11 years ago
comment:37

Replying to @zimmermann6:

In SPKG.txt:

 * We currently work around a linker bug on MacOS X 10.5 PPC (with
   GCC 4.2.1) which breaks 'configure' if debug symbols are enabled.
   This *might* get fixed in later upstream releases.

Is that still true, since there is no patch any more?

OK, removed that comment.

zimmermann6 commented 11 years ago
comment:38

it is now fine for me. Jeroen, please can you modify to "needs review"?

Paul

jdemeyer commented 11 years ago

Changed reviewer from François Bissey to François Bissey, Paul Zimmermann

jdemeyer commented 11 years ago
comment:39

I would say positive review then, given that François already gave positive review and you (Paul) agreed with the further changes.

zimmermann6 commented 11 years ago
comment:40

I would say positive review then...

yes please!

Paul

jdemeyer commented 11 years ago

Changed work issues from SPKG,txt clean up to none

jdemeyer commented 11 years ago

Merged: sage-5.10.beta0

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:43

Replying to @kiwifb:

Replying to @zimmermann6:

sorry Francois was quicker than myself. Here are my 2 cents.

In SPKG.txt:

 * We currently work around a linker bug on MacOS X 10.5 PPC (with
   GCC 4.2.1) which breaks 'configure' if debug symbols are enabled.
   This *might* get fixed in later upstream releases.

Is that still true, since there is no patch any more?

"We work around ..." != "We patch ...", and the comment was in "Special !Update/Build Instructions", not its "Patches" subsection.

I believe it is fixed as from the description of the patch:

* configure.patch: 
     - Disable "asm-redc" on 32-bit Darwin PPCs (upstream revision 1516 / bug 
       #10646). 
       (Note that this upstream patch is likely to slow down GMP-ECM on 64-bit 
       PPC CPUs running (32-bit) MacOS X, since the extended instruction set of 
       the CPU is no longer exploitet.  A proper fix would just pass an option 
       to Apple's assembler to allow the use of the extended instruction set.)   
         - Fix compilation error on x86 CPUs supporting SSE2. (Sage trac #10252, 
       upstream revision 1546). 

So there is mention of two revisions in gmp-ecm taking care of the problem from that patch. But good spotting, SPKG.txt needs cleaning to remove that statement.

The comment referred to the following part of spkg-install (which is still in, and presumably still necessary -- perhaps ask Dima, so should have been kept):

    case "`uname -srm | tr ' ' '-'`" in
        Darwin-9*-[Pp]ower*)
            # Don't add debug symbols because configure otherwise
            # fails due to a bus error in Apple's 'ld' when trying
            # to determine if global symbols are prefixed with an
            # underscore (cf. ticket:5847#comment:35 ff.):
            echo >&2 "Warning: Disabling debug symbols on MacOS X 10.5" \
                "PowerPC because of a linker (?) bug."
            echo >&2 "See ticket:5847#comment:35" \
                "ff. for details."
            echo >&2
            CFLAGS="-O3 $CFLAGS"
            ;;
        *)
            CFLAGS="-g -O3 $CFLAGS"
    esac

By the way, the attached diff doesn't reflect the latest spkg version.

zimmermann6 commented 8 years ago
comment:44

a release candidate for GMP-ECM is available. See https://lists.gforge.inria.fr/pipermail/ecm-discuss/2016-February/004328.html. It would be nice if someone could try it within Sage.

Paul

kiwifb commented 8 years ago
comment:45

I am looking at it. Remarks so far:

 * QA Notice: The following files contain writable and executable sections
 *  Files with such sections will not work properly (or at all!) on some
 *  architectures/operating systems.  A bug should be filed at
 *  http://bugs.gentoo.org/ to make sure the issue is fixed.
 *  For more information, see:
 * 
 *    https://wiki.gentoo.org/wiki/Hardened/GNU_stack_quickstart
 * 
 *  Please include the following list of files in your report:
 *  Note: Bugs should be filed for the respective maintainers
 *  of the package in question and not hardened@g.o.
 * RWX --- --- usr/lib64/libecm.so.0.0.0

Now I will test drive sage with it.