Opened 11 years ago

Closed 10 years ago

#9382 closed defect (duplicate)

atlas not respecting SAGE_FAT_BINARY on i686 systems

Reported by: mariah Owned by: Mariah Lenox
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords:
Cc: Merged in:
Authors: Reviewers: Leif Leonhardy, Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

atlas on i686 systems (x86-Linux) is not respecting SAGE_FAT_BINARY.

The attached mercurial patch adds i686 to the list of other architectures (i386, x86_64) that respect SAGE_FAT_BINARY.

Attachments (2)

atlas-i686-FAT.patch (1.5 KB) - added by mariah 11 years ago.
9382.patch (1.5 KB) - added by ddrake 10 years ago.
updated version of previous patches

Download all attachments as: .zip

Change History (27)

Changed 11 years ago by mariah

comment:1 Changed 11 years ago by mariah

  • Status changed from new to needs_review

comment:2 Changed 11 years ago by was

Very likely positive review -- just need Robert Miller to try it on some 32-bit linux box.

comment:3 Changed 11 years ago by rlm

  • Status changed from needs_review to needs_work

I get an error installing atlas with this patch:

Host system
uname -a:
Linux cicero 2.6.32.12-115.fc12.i686.PAE #1 SMP Fri Apr 30 20:14:08 UTC 2010 i686 i686 i386 GNU/Linux
****************************************************
****************************************************
CC Version
gcc -v
Using built-in specs.
Target: i686-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-languages=c,c++,objc,obj-c++,java,fortran,ada --enable-java-awt=gtk --disable-dssi --enable-plugin --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic --with-arch=i686 --build=i686-redhat-linux
Thread model: posix
gcc version 4.4.4 20100630 (Red Hat 4.4.4-10) (GCC) 
****************************************************
Platform detected to be 32 bits
system_atlas.py:6: DeprecationWarning: os.popen2 is deprecated.  Use the subprocess module.
  fortran = os.popen2(os.environ['SAGE_LOCAL']+'/bin/'+'which_fortran')[1].read()
./spkg-install-script: line 119: syntax error near unexpected token `}'
./spkg-install-script: line 119: `}'
Failed to build ATLAS.

real    0m0.326s
user    0m0.129s
sys     0m0.122s
sage: An error occurred while installing atlas-3.8.3.p13

Looking at the patch, do I see a mismatched single quote?

comment:4 Changed 11 years ago by jhpalmieri

Looking at the patch, do I see a mismatched single quote?

That's what it looks like to me. What if you just delete the quote? Replace

