Ticket #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: | Work issues: | ||
| Report Upstream: | N/A | Reviewers: | Leif Leonhardy, Volker Braun |
| Authors: | Merged in: | ||
| Dependencies: | Stopgaps: |
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
Change History
comment:2 Changed 3 years ago by was
Very likely positive review -- just need Robert Miller to try it on some 32-bit linux box.
comment:3 Changed 3 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 3 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:6 follow-up: ↓ 12 Changed 3 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 3 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 2 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 2 years ago by wjp
- Status changed from needs_work to needs_review
- Authors changed from Mariah Lenox to Mariah Lenox, Willem Jan Palenstijn
comment:10 Changed 2 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 2 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
comment:12 in reply to: ↑ 6 Changed 2 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:14 follow-up: ↓ 15 Changed 2 years ago by wjp
- Status changed from needs_work to needs_review
comment:15 in reply to: ↑ 14 ; follow-up: ↓ 16 Changed 2 years ago by ddrake
comment:16 in reply to: ↑ 15 ; follow-up: ↓ 17 Changed 22 months 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)
one should use
if [ ... -o ... -o ... -o ... ]; then ...
instead, which is not only more readable, but also more robust (and portable w.r.t. broken / buggy test commands).case "`uname -m`" in i[3456]86|i86pc) ... ;; amd64|x86_64) ... ;; ia64) # Itanium ... ;; # etc. esac
comment:17 in reply to: ↑ 16 ; follow-up: ↓ 19 Changed 22 months 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 22 months 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 22 months 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: ↓ 21 Changed 22 months ago by vbraun
- Status changed from needs_review to positive_review
- Reviewers set to Leif Leonhardy, Volker Braun
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 22 months 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: ↓ 23 ↓ 25 Changed 22 months ago by vbraun
-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 22 months 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 22 months 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 22 months ago by jdemeyer
- Status changed from positive_review to closed
- Milestone changed from sage-4.7.1 to sage-duplicate/invalid/wontfix
- Resolution set to duplicate
- Authors Mariah Lenox, Willem Jan Palenstijn deleted

