Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade MPIR to a more recent upstream release #11616

Closed
nexttime mannequin opened this issue Jul 20, 2011 · 94 comments
Closed

Upgrade MPIR to a more recent upstream release #11616

nexttime mannequin opened this issue Jul 20, 2011 · 94 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Jul 20, 2011

This is a follow-up to #8664 (and a couple of other tickets).

The following new spkg is based on the latest MPIR 2.1.3 spkg, the p9 from #12131:

New spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/mpir-2.4.0.p3.spkg

mpir-2.4.0.p3 (Jeroen Demeyer, April 26th, 2012)

Trac #11616, reviewer fixes:

  • When the first configure run (with CFLAGS unset) of MPIR fails, bail
    out with an error. I am not aware of any system where MPIR fails
    to configure with CFLAGS unset but succeeds with CFLAGS set.
    This implies the following simplifications:
    • We no longer read CC and CFLAGS from /usr/include/gmp.h or
      /usr/local/include/gmp.h
    • We no longer try to add -march=native, we simply use MPIR's flags.
  • Extract $CC and $CFLAGS from Makefile instead of mpir.h, which is
    simpler and more reliable.
  • Added quote_asm.patch to add proper quoting to the m4 in .asm files.
  • Use patch to patch gmp-h.in instead of copying the file.
  • In get_processor_specific_cflags() in spkg-install, also check for
    -mpower* and -mno-power* flags, as MPIR adds -mpower on 32-bit
    PowerPC systems.

mpir-2.4.0.p2 (Leif Leonhardy, April 4th, 2012)

#11616 (upgrading MPIR), further fixes:

  • Before enabling -march=native, minimalistically check whether the
    system's assembler also understands the instructions the compiler emits
    with that option. (Work-around for e.g. GCC 4.6.3 on MacOS X 10.x and
    Intel Core i7-family CPUs with AVX.)
  • Do not unconditionally unset PYTHON, since Sage (>=5.0.beta10) no longer
    pollutes the environment with its package version variables, which previous-
    ly confused yasm's configure.
  • Fix extraction of __GMP_CC and __GMP_CFLAGS from gmp.h, since MPIR
    meanwhile defines these to preprocessor variables (rather than literals).
    Also don't use \+ in sed patterns, as this is less portable.
  • Work around GCC 4.7.0 bug (compilation error) on Linux ia64 (Itanium) by
    almost completely disabling optimization on that platform if GCC 4.7.x
    is detected. This doesn't hurt much if we later rebuild MPIR with a (non-
    broken) GCC from the new GCC spkg. Cf. MPIR doesn't compile with GCC-4.7.0 on ia64 #12765.
  • Do not build the C++ interface and static libraries when bootstrapping the
    GCC spkg, i.e. if SAGE_BUILD_TOOLCHAIN=yes. (GMP/MPIR is a prerequisite
    for it, and MPIR will later get rebuilt with both enabled, with the newly
    built GCC.) Cf. When building GCC, build MPIR without the C++ interface #12782.
  • Fix a potential race condition in yasm's build by patching the re2c source.
    Cf. Race condition in building MPIR/yasm #11844.
  • Add "patch loop" to apply any patches (*.patch) located in patches/.
    Currently only the re2c patch matches that; the prepatched header to support
    Sun's C compiler is still copied over (and only on SunOS, although it doesn't
    do any harm on other platforms).
  • Minor clean-up; e.g. redirect error messages and warnings to stderr,
    quote parameter to --libdir, add some comments and messages, also save
    user's setting of LDFLAGS and ABI.

