Closed zimmermann6 closed 7 years ago
Description changed:
---
+++
@@ -1,4 +1,4 @@
-GMP-ECM 7.0 is available at `https://gforge.inria.fr/frs/download.php/file/35642/ecm-7.0.tar.gz`.
+GMP-ECM 7.0 is available at [https://gforge.inria.fr/frs/download.php/file/35642/ecm-7.0.tar.gz](https://gforge.inria.fr/frs/download.php/file/35642/ecm-7.0.tar.gz).
Some issues were reported for the Debian package:
-`https://buildd.debian.org/status/package.php?p=gmp-ecm`, in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?
+[https://buildd.debian.org/status/package.php?p=gmp-ecm](https://buildd.debian.org/status/package.php?p=gmp-ecm), in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?
Cool!
Replying to @zimmermann6:
Some issues were reported for the Debian package: https://buildd.debian.org/status/package.php?p=gmp-ecm, in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?
I fail to see failures there by now (the version listed there also is 7.0.3+ds-1 though).
And no, AFAICT we don't have any PowerPC (neither 32-bit nor 64-bit) nor any FreeBSD buildbot.
I guess François regularly tests on Linux PowerPC64 though, and I think Jeroen still has access to some PowerPC running MacOS X 10.5 (we might no longer support; AFAIK 32-bit OS, not sure, but 64-bit CPU).
Description changed:
---
+++
@@ -1,4 +1,10 @@
-GMP-ECM 7.0 is available at [https://gforge.inria.fr/frs/download.php/file/35642/ecm-7.0.tar.gz](https://gforge.inria.fr/frs/download.php/file/35642/ecm-7.0.tar.gz).
+GMP-ECM 7.0.3 is available at [https://gforge.inria.fr/frs/download.php/file/36043/ecm-7.0.3.tar.gz](https://gforge.inria.fr/frs/download.php/file/36043/ecm-7.0.3.tar.gz).
+
+---
+
+(This was referring to version 7.0:)
Some issues were reported for the Debian package:
+
+
[https://buildd.debian.org/status/package.php?p=gmp-ecm](https://buildd.debian.org/status/package.php?p=gmp-ecm), in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?
"Upgraded" to version 7.0.3 (of July 4th, currently the latest available).
(Strange that this version and 7.0.2 apparently haven't been tagged in the svn repo.)
According to Paul's comment, we can drop the only patch (from #19233) we currently have in Sage.
Should we repackage the tarball with bzip2
?
Replying to @nexttime:
Should we repackage the tarball with
bzip2
?
900K ecm-7.0.3.tar.bz2
1.1M ecm-7.0.3.tar.gz
Author: Leif Leonhardy
Note that the patch is preliminary (cf. the commit message / ticket description), but I'm setting it to "needs review" as there's something that can already be tested.
New commits:
c0f7846 | GMP-ECM: #20385: Upgrade package to 7.0.3 (preliminary patch, see below) |
Branch: u/leif/gmp-ecm-7.x
Commit: c0f7846
Description changed:
---
+++
@@ -8,3 +8,29 @@
[https://buildd.debian.org/status/package.php?p=gmp-ecm](https://buildd.debian.org/status/package.php?p=gmp-ecm), in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?
+
+---
+
+Upgrade GMP-ECM to 7.0.3
+
+* I haven't repackaged the upstream tarball (`.tar.gz`) [yet].
+ Not sure whether we could/should also delete some stuff not
+ needed for Sage (cf. `SPKG.txt`); at least there's no `spkg-src`
+ script.
+
+* I haven't touched `spkg-install` yet. We may address potential
+ issues with `SAGE_FAT_BINARY` (mentioned both there and in `SPKG.txt`),
+ as well as known problems with `-march=native -O3` on bdver[1-4]
+ (AMD FX-???? !Bulldozer/Piledriver, AMD Opteron 6[23]xx, etc.)
+ where we should add `-mno-xop` because of bugs in (AFAIK) **every**
+ GCC version that supports these processors, silently leading
+ to broken code ("just" testsuite failures IIRC).
+ Also, we currently copy Sage's `config.guess` (because it was newer
+ at some point), which presumably is no longer necessary (but
+ shouldn't hurt either, provided we keep Sage's up-to-date *and*
+ GMP-ECM won't rely on its own).
+
+* The doctests in `sage/libs/libecm.pyx` may need some changes,
+ especially with respect to `verbose` mode tests. (But I'll presumably
+ work on them elsewhere anyway, independently of this upgrade.)
+
Replying to [ticket:20385 leif's ticket description]:
- I haven't touched
spkg-install
yet. We may address [...] known problems with-march=native -O3
on bdver[1-4] (AMD FX-???? !Bulldozer/Piledriver, AMD Opteron 6[23]xx, etc.) where we should add-mno-xop
because of bugs in (AFAIK) every GCC version that supports these processors, silently leading to broken code ("just" testsuite failures IIRC).
This is still true for 7.0.3 (and Ubuntu's GCC 4.6.3 at least), while what used to be make check
is now make longcheck
:
...
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
Input number is 2277189375098448170118558775447117254551111605543304035536750762506158547102293199086726265869065639109 (103 digits)
Using B1=1000000, B2=0, polynomial x^1, x0=3
Step 1 took 584ms
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
############### ERROR ###############
Expected return code 14 but got 0
make: *** [longcheck] Error 1
I think we should do make check && make longcheck
in spkg-check
then.
Ooops, on the other hand make check
now fails with -march=native -mno-xop -O3 -g
(while it passed without -mno-xop
?!?!):
FAIL: test.pp1
PASS: test.pm1
PASS: test.ecm
============================================================================
Testsuite summary for ecm 7.0.3
============================================================================
# TOTAL: 3
# PASS: 2
# SKIP: 0
# XFAIL: 0
# FAIL: 1
# XPASS: 0
# ERROR: 0
============================================================================
See ./test-suite.log
Please report to ecm-discuss@lists.gforge.inria.fr
============================================================================
make[3]: *** [test-suite.log] Error 1
make[3]: Leaving directory `/home/leif/build/ecm-7.0.3-build.bulldozer-gcc-4.6.3-default'
make[2]: *** [check-TESTS] Error 2
make[2]: Leaving directory `/home/leif/build/ecm-7.0.3-build.bulldozer-gcc-4.6.3-default'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/home/leif/build/ecm-7.0.3-build.bulldozer-gcc-4.6.3-default'
make: *** [check-recursive] Error 1
test-suite.log
:
...
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
Input number is 2277189375098448170118558775447117254551111605543304035536750762506158547102293199086726265869065639109 (103 digits)
Using B1=2337233, B2=173055082, polynomial x^1, x0=3
Step 1 took 1368ms
Step 2 took 104ms
********** Factor found in step 2: 4190453151940208656715582382315221647
Found prime factor of 37 digits: 4190453151940208656715582382315221647
Prime cofactor 543423179434447039008165356160798838947285203071935410761431031147 has 66 digits
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
Save file test.pp1.save already exists, will not overwrite
############### ERROR ###############
Expected return code 0 but got 1
FAIL test.pp1 (exit status: 1)
(make longcheck
fails in the same way as it apparently also performs the same test(s).)
Hmmm, or maybe not, and make clean
was just not enough.
After make distclean
, make check
again passes with -march=native -mtune=native -mno-xop -O3 -g
in gmp.h
(but now also --enable-assert
).
But in make longcheck
, I'm now getting a different, apparently unrelated error:
...
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc, --enable-assert] [P+1]
Input number is 18446744073709551337 (20 digits)
Using B1=70823, B2=1320588, polynomial x^1, x0=2
Step 1 took 16ms
********** Factor found in step 1: 18446744073709551337
Found input number N
../../src/ecm-7.0.3/test.pp1: 151: ../../src/ecm-7.0.3/test.pp1: cannot open ./c155: No such file
############### ERROR ###############
Expected return code 0 but got 2
make: *** [longcheck] Error 1
Branch pushed to git repo; I updated commit sha1. New commits:
64eb69e | GMP-ECM: #20385: (Also) run 'make longcheck' in spkg-check |
It seems make longcheck
needs some upstream work, as I'm also getting (ignored!) "meta-errors" (syntax errors etc.) with a clean, fresh build on another (non-AMD) machine.
Replying to @nexttime:
It seems
make longcheck
needs some upstream work, as I'm also getting (ignored!) "meta-errors" (syntax errors etc.) with a clean, fresh build on another (non-AMD) machine.
Apparently also building out-of-tree is broken (more precisely, running make longcheck
in an out-of-tree build, which definitely fails).
Looks like I wasn't on cc even so Paul explicitly added me when the ticket was created. I missed all your action from the last few hours. My gentoo ebuild passes "make check" but then I don't do fancy flags. Will try "make longcheck". Before the release Paul opened a conversation on the last ecm upgrade ticket. Apart from the doctest there is interface change to deal with [#14151 comment:46]
Replying to @kiwifb:
Looks like I wasn't on cc even so Paul explicitly added me when the ticket was created. I missed all your action from the last few hours.
Hmmm, perhaps recheck your trac notification preferences as you already commented on this ticket, too (so you should by default receive notifications even when not explicitly cc'ed).
My gentoo ebuild passes "make check" but then I don't do fancy flags. Will try "make longcheck". Before the release Paul opened a conversation on the last ecm upgrade ticket. Apart from the doctest there is interface change to deal with [#14151 comment:46]
And although I participated in that old ticket, I somehow missed all that discussion from a couple of months ago...
I have to admit I totally forgot Sage's pexpect interface to ecm
.
Is the execstack issue meanwhile resolved upstream?
Branch pushed to git repo; I updated commit sha1. New commits:
0affd29 | GMP-ECM: #20385: Restore note on work-around in SPKG.txt that accidentally got dropped in the 6.4.4 package. |
Hope you get this now, as the cc field was still empty...
Description changed:
---
+++
@@ -34,3 +34,7 @@
especially with respect to `verbose` mode tests. (But I'll presumably
work on them elsewhere anyway, independently of this upgrade.)
+* We definitely also have to fix the pexpect interface to `ecm`
+ in `src/sage/interfaces/ecm.py`, as the command line interface
+ has changed in GMP-ECM 7.x.
+
Added command line interface change to be addressed.
Replying to @nexttime:
Replying to @kiwifb:
Replying to @nexttime:
Is the execstack issue meanwhile resolved upstream?
Nope.
Should we add
-Wl,-z,noexecstack
inspkg-install
until that's fixed upstream?
Yes. Upstream is open to a patch. Problem is the files are generated so you have to carefully fix the generator :(
Still no notification, I'll have to inspect the spam filter tomorrow but I suspect it is not even making it there. May have to change the email address for my notification. And go to the dashboard to see if I missed updates on other tickets.
My goodness! notification this time.
So after N attempts, I finally built GMP-ECM in-place two times (separate folders) on a Bulldozer (Opteron 6276), once with -march=native -mtune=native -O3 -g
, and once with -march=native -mtune=native -mno-xop -O3 -g
.
In both cases, make check
and make longcheck
succeeded now.
(This presumably means the relevant code in GMP-ECM 6.x has changed enough to no longer expose the bugs in GCC when mixing XOP and AVX instructions at higher optimization levels. That's not too surprising, as even minor changes made the issue vanish when I tried (hard!) to create a testcase for bugzilla.)
Replying to @kiwifb:
Problem is the files are generated so you have to carefully fix the generator :(
? LDFLAGS
? Perhaps CCASFLAGS
?
Actually, upstream should add a configure
check and add the appropriate flags when supported.
Do we have to care about anything but Linux (x86/x86_64/ppc/ppc64/SPARC...)?
Anything but GNU ld? (Yes, on Solaris, the Sun linker may get used, but Solaris' not Linux.)
LDFLAGS
, linux x86(_64) only I think. As for ld
I still suffer AIX boxes on ppc64 but sage won't build over there anyway (building python packages need hand holding for starter).
GMP (6.1.1) has this:
dnl Checks whether the stack can be marked nonexecutable by passing an option
dnl to the C-compiler when acting on .s files. Appends that option to ASMFLAGS.
dnl This macro is adapted from one found in GLIBC-2.3.5.
dnl FIXME: This test looks broken. It tests that a file with .note.GNU-stack...
dnl can be compiled/assembled with -Wa,--noexecstack. It does not determine
dnl if that command-line option has any effect on general asm code.
AC_DEFUN([CL_AS_NOEXECSTACK],[
dnl AC_REQUIRE([AC_PROG_CC]) GMP uses something else
AC_CACHE_CHECK([whether assembler supports --noexecstack option],
cl_cv_as_noexecstack, [dnl
cat > conftest.c <<EOF
void foo() {}
EOF
if AC_TRY_COMMAND([${CC} $CFLAGS $CPPFLAGS
-S -o conftest.s conftest.c >/dev/null]) \
&& grep .note.GNU-stack conftest.s >/dev/null \
&& AC_TRY_COMMAND([${CC} $CFLAGS $CPPFLAGS -Wa,--noexecstack
-c -o conftest.o conftest.s >/dev/null])
then
cl_cv_as_noexecstack=yes
else
cl_cv_as_noexecstack=no
fi
rm -f conftest*])
if test "$cl_cv_as_noexecstack" = yes; then
ASMFLAGS="$ASMFLAGS -Wa,--noexecstack"
fi
AC_SUBST(ASMFLAGS)
])
Note that --noexecstack
gets eventually passed to the assembler, not -z noexecstack
to the linker.
Ha, when I was looking at 7.0rc with Paul I only had gmp-6.1.0 and I still had a message about the executable stack from portage if I didn't add -Wl,-z,noexecstack
to LDFLAGS
. I moved to 6.1.1 on 21st of June. And indeed I don't need to add it anymore, it is added automatically:
libtool: link: x86_64-pc-linux-gnu-gcc -shared -fPIC -DPIC .libs/libecm_la-ecm.o .libs/libecm_la-ecm2.o .libs/libecm_la-pm1.o .libs/libecm_la-pp1.o .libs/libecm_la-getprime_r.o .libs/libecm_la-listz.o .libs/libecm_la-lucas.o .libs/libecm_la-stage2.o .libs/libecm_la-mpmod.o .libs/libecm_la-mul_lo.o .libs/libecm_la-polyeval.o .libs/libecm_la-median.o .libs/libecm_la-schoen_strass.o .libs/libecm_la-ks-multiply.o .libs/libecm_la-rho.o .libs/libecm_la-bestd.o .libs/libecm_la-auxlib.o .libs/libecm_la-random.o .libs/libecm_la-factor.o .libs/libecm_la-sp.o .libs/libecm_la-spv.o .libs/libecm_la-spm.o .libs/libecm_la-mpzspm.o .libs/libecm_la-mpzspv.o .libs/libecm_la-ntt_gfp.o .libs/libecm_la-ecm_ntt.o .libs/libecm_la-pm1fs2.o .libs/libecm_la-sets_long.o .libs/libecm_la-auxarith.o .libs/libecm_la-batch.o .libs/libecm_la-parametrizations.o .libs/libecm_la-cudawrapper.o aprtcle/.libs/libecm_la-mpz_aprcl.o -Wl,--whole-archive ./x86_64/.libs/libmulredc.a -Wl,--no-whole-archive -lgmp -lrt -lm -g -O1 -march=native -ggdb -g -Wl,-znoexecstack -Wl,-O1 -Wl,--as-needed -O1 -march=native -ggdb -Wl,-soname -Wl,libecm.so.1 -o .libs/libecm.so.1.0.0
I was just about to note that GMP-ECM consistently uses CCAS
(which defaults to CC
) and CCASFLAGS
(which default to CFLAGS
)...
In any case sage's default is mpir
so we should pass it in the default case. Passing it with gmp-6.1.1 doesn't really hurt.
Oh, GMP-ECM just harcodes it into Makefile.am
:
libecm_la_LDFLAGS = $(LIBECM_LDFLAGS) -version-info 1:0:0 -g -Wl,-znoexecstack
(Executables are not linked with that option, but link to the above library, and presumably don't contain any other asm modules.)
And some linkers require a space IIRC...
Replying to @kiwifb:
In any case sage's default is
mpir
so we should pass it in the default case. Passing it with gmp-6.1.1 doesn't really hurt.
? The option doesn't originate from GMP (see my last comment).
collision. I posted before you. I guess it must have steamed from me communicating my concern to Paul. It definitely wasn't there in 7.0rc and I don't think it was in 7.0 either. One less thing to worry about.
Replying to @kiwifb:
collision. I posted before you. I guess it must have steamed from me communicating my concern to Paul. It definitely wasn't there in 7.0rc and I don't think it was in 7.0 either. One less thing to worry about.
Well, until some linker bails out on that flag... Or is libtool smart enough to remove it in case it isn't supported?
Work Issues: Fix doctests and pexpect interface to ecm
Replying to @nexttime:
Replying to @kiwifb:
collision. I posted before you. I guess it must have steamed from me communicating my concern to Paul. It definitely wasn't there in 7.0rc and I don't think it was in 7.0 either. One less thing to worry about.
Well, until some linker bails out on that flag... Or is libtool smart enough to remove it in case it isn't supported?
Point. Most linkers discard unrecognised option without side effect, the problem is when it can be interpreted in a completely different way by the linker (yes I have a "funny" example of that - it involves AIX's linker). It would be better if the flag was tested for in configure.
Did you already give it a try on MacOS X (recently)?
No, and it will have to wait until morning now because it is my bed time. And I don't think upstream does regular check on OS X.
What's weird is that despite
--enable-shared[=PKGS] build shared libraries [default=no]
--enable-static[=PKGS] build static libraries [default=yes]
and
...
checking whether the gcc-6.1 linker (ld -m elf_x86_64) supports shared libraries... yes
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... no
checking whether to build static libraries... yes
...
still a shared library gets built.
Ok, at least within Sage, only a static library gets installed.
Replying to @nexttime:
Ok, at least within Sage, only a static library gets installed.
I personally don't think it is a good idea. Yes that's supposed to be better performance but if your going to mix it python module it will probably have to be compiled with pic
which will remove part of your performance gain. Unless you have specialised hardware (embedded, BlueGene) the benefits are marginal in my opinion.
Replying to @kiwifb:
Replying to @nexttime:
Ok, at least within Sage, only a static library gets installed.
[...] if your going to mix it python module it will probably have to be compiled with
pic
We of course enforce that in spkg-install
.
which will remove part of your performance gain.
Not necessarily, depends on the processor. You need relative addressing, but not indirect function calls, and most symbols will get resolved at .so build time, not when loading the shared library.
Unless you have specialised hardware (embedded, BlueGene) the benefits are marginal in my opinion.
Hmmm, but having a static library included in the Python extension module (which of course is a shared library) prevents us from further headaches, too, IMHO. :-)
GMP-ECM 7.0.4 is available at https://gforge.inria.fr/frs/download.php/file/36224/ecm-7.0.4.tar.gz.
(This was referring to version 7.0:)
Some issues were reported for the Debian package:
https://buildd.debian.org/status/package.php?p=gmp-ecm, in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?
Upgrade GMP-ECM to 7.0.4
I haven't repackaged the upstream tarball (
.tar.gz
) [yet]. Not sure whether we could/should also delete some stuff not needed for Sage (cf.SPKG.txt
); at least there's nospkg-src
script.I haven't touched
spkg-install
yet. We may address potential issues withSAGE_FAT_BINARY
(mentioned both there and inSPKG.txt
), as well as known problems with-march=native -O3
on bdver[1-4] (AMD FX-???? !Bulldozer/Piledriver, AMD Opteron 6[23]xx, etc.) where we should add-mno-xop
because of bugs in (AFAIK) every GCC version that supports these processors, silently leading to broken code ("just" testsuite failures IIRC). Also, we currently copy Sage'sconfig.guess
(because it was newer at some point), which presumably is no longer necessary (but shouldn't hurt either, provided we keep Sage's up-to-date and GMP-ECM won't rely on its own).The doctests in
sage/libs/libecm.pyx
may need some changes, especially with respect toverbose
mode tests. (But I'll presumably work on them elsewhere anyway, independently of this upgrade.)We definitely also have to fix the pexpect interface to
ecm
insrc/sage/interfaces/ecm.py
, as the command line interface has changed in GMP-ECM 7.x.CC: @kiwifb
Component: packages: standard
Author: Leif Leonhardy, Jeroen Demeyer, François Bissey, Antonio Rojas
Branch:
946c580
Reviewer: Jean-Pierre Flori
Issue created by migration from https://trac.sagemath.org/ticket/20385