Opened 10 years ago

Closed 10 years ago

Last modified 6 years ago

#11226 closed defect (fixed)

Sympow spkg fails with gcc 4.6.0

Reported by: jdemeyer Owned by: mariah
Priority: blocker Milestone: sage-4.7
Component: packages: standard Keywords: sympow spkg
Cc: Merged in: sage-4.7.rc2
Authors: Jeroen Demeyer Reviewers: David Kirkby
Report Upstream: Reported upstream. Developers deny it's a bug. Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by chapoton)

On Linux Itanium (ia64) systems, sympow builds with gcc 4.6.0 but fails doctests. Example from cleo (RHEL 5.3 ia64 Itanium 2):

sage -t -long  -force_lib devel/sage/sage/lfunctions/sympow.py
**********************************************************************
File "/home/buildbot/build/sage/cleo-1/cleo_full/build/sage-4.7.alpha5/devel/sage-main/sage/lfunctions/sympow.py", line 213:
    sage: sympow.modular_degree(EllipticCurve('11a'))
Exception raised:
    Traceback (most recent call last):
      File "/home/buildbot/build/sage/cleo-1/cleo_full/build/sage-4.7.alpha5/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/buildbot/build/sage/cleo-1/cleo_full/build/sage-4.7.alpha5/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/buildbot/build/sage/cleo-1/cleo_full/build/sage-4.7.alpha5/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_6[2]>", line 1, in <module>
        sympow.modular_degree(EllipticCurve('11a'))###line 213:
    sage: sympow.modular_degree(EllipticCurve('11a'))
      File "/home/buildbot/build/sage/cleo-1/cleo_full/build/sage-4.7.alpha5/local/lib/python/site-packages/sage/lfunctions/sympow.py", line 229, in modular_degree
        raise RuntimeError, "failed to compute modular degree"
    RuntimeError: failed to compute modular degree
**********************************************************************

[...]

The following tests failed:

sage -t -long  -force_lib devel/sage/sage/modular/hecke/submodule.py # 1 doctests failed
sage -t -long  -force_lib devel/sage/sage/modular/abvar/abvar.py # 1 doctests failed
sage -t -long  -force_lib devel/sage/sage/lfunctions/sympow.py # 13 doctests failed
sage -t -long  -force_lib devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py # 17 doctests failed
----------------------------------------------------------------------

Starting up the sympow executable simply gives

sympow 1.018 RELEASE  (c) Mark Watkins -**ERROR** QD_check failed at x[0]

Reported upstream to gcc: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48823

A revised package, which works around this problem by compiling with -fno-expensive-optimizations is http://boxen.math.washington.edu/home/jdemeyer/spkg/sympow-1.018.1.p9.spkg

For other issues related to gcc 4.6.0 see #11216

Attachments (1)

sympow-1.018.1.p8-p9.diff (1.4 KB) - added by jdemeyer 10 years ago.
Diff for the sympow spkg, for reviewing only

Download all attachments as: .zip

Change History (33)

comment:1 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 10 years ago by jdemeyer

On first sight, it looks like compiling with -O1 solves the problem.

comment:3 Changed 10 years ago by drkirkby

  • Description modified (diff)

comment:4 follow-up: Changed 10 years ago by fbissey

Can't say I am surprised. Any clues from the build log?

comment:5 in reply to: ↑ 4 Changed 10 years ago by drkirkby

Replying to fbissey:

Can't say I am surprised.

Me neither!

Dave

comment:6 follow-up: Changed 10 years ago by drkirkby

Has the '-fno-ivopts' option, (which has fixed several gcc-4.6.0 specific issues), been tried?

comment:7 in reply to: ↑ 6 Changed 10 years ago by mariah

Replying to drkirkby:

Has the '-fno-ivopts' option, (which has fixed several gcc-4.6.0 specific issues), been tried?

Yes. I tried compiling with -O3 -fno-ivopts, but sadly the reported problem still exists.

comment:8 follow-up: Changed 10 years ago by mariah

If src/QD.c is compiled with -O1 (and the rest with -O3) then sage -t -long -force_lib devel/sage/sage/lfunctions/sympow.py no longer fails.

I am working on isolating the code in QD.c that demonstrates the gcc-4.6.0 optimization bug.

comment:9 in reply to: ↑ 8 Changed 10 years ago by drkirkby

Replying to mariah:

If src/QD.c is compiled with -O1 (and the rest with -O3) then sage -t -long -force_lib devel/sage/sage/lfunctions/sympow.py no longer fails.

I am working on isolating the code in QD.c that demonstrates the gcc-4.6.0 optimization bug.

You are a brave lady Mariah - any code in SYMPOW is difficult to understand. Bill Hart described it as "virtually obfuscated". Unfortunately, since it not actually standard C (the Sun compiler wont compile it), it can't be entered into The International Obfuscated C Code Contest

http://www.ioccc.org/

I particualry like SYMPOW's Configure script

http://boxen.math.washington.edu/home/kirkby/bad-code/sympow-1.018.1.p7/src/Configure

which starts off

#!/bin/sh

then later has a test to see if sh exists on the system!

SH=`whichexe sh` && echo "#define SH \"$SH\"" >> $CONFIG
if [ -z "$SH" ]; then
  echo "**ERROR**: Could not find sh"; exit 1;
else
  echo "SH = $SH"
fi

Good luck Mariah - I don't envy you.

Dave

comment:10 Changed 10 years ago by mariah

  • Owner changed from tbd to mariah

comment:11 Changed 10 years ago by mariah

  • Authors set to Mariah Lenox
  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. Little or no feedback.
  • Status changed from new to needs_review

gcc-4.6.0 optimization regression of src/QD.c reported to gcc bugzilla http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48823

comment:12 follow-ups: Changed 10 years ago by drkirkby

  • Status changed from needs_review to needs_work

Hi Mariah,

the test in the code

if [ "`uname -m`" = "ia64" ] && [ $CC = "gcc" ]; then 

will fail if CC is set to something like /usr/local/bin/gcc. In this case, $CC is not just gcc.

I would suggest making use of the script $SAGE_ROOT/local/bin/testcc.sh, which actually compiles a bit of code and checks that GNU is defined. Then use something like the following, which I'm doing from memory and have not tested.

if [ "x$UNAME" = xLinux ] && [ "x`$SAGE_LOCAL/bin/testcc.sh $CC`" = xGCC ] && [ "x`$CC -dumpversion 2>/dev/null`" = x4.6.0 ] ; then  

That will

  • Ensure the test only gets performed on Linux. We currently have no evidence to assume this will be an issue on HP-UX, which also runs on the Itanium processor.
  • Since the first test uses $UNAME, it should be faster on non-Linux systems, as there is no need to call any external scripts or programs.
  • $CC really is a GNU-like compiler.

You might want to review #11169 which changes the exit code of the script testcc.sh if it fails. But it wont effect your code here, as you are checking the text output.

Dave

comment:13 in reply to: ↑ 12 ; follow-up: Changed 10 years ago by jdemeyer

Replying to drkirkby:

  • Ensure the test only gets performed on Linux. We currently have no evidence to assume this will be an issue on HP-UX, which also runs on the Itanium processor.

I would prefer to be on the safe side and lower the optimization on any ia64 machine (Linux or not).

comment:14 in reply to: ↑ 13 Changed 10 years ago by drkirkby

Replying to jdemeyer:

Replying to drkirkby:

  • Ensure the test only gets performed on Linux. We currently have no evidence to assume this will be an issue on HP-UX, which also runs on the Itanium processor.

I would prefer to be on the safe side and lower the optimization on any ia64 machine (Linux or not).

Fair enough, so change the test to something like:

if [ "x`$SAGE_LOCAL/bin/testcc.sh $CC`" = xGCC ] && [ "x`$CC -dumpversion 2>/dev/null`" = x4.6.0 ] ; then  

then we will know it is gcc, even if someone sets the environment variable CC to be {{{/usr/local/bin/gcc}}

This release of gcc seems to have caused a fair number of problems for Sage. I'm not aware of any other gcc "upgrade" which has caused so much trouble.

Dave

comment:15 in reply to: ↑ 12 Changed 10 years ago by drkirkby

Replying to drkirkby:

I would suggest making use of the script $SAGE_ROOT/local/bin/testcc.sh, which actually compiles a bit of code and checks that GNU__ is defined.

The trac formatting has screwed that up a bit. The script $SAGE_ROOT/local/bin/testcc.sh checks if __GNU__ is defined (I have to put a couple of exclamation marks to escape the formatting, as two underscores causes the text to be underlined).

Dave

comment:16 Changed 10 years ago by jdemeyer

I simplified the testcase and came up with the following minimal code to demonstrate the problem:

extern int printf (__const char *__restrict __format, ...);

double p0 = 2.718281828459045091e+00;
double p1 = 1.445646891729250158e-16;
double q0 = 3.141592653589793116e+00;
double q1 = 1.224646799147353207e-16;

int main()
{
  double x;
  x = p0*q1 + p1*q0 + p0*q0;
  printf("%a\n", x);

  return 0;
}

Also, -fno-expensive-optimizations solves this problem.

comment:17 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:18 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:19 Changed 10 years ago by jdemeyer

  • Authors changed from Mariah Lenox to Jeroen Demeyer, Mariah Lenox
  • Priority changed from major to blocker
  • Status changed from needs_work to needs_review

New spkg (with uncommitted changes) needs review: http://boxen.math.washington.edu/home/jdemeyer/spkg/sympow-1.018.1.p9.spkg

comment:20 Changed 10 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:21 Changed 10 years ago by jdemeyer

  • Keywords spkg added
  • Milestone changed from sage-4.7.1 to sage-4.7
  • Status changed from needs_work to needs_review

comment:22 follow-up: Changed 10 years ago by drkirkby

  • Status changed from needs_review to needs_work

The changes are not checked in

drkirkby@hawk:~/sage-4.7.rc0/spkg/standard/sympow-1.018.1.p9$ hg status
M SPKG.txt
M spkg-install

That obviously needs fixing. A minor point, gcc 4.6.1 will cause the patch to be applied too, despite the comment saying "Working around problems with gcc 4.6.0 on Itanium systems"

Dave

comment:23 follow-up: Changed 10 years ago by mariah

I recommend that the patch only apply to gcc-4.6.0. It might be fixed in later gcc versions.

comment:24 in reply to: ↑ 23 Changed 10 years ago by jdemeyer

Replying to mariah:

I recommend that the patch only apply to gcc-4.6.0. It might be fixed in later gcc versions.

I think that having the workaround for all versions 4.6.x doesn't really hurt. It is certainly a safer alternative than applying the workaround only for gcc 4.6.0. If at some point, this bug is fixed in a later gcc 4.6.x, then we can still change the spkg accordingly.

comment:25 in reply to: ↑ 22 Changed 10 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Replying to drkirkby:

The changes are not checked in

drkirkby@hawk:~/sage-4.7.rc0/spkg/standard/sympow-1.018.1.p9$ hg status
M SPKG.txt
M spkg-install

That obviously needs fixing.

Never mind this, it will be fixed after positive_review.

A minor point, gcc 4.6.1 will cause the patch to be applied too, despite the comment saying "Working around problems with gcc 4.6.0 on Itanium systems"

I have changed the comment slightly to:

        echo "Detected GCC version `$CC -dumpversion` on Itanium. Adding"
        echo "work-around to fix a bug when sympow is compiled with gcc 4.6.0"

Changed 10 years ago by jdemeyer

Diff for the sympow spkg, for reviewing only

comment:26 Changed 10 years ago by jdemeyer

  • Authors changed from Jeroen Demeyer, Mariah Lenox to Jeroen Demeyer

comment:27 Changed 10 years ago by drkirkby

  • Reviewers set to David Kirkby
  • Status changed from needs_review to positive_review

That looks fine, so positive review, though don't forget to commit the changes.

comment:28 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.rc2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:29 Changed 10 years ago by jdemeyer

  • Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. Developers deny it's a bug.

According to gcc, it's okay to violate the floating-point standard with -O3. They also say the correct workaround is the undocumented option

-ffp-contract=on

comment:30 Changed 10 years ago by jdemeyer

Sorry, it is documented. I was looking at documentation of an older gcc.

comment:31 Changed 10 years ago by jdemeyer

From http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html:

-ffp-contract=style
    -ffp-contract=off disables floating-point expression contraction. -ffp-contract=fast enables floating-point expression contraction such as forming of fused multiply-add operations if the target has native support for them. -ffp-contract=on enables floating-point expression contraction if allowed by the language standard. This is currently not implemented and treated equal to -ffp-contract=off.

    The default is -ffp-contract=fast.

comment:32 Changed 6 years ago by chapoton

  • Description modified (diff)
Note: See TracTickets for help on using tickets.