mpir-2.4.0.p1 (Leif Leonhardy, March 21st, 2012)

  • Upstream upgrade to MPIR 2.4.0 (Upgrade MPIR to a more recent upstream release #11616).
    The 2.4.0.p0 spkg isn't in this history, as it was based
    on the 2.1.3.p4 spkg, i.e., is "on another branch",
    and never got merged into Sage.
  • Remove forcing a sequential make install again, since
    the potential race condition was fixed in MPIR 2.1.4.
  • Fix .hgtags, which contained duplicate entries, and
    was missing others.

This fixes also:

  1. Race condition in building MPIR/yasm #11844: a potential race condition due to yasm when building MPIR in parallel. We've never run into this [before] though. The MPIR 2.4.0.p2 spkg now includes a patch to upstream fixing that.

  2. When building GCC, build MPIR without the C++ interface #12782: when building MPIR to bootstrap GCC (i.e. when SAGE_BUILD_TOOLCHAIN=yes), do not build the C++ interface (and not the static library). This would allow to build Sage on systems which have a C compiler but not a C++ compiler, and also saves time.


The list of changes between MPIR 2.1.3 (more precisely, 2.1.1) and MPIR 2.4.0 is fairly long, so I haven't put them into the description, but attached them in a plain text file.


To install / test the spkg, it is sufficient to just

  1. Download the new spkg and copy it into $SAGE_ROOT/spkg/standard/.

  2. Install the MPIR spkg:

$ ./sage -f spkg/standard/mpir-<version>.spkg
  1. Re-install all packages depending on MPIR:
$ env SAGE_UPGRADING=yes make build

(or omit build to also rebuild the documentation in the same make run).

To run just MPIR's test suite, you can reinstall the spkg with SAGE_CHECK=yes:

$ env SAGE_CHECK=yes ./sage -f spkg/standard/mpir-<version>.spkg

Or, if you haven't yet installed the spkg (but copied it into $SAGE_ROOT/spkg/standard/ as mentioned above), do:

$ env SAGE_CHECK=yes ./sage -i spkg/standard/mpir-<version>.spkg
$ env SAGE_UPGRADING=yes make build # rebuilds all dependent packages

Afterwards you can run make doc to (re)build the documentation, and / or make ptestlong to run Sage's full test suite in parallel.

CC: @RalphieBoy @wbhart @jpflori

Component: packages: standard

Keywords: sd32, GMP, SandyBridge, Westmere, yasm re2c race condition, Linux ia64 Itanium GCC 4.7.0 bug

Author: Leif Leonhardy, Jeroen Demeyer

Reviewer: Jeroen Demeyer, Leif Leonhardy, Volker Braun

Merged: sage-5.0.rc0

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

@nexttime nexttime mannequin added this to the sage-5.0 milestone Jul 20, 2011
@nexttime nexttime mannequin self-assigned this Jul 20, 2011
@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Jul 22, 2011

Note that some changes between 2.1.1 and 2.1.4 may be missing (documented elsewhere, i.e. at least on mpir-devel I think). The segfault was fixed in 2.1.3, the race condition in 2.1.4.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Jul 23, 2011

comment:2

Attachment: MPIR_upstream_changes_between_2.1.1_and_2.4.0.txt

Setting this to "needs review" since the MPIR 2.1.3.p4 from #8664 got positive review again, though so far only by a single reviewer.

The current packages are mainly meant for testing the new upstream releases, hopefully on a variety of platforms supported by Sage; some improvements or changes to Sage's part will most probably follow.

It would just be nice to relatively early know whether any of them (more important, MPIR 2.4.0) causes any problems on one of our platforms. (MPIR 2.5.0 is scheduled for September 1st, which isn't that far...)

@nexttime nexttime mannequin added the s: needs review label Jul 23, 2011
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Aug 18, 2011

comment:3

Replying to @nexttime:

(MPIR 2.5.0 is scheduled for September 1st, which isn't that far...)

News from mpir-devel:

"The release date for MPIR-2.5 was penciled in for 1st Sept, however we have decided to put it back a month to 1st Oct to allow us to get some more code in this release rather than wait for the MPIR-2.6 release on 1st Dec."

@ohanar
Copy link
Member

ohanar commented Aug 23, 2011

comment:4

I've successfully built and tested sage using the 2.4.0.p0 spkg on skynet/eno. I'm currently building/testing on some other machines as well (although, I don't have access to a ppc mac, so I can't help in that regard).

@ohanar
Copy link
Member

ohanar commented Aug 24, 2011

comment:5

Hmm, well I'm getting some random test failures on sage.math and skynet/iras with 2.4.0.p0. I'll try again with 2.3.1.p0 and report back.

@williamstein
Copy link
Contributor

Changed keywords from GMP, SandyBridge, Westmere to sd32, GMP, SandyBridge, Westmere

@nexttime

This comment has been minimized.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 24, 2011

comment:8

Added a reference to #11844 (potential race condition due to yasm when building MPIR in parallel).

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Oct 3, 2011

comment:9

Added a note on the need to set ABI=32 on 64-bit machines running 32-bit operating systems. (The same is true for the already merged MPIR 2.1.3.p4 spkg.)

Also added a reference to #9858; FLINT 1.5.0's test suite won't build with any of the MPIR 2.x spkgs because it uses deprecated functions, and updated the installation instructions.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Oct 5, 2011

Work Issues: Rebase the spkg(s) on the MPIR 2.1.3.p5 spkg from #11896.

@nexttime nexttime mannequin added s: needs work and removed s: needs review labels Oct 5, 2011
@jdemeyer
Copy link

Changed dependencies from #8664 #5847 to none

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

Changed work issues from Rebase the spkg(s) on the MPIR 2.1.3.p5 spkg from #11896. to Rebase the spkg(s) on the MPIR 2.1.3.p7 spkg from #11964

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 19, 2012

Changed work issues from Rebase the spkg(s) on the MPIR 2.1.3.p7 spkg from #11964 to Rebase the spkg(s) on the MPIR 2.1.3.p9 spkg from #12131

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 19, 2012

comment:13

MPIR 2.5.1 has been released on March 11th. We could also give that a try, although 2.4.0 has IMHO been tested a lot and didn't raise any problems within Sage, AFAIK, at least apparently not reproducible ones.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 21, 2012

comment:14

It's not unlikely that I'll soon provide an MPIR 2.4.0.p2 spkg, also fixing a potential race condition in the yasm build, as well as an "experimental" MPIR 2.5.1 spkg.

Please build, test, and review!

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 21, 2012

Author: Leif Leonhardy

@vbraun
Copy link
Member

vbraun commented Apr 19, 2012

comment:62

Looks good to me. If you commit the outstanding changes then I'll give it a positive review.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 19, 2012

comment:63

Replying to @jdemeyer:

Replying to @nexttime:

Reading them from Makefile is absolutely the easiest way.

Is it? You use almost the same sed commands. And it's more error-prone; just imagine we have

CFLAGS = -foo -bar # This is a comment.

Then mpir_cflags would certainly not be empty, but ... (Likewise for CC.) Even the usage of one or more tabs can break your code.

This is all true, but the point is that MPIR (actually, automake) doesn't put those comments, nor do they use TABs. We cannot predict how Makefile will look like in the future, but we also cannot predict how gmp.h or mpir.h will look like in the future. I think it is very unlikely that a future MPIR will break my script in a way that nobody notices.

This doesn't make sense at all. Why do you refuse to use the documented, supposed (and hence safest) way (or location) to get the flags from? (And it's in no way more complicated than what you currently do. You could of course also run the preprocessor on gmp.h, but that's IMHO slight overkill.)

The structure and/or contents of the generated Makefiles is likely to change with other versions of autotools, probably without intent of the MPIR developers.

In any case, you should have just changed the get_upstream_selected_cflags() function, instead of removing it.


You should also add -mno-power* to get_processor_specific_cflags(), since at least a user might use such.

The other changes are reasonable, so I'm ok with the spkg modulo the useless changes w.r.t. extracting CFLAGS (and as already mentioned, I'd just change the .asm files on the fly, not add a large patch which is under revision control for that*).

Don't know whether we should cat the whole config.log in case configure failed in the first place; I'd just print its full filename (and we don't have to "rename" it), like we do in some other spkgs.

[* The "autotools way" would by the way be to check whether m4 bails out on define(,1), and only if that's the case, modify the offending source files.]

@jdemeyer
Copy link

comment:64

Replying to @nexttime:

Why do you refuse to use the documented, supposed (and hence safest) way (or location) to get the flags from?

Is it really documented as such? I don't find any mention of GMP_CC or MPIR_CC in the MPIR manual.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

comment:66

Replying to @nexttime:

You should also add -mno-power* to get_processor_specific_cflags(), since at least a user might use such.

Done.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 26, 2012

comment:67

Replying to @nexttime:

Don't know whether we should cat the whole config.log in case configure failed in the first place; I'd just print its full filename (and we don't have to "rename" it), like we do in some other spkgs.

I see, we don't cat config.log, but the otherwise normal output. (Must have been late...)

Still, I'd print the pathname of config.log, which usually reveals what really went wrong, if configure failed.

@jhpalmieri
Copy link
Member

comment:68

This builds successfully (and passes self-tests) for me on various platforms. I don't think I'm competent to review all of the patches; what can we do to make some progress on this ticket? (One very minor comment: in the last version, I would have used "hg rename" to change gmp-h.in.diff to gmp-h.in.patch.)

@vbraun
Copy link
Member

vbraun commented Apr 27, 2012

comment:69

I think this ticket shows that there is a need for a better management of C(XX)FLAGS in Sage. Ideally via some shell script that is part of Sage, so that not every spkg has to reinvent the wheel. The whole approach of parsing the install of another spkg to recreate its CFLAGS is just wrong, and arguing how to make this bandaid less likely to break in the future is not particularly helpful.

But now is not the time to create some CFLAGS framework. The spkg here seems to work pretty well, is better what we currently ship, is easy enough to read to see what it does, and is unlikely to be improved upon with some minor patches. So lets ship it!

@jdemeyer
Copy link

Changed reviewer from Jeroen Demeyer, Leif Leonhardy to Jeroen Demeyer, Leif Leonhardy, Volker Braun

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 29, 2012

comment:71

For the record:

There seems to be some progress on the GCC 4.7 on ia64 issue, but assuming that

  • GCC 4.7.1 will be released in July, and

  • there'll (hopefully) be some Sage 5.1 or whatever final release before that,

I think we can leave the GCC 4.7.x (rather than 4.7.0) work-around as is here. (Same for the MPFR, GMP-ECM and some other, not yet merged / positively reviewed tickets.)

@jdemeyer
Copy link

comment:72

Leif, do you agree with giving this ticket positive_review?

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 29, 2012

comment:73

Replying to @jdemeyer:

Leif, do you agree with giving this ticket positive_review?

Well, printing the path to config.log in case the first configure run failed is certainly a minor issue (although adding it is a minor change which wouldn't need large-scale testing as well).

Volker said he was waiting for the changes to be committed, and I haven't looked at the latest spkg (w.r.t. usual spkg sanity checks) myself, otherwise I would have set it to positive review already.

So I just don't know whether you were planning to make some further changes, and/or commit them.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 29, 2012

comment:74

Replying to @nexttime:

So I just don't know whether you were planning to make some further changes, and/or commit them.

Also the description still states the p3 was preliminary.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 29, 2012

comment:75

Replying to @nexttime:

Volker said he was waiting for the changes to be committed, and I haven't looked at the latest spkg (w.r.t. usual spkg sanity checks) myself, otherwise I would have set it to positive review already.

FWIW, the current(?) p3 spkg (md5sum 45bd5544cfa568e1d7752f335495c336) looks technically sane to me (except that the changes aren't committed yet).

(I guess it's unnecessary to mention that I of course don't agree on parts of the changelog entry in SPKG.txt.)

@jdemeyer
Copy link

Diff between the 2.4.0.p2 and 2.4.0.p3 spkgs. For reference / review only.

@jdemeyer
Copy link

comment:76

Attachment: mpir-2.4.0.p2-p3.diff.gz

Added path to config.log in case of failure, committed changes.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

Merged: sage-5.0.rc0

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 30, 2012

comment:78

Replying to @vbraun:

I think this ticket shows that there is a need for a better management of C(XX)FLAGS in Sage. Ideally via some shell script that is part of Sage, so that not every spkg has to reinvent the wheel. The whole approach of parsing the install of another spkg to recreate its CFLAGS is just wrong [...]

See #12890 for a partial reply.

W.r.t. "parsing the install of another spkg":

No spkg does this. Some use preprocessor variables defined in other [s]pkgs' C headers, some use Python files of another, or use Python's Makefile settings through distutils, some even link to another's library and call functions for that purpose, others e.g. use pkgconfig. Whether MPFR's, MPC's and GMP-ECM's (upstream) behaviour is appropriate for Sage has already been discussed elsewhere.

The specific problem with MPIR (similar to those with MPFR etc.) simply is that it by default doesn't allow to add or override some CFLAGS. Therefore we have to cheat it by feeding it with its own, slightly modified flags after doing a "dry" build (i.e., running configure once just to see what flags it otherwise would use).

What Jeroen calls "good CFLAGS" are basically -march=...(-related) parameters. Also depending on MPIR's age (especially within Sage), these aren't always optimal. Further, most of MPIR is implemented in assembly language, where CFLAGS are almost completely irrelevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants