Opened 7 years ago

Closed 7 years ago

#18247 closed defect (fixed)

MPIR's configure fails with GCC 5.x

Reported by: leif Owned by:
Priority: blocker Milestone: sage-6.7
Component: packages: standard Keywords: GNUC STDC __inline__
Cc: wbhart, fbissey Merged in:
Authors: Leif Leonhardy Reviewers: Jeroen Demeyer
Report Upstream: Completely fixed; Fix reported upstream Work issues:
Branch: 764ad4e (Commits, GitHub, GitLab) Commit: 764ad4e2462639dfb2a6d105004d9b19cf907e26
Dependencies: Stopgaps:

Status badges

Description

...
checking whether gcc-5.0 is gcc... yes
checking compiler gcc-5.0 -m64 ... no, long long reliability test 1
configure: error: could not find a working compiler, see config.log for details
Error configuring MPIR (with CFLAGS unset).

This test already failed with Clang (cf. #13948); now that GCC (5.x) defaults to STDC inlining (as opposed to GNUC inlining) semantics, we have to adapt the test (which is supposed to detect an unrelated bug in older GCC versions) again.

Attachments (1)

MPIR-2.7.0_fix_configure_with_GCC_5.x.upstream.patch (697 bytes) - added by leif 7 years ago.
Patch submitted upstream. (Only fixes acinclude.m4, not configure.)

Download all attachments as: .zip

Change History (19)

comment:1 Changed 7 years ago by leif

The following patch to configure fixes the issue:

  • src/configure

    old new  
    56635663
    56645664#if defined(__GNUC__) && !defined(__clang__)
    56655665typedef unsigned long long t1;typedef t1*t2;
     5666#if defined(__GNUC_STDC_INLINE__)  /* e.g. GCC 5.x default */
     5667extern
     5668#endif
    56665669__inline__ t1 e(t2 rp,t2 up,int n,t1 v0)
    56675670{t1 c,x,r;int i;if(v0){c=1;for(i=1;i<n;i++){x=up[i];r=x+1;rp[i]=r;}}return c;}
    56685671f(){static const struct{t1 n;t1 src[9];t1 want[9];}d[]={{1,{0},{1}},};t1 got[9];int i;
     
    69776980
    69786981#if defined(__GNUC__) && !defined(__clang__)
    69796982typedef unsigned long long t1;typedef t1*t2;
     6983#if defined(__GNUC_STDC_INLINE__)  /* e.g. GCC 5.x default */
     6984extern
     6985#endif
    69806986__inline__ t1 e(t2 rp,t2 up,int n,t1 v0)
    69816987{t1 c,x,r;int i;if(v0){c=1;for(i=1;i<n;i++){x=up[i];r=x+1;rp[i]=r;}}return c;}
    69826988f(){static const struct{t1 n;t1 src[9];t1 want[9];}d[]={{1,{0},{1}},};t1 got[9];int i;

(Haven't messed with configure.ac, its source, yet.)

comment:2 Changed 7 years ago by leif

  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

comment:3 Changed 7 years ago by leif

P.S.: __GNUC_STDC_INLINE__ is implied by -std=gnu11 (and -std=gnu99), which is GCC 5.x's default, while all older versions defaulted to -std=gnu89 (implying __GNUC_GNU_INLINE__).

Changed 7 years ago by leif

Patch submitted upstream. (Only fixes acinclude.m4, not configure.)

comment:4 Changed 7 years ago by leif

Just noticed there are bugs w.r.t. $SAGE_LOCAL/share/mp_config in both MPIR's spkg-install and $SAGE_ROOT/build/install, but that's for another ticket.

(E.g. if I install MPIR with sage -i ... before I've ever run make, MPIR is built without GMP compatibility, due to a syntax error caused by improper quoting. Also, MPIR should always get configured with --enable-gmpcompat if we're bootstrapping GCC. I'd also add an explicit message on whether GMP compatibility is enabled or not. Not sure whether we should also handle the whole stuff after a successful installation if GMP compatibility was enabled in MPIR's spkg-install, and also GMP's spkg-install, i.e., when the default MP library changed by manual installation of the packages.)

comment:5 Changed 7 years ago by leif

  • Report Upstream changed from Reported upstream. No feedback yet. to Completely fixed; Fix reported upstream

comment:6 Changed 7 years ago by leif

  • Authors set to Leif Leonhardy
  • Branch set to u/leif/MPIR_configure_GCC_5.x
  • Commit set to 764ad4e2462639dfb2a6d105004d9b19cf907e26
  • Status changed from new to needs_review

New commits:

764ad4eAdd patch fixing MPIR (2.7.0.alpha12) 'configure' failure with GCC 5.x (#18247)

comment:7 Changed 7 years ago by leif

  • Summary changed from MPIR's configure fails with GCC 5 to MPIR's configure fails with GCC 5.x

comment:8 follow-up: Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Why not unconditionally use static __inline__? I think that should work on all compilers.

comment:9 in reply to: ↑ 8 Changed 7 years ago by leif

  • Status changed from needs_info to needs_review

Replying to jdemeyer:

Why not unconditionally use static __inline__? I think that should work on all compilers.

Thought of that, too (already when we were fixing the test for clang), but this way it's more explicit, and more importantly doesn't change the test at all for older compilers.

After all, upstream will decide...

comment:10 follow-up: Changed 7 years ago by leif

P.S.: I didn't "bump" the patch level because MPIR 2.7.0-alpha12 didn't have one, although it was already patched before (meanwhile, in Sage 6.7.beta3, even twice)... ;-)

comment:11 Changed 7 years ago by fbissey

  • Cc fbissey added

comment:12 follow-up: Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_info

On the other hand, with static __inline__ I will be more at ease regarding other (non-GCC) compilers. Here, you are fixing the test just for GCC but what about other compilers with potentially the same problem?

comment:13 in reply to: ↑ 10 Changed 7 years ago by jdemeyer

Replying to leif:

P.S.: I didn't "bump" the patch level because MPIR 2.7.0-alpha12 didn't have one, although it was already patched before (meanwhile, in Sage 6.7.beta3, even twice)... ;-)

Since this is purely about a package not compiling, there is not even a need to bump the patchlevel:

  • if a user already managed to compile mpir, there is no need for this fix.
  • if a user did not manage to compile mpir, the version number doesn't matter anyway.

comment:14 in reply to: ↑ 12 ; follow-up: Changed 7 years ago by leif

  • Status changed from needs_info to needs_review

Replying to jdemeyer:

On the other hand, with static __inline__ I will be more at ease regarding other (non-GCC) compilers. Here, you are fixing the test just for GCC but what about other compilers with potentially the same problem?

Bike-shedding?

This test is solely for GCC. Other compilers may pretend they're GCC, but then they should also define the respective macros. (We explicitly exclude clang, we could in principle exclude ICC as well, but that's something upstream should eventually decide, and not at all an issue for Sage. I'm not aware of any other relevant compiler identifying as GCC.)

Feel free to submit some other patch upstream.

comment:15 Changed 7 years ago by leif

P.S.: Using static __inline__ would actually modify the test, so it wouldn't be guaranteed that it catches the situation it was written for (namely a bug in older GCC versions).

comment:16 in reply to: ↑ 14 ; follow-up: Changed 7 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

Replying to leif:

This test is solely for GCC.

I missed this part. I still don't like your patch (in fact, I don't even like the test), but I can accept it.

comment:17 in reply to: ↑ 16 Changed 7 years ago by leif

Replying to jdemeyer:

I still don't like your patch (in fact, I don't even like the test), but I can accept it.

I don't like the test either; I'd simply require some minimal version of GCC.

But as I said, it's IMHO up to upstream to clean up / decide on the tests...


but I can accept it.

Thanks. We can discuss again when finally a new upstream release gets out... ;-)

I just don't want to have a Sage 6.7 that cannot even bootstrap Sage's GCC 4.9.2 with GCC 5.x for no real reason; distros with GCC 5.1 will presumably pop up soon.

comment:18 Changed 7 years ago by vbraun

  • Branch changed from u/leif/MPIR_configure_GCC_5.x to 764ad4e2462639dfb2a6d105004d9b19cf907e26
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.