Compile with -march=native #27122

kliem · 2019-01-25T11:36:49Z

We should compile Sage with the extra compile argument -march=native for extra performance, at least if SAGE_FAT_BINARY is not set.

In gcc/spkg_configure.ac we check, whether the argument -march=native is accepted.

In build/make/Makefile.in we assemble the flags.

We add -march=native to the compiler flags, if not building fat binaries and the compiler and the package accepts it.

During the build of sage there are now the following flags that can be exported in spkg_install.in with one line, instead of checking SAGE_DEBUG etc. for each individual package, e.g. by

export CFLAGS=$CFLAGS (redundant, use flags as above with -march=native if possible)
export CFLAGS=$CFLAGS_NON_NATIVE (flags as above without -march=native)
export CFLAGS=$CFLAGS_O3 (O3 instead of O2, but only if not in debug mode, add -march=native if possible)
export CFLAGS=$CFLAGS_O3_NON_NATIVE (O3 instead of O2, no -march=native)
export CFLAGS=$ORIGINAL_CFLAGS (use $CFLAGS as set before configure)

Likewise we do with CXXFLAGS, FCFLAGS, F77FLAGS.

We try to respect the users choice and do not overwrite the flags if already set by the user, but only add compile flags if absolutely necessary for packages to work.

Inidividual packages can still be compiled with specified flags or in debug mode using

make CFLAGS=-O2 some-package

or

make SAGE_DEBUG="yes" some-package

Depends on #30375

CC: @sophiasage @vbraun @embray @kiwifb

Component: build

Keywords: Intrinsics, SIMD

Author: Jonathan Kliem

Branch: 7bc1f28

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/27122

The text was updated successfully, but these errors were encountered:

jdemeyer · 2019-01-25T11:57:50Z

comment:1

The topic has nothing to do with polyhedra, many parts of Sage could benefit from it.

jdemeyer · 2019-01-25T11:57:50Z

Changed keywords from Intrinsics, SIMD, Polyhedra, FindPoly to Intrinsics, SIMD

embray · 2019-03-25T10:56:15Z

comment:3

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

embray · 2019-06-14T14:55:02Z

comment:4

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

kliem · 2019-10-23T10:06:35Z

Branch: public/27122

kliem · 2019-10-23T10:06:35Z

New commits:

`cdc8656`	compiling with `-march=native` for GCC from 5.1 and clang from 6.0 if SAGE_FAT_BINARY is not set

kliem · 2019-10-23T10:06:35Z

Commit: cdc8656

kliem · 2019-10-23T10:21:42Z

comment:7

I added a very naive approach of doing it. Wondering if this is remotely ok or going in the right direction.

dimpase · 2019-10-23T11:36:14Z

comment:8

looks OK - not sure whether converting version to float() is normal practice, but that's minor.

I'm also not sure whether this will pass the argument to Cython, or it needs

# distutils: extra_compile_args = -march=native

in *.pxd files.

kliem · 2019-10-23T19:32:22Z

comment:9

I believe it works, but I'm not entirely sure.

This seems to state that sage/stats/distributions/discrete_gaussian_integer.pyx was compiled with -march=native.

I have not set it with CFLAGS, in that case it would have appeared twice on the list.

[sagelib-9.0.beta2] [469/499] gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -fPIC -I./sage/cpython -I/srv/public/kliem/sage/local/lib/python2.7/site-packages/cypari2 -I./sage/libs/ntl -I/srv/public/kliem/sage/local/lib/python2.7/site-packages/
cysignals -I/srv/public/kliem/sage/local/include -I/srv/public/kliem/sage/src -I/srv/public/kliem/sage/src/sage/ext -I/srv/public/kliem/sage/local/include/python2.7 -I/srv/public/kliem/sage/local/lib/python2.7/site-packages/numpy/core/include -Ibuild/cythonized -I/srv/pu
blic/kliem/sage/local/include/python2.7 -c build/cythonized/sage/stats/distributions/discrete_gaussian_integer.c -o build/temp.linux-x86_64-2.7/build/cythonized/sage/stats/distributions/discrete_gaussian_integer.o -fno-strict-aliasing -DCYTHON_CLINE_IN_TRACEBACK=1 -march
=native -D_XOPEN_SOURCE=600 -std=c99                                                                                                                                                                                                                                           
[sagelib-9.0.beta2] build/cythonized/sage/stats/hmm/hmm.c: In function ‘__pyx_pw_4sage_5stats_3hmm_3hmm_25DiscreteHiddenMarkovModel_17_forward’:

kliem · 2019-10-25T08:06:47Z

comment:10

It works for me. All tests passed. No significant time difference.

Operating System: Debian GNU/Linux 10 (buster)
Kernel: Linux 4.19.0-6-amd64
Architecture: x86-64
CPU Model name: Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz

kliem · 2019-10-26T10:29:14Z

comment:13

Works on

Operating System: Ubuntu 18.04.3 LTS
Kernel: Linux 4.15.0-65-generic
Architecture: x86-64
Model name: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz

EmmanuelCharpentier · 2019-10-26T15:33:05Z

comment:14

Success on my notebook running a Python 3-based 9.0.beta2 on top of Debian testing :

Operating System: Debian GNU/Linux bullseye/sid
Kernel: Linux 5.2.0-3-amd64
Architecture: x86-64
Nom de modèle : Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz

HTH,

dimpase · 2019-10-27T08:21:23Z

comment:15

works on Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz (in 32-bit mode), Ubunty 14.04 LTS

orlitzky · 2019-10-28T13:50:07Z

comment:16

I don't think we should be reinventing the thirty-year-old standard CFLAGS here. It looks like setup.py will already use CFLAGS if it's set. I'm not arguing against sane defaults, just saying that appending -march=native on top of my CFLAGS is a bit rude if I've set them to what I want.

Instead, why don't we try to set a decent default for CFLAGS if the user does not already have it set? Typically this amounts to something like

CFLAGS ?= -march=native -O2

in a makefile somewhere. For people who don't care, CFLAGS will get set to something nice, and setup.py will use that value. For the people who have set CFLAGS to something special, nothing goes wrong.

sagetrac-git · 2020-12-15T06:21:35Z

Changed commit from a1ffd9b to ea601ea

mkoeppe · 2020-12-21T04:32:52Z

Changed work issues from Merge latest branch of dependencies to none

mkoeppe · 2020-12-21T04:32:52Z

Changed reviewer from Matthias Koeppe to Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861

mkoeppe · 2020-12-21T18:26:16Z

comment:135

Instead of SAGE_MARCH, perhaps something like CFLAGS_MARCH...?!
And I would suggest to not use the prefix CONFIGURED_ here -- because I think this is not a case as for CFLAGS where we want to respect user variable settings?

sagetrac-git · 2020-12-21T18:48:54Z

Changed commit from ea601ea to 73ecf70

sagetrac-git · 2020-12-21T18:48:54Z

Branch pushed to git repo; I updated commit sha1. New commits:

`73ecf70`	`better naming scheme`

kliem · 2020-12-21T18:53:20Z

comment:137

Replying to @mkoeppe:

Instead of SAGE_MARCH, perhaps something like CFLAGS_MARCH...?!
And I would suggest to not use the prefix CONFIGURED_ here -- because I think this is not a case as for CFLAGS where we want to respect user variable settings?

Sure.

mkoeppe · 2020-12-21T18:56:35Z

comment:138

And I thought you had found a better solution for that free-form business in #30375?

sagetrac-git · 2020-12-21T20:36:29Z

Branch pushed to git repo; I updated commit sha1. New commits:

`7bc1f28`	`remove redundant free-form patch`

sagetrac-git · 2020-12-21T20:36:29Z

Changed commit from 73ecf70 to 7bc1f28

kliem · 2020-12-21T20:37:25Z

comment:140

Replying to @mkoeppe:

And I thought you had found a better solution for that free-form business in #30375?

Right, I forgot to change that.

mkoeppe · 2020-12-21T23:00:47Z

comment:141