if [ "`uname -p`" = "i386" -o "'`uname -p`" = "i686" -o `uname -p`" = "x86_64" ]; then 

with

if [ "`uname -p`" = "i386" -o "`uname -p`" = "i686" -o `uname -p`" = "x86_64" ]; then 

(no single quote before the second `uname...`).

comment:5 Changed 11 years ago by rlm

The third uname then has a mismatched double quote.

comment:6 follow-up: Changed 11 years ago by mariah

  • Status changed from needs_work to needs_review

Apologies for my silly error. Here is a revised patch 9382.patch and the corresponding spkg atlas-3.8.3.p13.spkg.

comment:7 Changed 10 years ago by ddrake

A similar problem has been noticed in Arch Linux: https://bugs.archlinux.org/task/21592

18:44 < td123> hello
18:45 < td123> it seems that when building sage, it doesn't respect the 
               SAGE_FAT_BINARY='yes' option because I built sage with that 
               option on a corei7 for x86 and someone kept getting illegal 
               instruction on a pentium 3
18:45 < td123> when I looked through the logs, it seemed that atlas (among 
               possibly others) were building with processort specific 
               optimizations
18:48 < td123> I'm the packager for archlinux :) 
http://repos.archlinux.org/wsvn/community/sage-mathematics/trunk/PKGBUILD is 
               the package source

comment:8 Changed 10 years ago by wjp

  • Status changed from needs_review to needs_work

uname -p on linux returns a textual description of the CPU for me, not a canonical architecture name. It looks like this may have to be uname -m on Linux.

On the other hand, on solaris it looks like uname -p is correct.

As td123 on IRC suggested, maybe we should just check both the results of -m and -p for these values?

comment:9 Changed 10 years ago by wjp

  • Authors changed from Mariah Lenox to Mariah Lenox, Willem Jan Palenstijn
  • Status changed from needs_work to needs_review

comment:10 Changed 10 years ago by vbraun

Willem: which machine and which uname outputs are you referring to in comment 8?

Instead of hacking around the current atlas build script, I think it would be a good point to switch to my rewritten atlas spkg at #10226 that detects the configuration in a much cleaner way.

comment:11 Changed 10 years ago by wjp

Nearly every linux machine I've tried, with a few exceptions where uname -m and uname -p give the same output.

Example (a 64 bit Gentoo machine):

mira: ~ > uname -m
x86_64
mira: ~ > uname -p
Intel(R) Xeon(R) CPU X3220 @ 2.40GHz

Changed 10 years ago by ddrake

updated version of previous patches

comment:12 in reply to: ↑ 6 Changed 10 years ago by ddrake

Replying to mariah:

Apologies for my silly error. Here is a revised patch 9382.patch

That patch has another tiny error: it has "uname-p" (no space between "uname" and "-p"). I just uploaded a patch to this ticket in which I fixed that problem. However, there are other problems: on Ubuntu, it seems that uname -p returns "unknown" and uname -m returns the architecture name that we want. I've tried this on several 32-bit and 64-bit (i.e., i686 and x86_64) computers, on Ubuntu 8.04, 10.04, and 10.10 and they all behave the same. So we will need to use "-m" to correctly detect this on Ubuntu.

comment:13 Changed 10 years ago by ddrake

  • Status changed from needs_review to needs_work

comment:14 follow-up: Changed 10 years ago by wjp

  • Status changed from needs_work to needs_review

You're looking at an old version of the patch. The most up to date one is the SPKG in comment #9.

Also, #10226 (which also needs review) might supersede this patch.

comment:15 in reply to: ↑ 14 ; follow-up: Changed 10 years ago by ddrake

Replying to wjp:

Also, #10226 (which also needs review) might supersede this patch.

It does, and I think we should work on that ticket and close this one when #10226 gets merged.

comment:16 in reply to: ↑ 15 ; follow-up: Changed 10 years ago by leif

Replying to ddrake:

Replying to wjp:

Also, #10226 (which also needs review) might supersede this patch.

It does, and I think we should work on that ticket and close this one when #10226 gets merged.

I currently don't know whether the new (Python install script) ATLAS spkg from #10226 supports SAGE_FAT_BINARY as desired, but for the record:

  • uname -p isn't portable (it may return "unknown" even on Linuces), one should use uname -m instead.
  • Rather than having (btw. incomplete)
        if [ ... -o ... -o ... -o ... ]; then ...
    
    one should use
        case "`uname -m`" in
          i[3456]86|i86pc)
            ...
            ;;
          amd64|x86_64)
            ...
            ;;
          ia64) # Itanium
            ...
            ;;
          # etc.
        esac
    
    instead, which is not only more readable, but also more robust (and portable w.r.t. broken / buggy test commands).

comment:17 in reply to: ↑ 16 ; follow-up: Changed 10 years ago by vbraun

For the record, the updated ATLAS spkg from #10226 uses the $UNAME environment variable (which Sage sets) if available, and python's portable platform.system() otherwise:

try:
    conf['system'] = os.environ['UNAME']
except KeyError:
    conf['system'] = platform.system()

Also there is some support for SAGE_FAT_BINARY:

    if os.environ.get('SAGE_FAT_BINARY', 'no') == 'yes' and conf['Intel?']:
        print 'Sage "fat" binary mode set: Building SSE2 only Hammer binary'
        print 'NOTE: This can result in a Sage that is significantly slower at certain numerical'
        print 'linear algebra since full FAT binary support has not been implemented yet.'
        arch = 'HAMMER'
        isa_ext = ('SSE2', 'SSE1')

