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

-march=native option makes gcc compiler to fail on Power8 system with Ubuntu #1591

Closed
sdmonov opened this issue Nov 16, 2017 · 8 comments
Closed
Labels
bug Bugs and behaviour differing from documentation

Comments

@sdmonov
Copy link

sdmonov commented Nov 16, 2017

My Environment:

  • Operating System: Ubuntu 16.04.3 LTS on Power8
  • Python Version Used: Python 3.6.2 |Anaconda, Inc.| (default, Sep 15 2017, 20:38:23) [GCC 4.8.4] on linux
  • spaCy Version Used: Master from 11/16/2017 (commit a3d4dd1)
  • gcc version: 5.4.0 20160609 (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.5)
  • System Information: PowerNV 8335-GTB with POWER8NVL CPU

Recent commit 79fcf85 added -march=native to the compile options for gcc/clang in setup.py. The problem is that -march=native is not supported by all the gcc compilers and as far as I know some older versions of clang too. I've tested it with gcc on ppc64le. Also tested gcc-arm-linux-gnueabi ARM cross compiler on x86 with Ubuntu 14.04.

This is the output of the ppc64le:

............
copying spacy/tokens/doc.pyx -> build/lib.linux-ppc64le-3.6/spacy/tokens
copying spacy/tokens/token.pyx -> build/lib.linux-ppc64le-3.6/spacy/tokens
copying spacy/tokens/span.pxd -> build/lib.linux-ppc64le-3.6/spacy/tokens
copying spacy/tokens/token.pxd -> build/lib.linux-ppc64le-3.6/spacy/tokens
copying spacy/tokens/__init__.pxd -> build/lib.linux-ppc64le-3.6/spacy/tokens
copying spacy/tokens/doc.pxd -> build/lib.linux-ppc64le-3.6/spacy/tokens
copying spacy/tests/tokenizer/sun.txt -> build/lib.linux-ppc64le-3.6/spacy/tests/tokenizer
running build_ext
building 'spacy.parts_of_speech' extension
creating build/temp.linux-ppc64le-3.6
creating build/temp.linux-ppc64le-3.6/spacy
gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/home/user/miniconda3/envs/spacy/include/python3.6m -I/home/user/AI/spaCy/include -I/home/user/miniconda3/envs/spacy/include/python3.6m -c spacy/parts_of_speech.cpp -o build/temp.linux-ppc64le-3.6/spacy/parts_of_speech.o -O3 -Wno-strict-prototypes -Wno-unused-function -march=native
gcc: error: unrecognized command line option ‘-march=native’
error: command 'gcc' failed with exit status 1

After removing the -march=native option, spaCy builds and runs just fine.

Here is the output from the arm cross compiler.

................
writing top-level names to spacy.egg-info/top_level.txt
reading manifest file 'spacy.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'spacy.egg-info/SOURCES.txt'
installing library code to build/bdist.linux-x86_64/egg
running install_lib
running build_py
running build_ext
building 'spacy.parts_of_speech' extension
arm-linux-gnueabihf-gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/home/moni/miniconda3/envs/pytorch/include/python3.6m -I/home/moni/src/ibm/AI/spaCy/include -I/home/moni/miniconda3/envs/pytorch/include/python3.6m -c spacy/parts_of_speech.cpp -o build/temp.linux-x86_64-3.6/spacy/parts_of_speech.o -O3 -Wno-strict-prototypes -Wno-unused-function -march=native
cc1plus: error: unrecognized argument in option ‘-march=native’
cc1plus: note: valid arguments to ‘-march=’ are: armv2 armv2a armv3 armv3m armv4 armv4t armv5 armv5e armv5t armv5te armv6 armv6-m armv6j armv6k armv6s-m armv6t2 armv6z armv6zk armv7 armv7-a armv7-m armv7-r armv7e-m armv8-a armv8-a+crc iwmmxt iwmmxt2 native
cc1plus: warning: command line option ‘-Wno-strict-prototypes’ is valid for C/ObjC but not for C++ [enabled by default]
error: command 'arm-linux-gnueabihf-gcc' failed with exit status 1

I guess the best solution would be to add code to detect the best CPU flags and add them as -march parameters or simply detect if -march=native is supported by the compiler and only use it in such case.

@honnibal honnibal added the bug Bugs and behaviour differing from documentation label Nov 16, 2017
@honnibal
Copy link
Member

honnibal commented Nov 16, 2017

Thanks; very valuable report and analysis.

I think spaCy should not set -march=native. There are a few dot products implemented as basic loops. This is fine so long as the compiler vectorises the loops reasonably well, but I see that cross-platform support is going to be a real problem. Instead of trying to get this right in spaCy, it'll be better to call into a library.

If we get rid of these dot products we can also compile with -O2, to hopefully reduce compile times, which have been creeping upwards. The long compilation is starting to get quite annoying, and there was a report of an out-of-memory error on a 1gb VM.

@rmax
Copy link

rmax commented Nov 16, 2017

Somewhat related, the default -march=native seems to cause problems in the conda packages: conda-forge/spacy-feedstock#24

@sdmonov
Copy link
Author

sdmonov commented Nov 18, 2017

Looks like the best would be the if compiler CPU optimization flags can be provided through env variable for example in case of binaries generation or detected using a library if not provided for local installs optimization.

@bensalilijames
Copy link

I think this might cause #1589

I was building a Docker image with spaCy with a different set of supported CPU instructions to the target set, which results in Illegal instruction. It would seem -march=native causes this to be the case.

I think the built library should be largely independent of target platform. For now, the workaround is simply to build the Docker image on the target platform (instead of "anywhere"!)

@NicolasEhrhardt
Copy link

We weren't able to use spaCy 2.0.5 on EMR because of this issue. We had to publish an internal version that reverses commit a3d4dd1 in order to avoid the Illegal instruction error at import time. I second folks suggesting that the -march=native option should not be the default bdist option.

honnibal added a commit that referenced this issue Jan 14, 2018
@honnibal
Copy link
Member

Thanks all for the report. I've now removed the march=native flag.

@megatesla
Copy link

The closest equivalent option for Power systems is currently -mcpu=native. See these gcc docs for details.

@lock
Copy link

lock bot commented May 12, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bugs and behaviour differing from documentation
Projects
None yet
Development

No branches or pull requests

6 participants