Looks good to me. If you have capacity on GH Actions, could you run it one more time? (Ideally with #31064 for the latest version of the cygwin workflow)

kliem · 2020-12-22T07:37:08Z

Changed reviewer from Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861 to Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861, https://github.com/kliem/sage/pull/30/checks

kliem · 2020-12-22T07:39:53Z

comment:143

Replying to @mkoeppe:

Looks good to me. If you have capacity on GH Actions, could you run it one more time? (Ideally with #31064 for the latest version of the cygwin workflow)

Capacities are fine. Actually, I'm not using it most of the time, so if you ping me, I can run something.

mkoeppe · 2020-12-25T19:37:23Z

Changed reviewer from Matthias Koeppe, https://github.com/mkoeppe/sage/actions/runs/434965861, https://github.com/kliem/sage/pull/30/checks to Matthias Koeppe

mkoeppe · 2020-12-25T19:37:23Z

comment:144

Looking good. Thanks for your patience with this long review process.

vbraun · 2020-12-27T23:25:41Z

Changed branch from u/gh-kliem/march_native_on_30375 to 7bc1f28

kliem · 2020-12-28T06:28:06Z

comment:146

Replying to @mkoeppe:

Looking good. Thanks for your patience with this long review process.

Thank you. And thank you in particular for creating the github workflow for testing.

kliem · 2020-12-28T06:28:06Z

Changed commit from 7bc1f28 to none

kliem added this to the sage-8.7 milestone Jan 25, 2019

kliem added c: build labels Jan 25, 2019

This comment has been minimized.

Sign in to view

jdemeyer changed the title ~~Adding -march=native to build/cythonized/setup.py~~ Compile with -march=native Jan 25, 2019

This comment has been minimized.

Sign in to view

embray modified the milestones: sage-8.7, sage-8.8 Mar 25, 2019

embray removed this from the sage-8.8 milestone Jun 14, 2019

kliem added p: major / 3 and removed p: minor / 4 labels Oct 22, 2019

kliem added the s: needs review label Oct 23, 2019

This comment has been minimized.

Sign in to view

mkoeppe added s: positive review and removed s: needs review labels Dec 25, 2020

vbraun removed the s: positive review label Dec 27, 2020

vbraun closed this as completed in 789604e Dec 27, 2020

EmmanuelCharpentier mannequin mentioned this issue Oct 26, 2019

Enable SIMD-instructions for Bitsets #27103

Closed

dimpase mentioned this issue Oct 20, 2021

ubuntu-groovy-standard, debian-bullseye-standard: numerics-related sage testsuite errors #31621

Open

Compile with -march=native #27122

Compile with -march=native #27122

Comments

kliem commented Jan 25, 2019

jdemeyer commented Jan 25, 2019

jdemeyer commented Jan 25, 2019

This comment has been minimized.

This comment has been minimized.

embray commented Mar 25, 2019

embray commented Jun 14, 2019

kliem commented Oct 23, 2019

kliem commented Oct 23, 2019

kliem commented Oct 23, 2019

kliem commented Oct 23, 2019

dimpase commented Oct 23, 2019

kliem commented Oct 23, 2019

This comment has been minimized.

kliem commented Oct 25, 2019

This comment has been minimized.

kliem commented Oct 26, 2019

EmmanuelCharpentier mannequin commented Oct 26, 2019

dimpase commented Oct 27, 2019

orlitzky commented Oct 28, 2019

sagetrac-git mannequin commented Dec 15, 2020

mkoeppe commented Dec 21, 2020

mkoeppe commented Dec 21, 2020

mkoeppe commented Dec 21, 2020

sagetrac-git mannequin commented Dec 21, 2020

sagetrac-git mannequin commented Dec 21, 2020

kliem commented Dec 21, 2020

mkoeppe commented Dec 21, 2020

sagetrac-git mannequin commented Dec 21, 2020

sagetrac-git mannequin commented Dec 21, 2020

kliem commented Dec 21, 2020

mkoeppe commented Dec 21, 2020

kliem commented Dec 22, 2020

kliem commented Dec 22, 2020

mkoeppe commented Dec 25, 2020

mkoeppe commented Dec 25, 2020

vbraun commented Dec 27, 2020

kliem commented Dec 28, 2020

kliem commented Dec 28, 2020