Though I'm not sure if anybody ever verified that ATLAS, with these settings, indeed does not use any more advanced isa extensions.

comment:18 Changed 10 years ago by vbraun

Oops we are talking about uname -m. For this, I'm using platform.machine():

conf['Intel?'] = (platform.machine() in ('i386', 'i486', 'i586', 'i686', 'x86_64', 
                                         'AMD64', 'i86pc'))
conf['IA64?']   = (platform.processor() == 'ia64')
conf['PPC?']   = (platform.processor() == 'powerpc')
conf['SPARC?'] = (platform.processor() == 'sparc')

comment:19 in reply to: ↑ 17 Changed 10 years ago by leif

Replying to vbraun:

Though I'm not sure if anybody ever verified that ATLAS, with these settings, indeed does not use any more advanced isa extensions.

Regarding that a) the notion of "fat" binaries is misleading (because "fat" originally refers to different code for different [flavours of] processors in the same binary / executable), and b) there's neither a definition nor a consistent practice of what ISA subsets Sage uses if SAGE_FAT_BINARY=yes, I think we can then close this ticket.

If someone complains, we can always change the behaviour of the affected spkg(s); with the exception of AFAIK very rare SIGILL reports for binary distributions Mariah seems to be the only one who actually regularly tests this feature.

We could of course over time collect what different spkgs do, on a wiki page, though.

(For most packages and binary distributions adding -march=... or -mcpu=... to CFLAGS -- or configuring GCC to default to these -- should be sufficient anyway; building binary Sage distributions on less advanced processors is another way.)

comment:20 follow-up: Changed 10 years ago by vbraun

  • Reviewers set to Leif Leonhardy, Volker Braun
  • Status changed from needs_review to positive_review

I agree and will set this ticket to positive review.

Since it is hard/impossible to tell the gcc flags from the compiled binary, I think we should eventually move the SAGE_FAT_BINARY support into the compilerwrapper. It would be easy for the compilerwrapper to set the desired -march / -mtune flags consistently for each binary that we build.

PS: -mcpu is a deprecated synonym for -mtune.

Release manager: this ticket is fixed by #10226 and can be closed.

comment:21 in reply to: ↑ 20 Changed 10 years ago by leif

Replying to vbraun:

PS: -mcpu is a deprecated synonym for -mtune.

Nope, -mcpu selects the instruction set, while -mtune only affects timing-specific choices (selection or reordering of instructions of that subset); -march=foo is equivalent to -mcpu=foo -mtune=foo. (I don't think that's changed recently, though configure supports --with-arch-NN=... and --with-tune-NN=..., NN in {32, 64}.)


Should we cc Jeroen and delete the ticket from the wishlist wiki page?

comment:22 follow-ups: Changed 10 years ago by vbraun

Well http://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/i386-and-x86_002d64-Options.html#i386-and-x86_002d64-Options says:

-mcpu=cpu-type
    A deprecated synonym for -mtune. 

The instruction set is selected with -march.

We should remove the ticket from the wishlist. Jeroen will close this ticket when he gets around to it, no need to contact him.

comment:23 in reply to: ↑ 22 Changed 10 years ago by leif

Replying to vbraun:

-mcpu=cpu-type
    A deprecated synonym for -mtune. 

Maybe a long-lasting typo (at least since GCC 4.3.3, to be reported upstream ;-) ). Sections on other architectures describe the [IMHO] correct meaning. Or it has been inconsistent for x86 from the beginning, and therefore been deprecated.

comment:24 Changed 10 years ago by vbraun

I don't see any consistency across architectures:

  • x86 has -march and -mtune==-mcpu
  • ARM has -march and -mtune==-mcpu (except perhaps with fewer possible values??)
  • MIPS has -march and -mtune but not -mcpu
  • PowerPC does not have -march, but supports -mtune and -mcpu

There is a ton of dead architectures, but I don't think they have any up-to-date interface.

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

  • Authors Mariah Lenox, Willem Jan Palenstijn deleted
  • Milestone changed from sage-4.7.1 to sage-duplicate/invalid/wontfix
  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.