Gather and clean up compiler flags

[kliem]

We gather and clean up compiler flags. Also we

• add --enable-fat-binary to configure (as an alternative to setting SAGE_FAT_BINARY)
• add --enable_debug={no|symbols|yes} as alternative to setting SAGE_DEBUG
• compile with -Og -g if SAGE_DEBUG=yes
• compile with -O2 if SAGE_DEBUG=no (some packages with O3 instead)
• compile with -O2 -g otherwise (some packages with O3 instead)

During the build of Sage there are now 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)
• export CFLAGS=$CFLAGS_O3 (O3 instead of O2, but only if not in debug mode)
• export CFLAGS=$ORIGINAL_CFLAGS (use $CFLAGS as set before configure)

Likewise we do with CXXFLAGS, FCFLAGS, F77FLAGS.

We try to respect the user's choice and do not overwrite the flags if already
set by the user, but only add compile flags if 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

This ticket prepares #27122, which adds -march=native to the defaults.

CC: @slel @dimpase

Component: build

Keywords: compile flags, sd111

Author: Jonathan Kliem

Branch/Commit: 3bb3099

Reviewer: Matthias Koeppe

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

[mkoeppe]

comment:3

One point that probably needs discussion is the following.

When an spkg is built, the current design of sage-spkg is to create a self-contained script spkg-install that sets all necessary environment variables (by sourcing sage-env etc.). (This makes it relatively easy to debug the installation process.)

Making package installs depend on exported environment variables in Makefile.in breaks this design.

It would probably be better to create a new script sage-build-env (to complement sage-build-env-config) to take care of these environment variable settings.

[kliem]

comment:4

Ok, I think I'm beginning to understand.

Create a new file build/bin/sage-build-env.in and move the stuff that I added to the makefile there?

Then make sure build/bin/sage-build-env is added to the correct places?

Why should I not add the things to build/bin/sage-build-env-config directly?

[mkoeppe]

comment:5

Replying to @kliem:

Create a new file build/bin/sage-build-env.in and move the stuff that I added to the makefile there?

Then make sure build/bin/sage-build-env is added to the correct places?

Yes

Why should I not add the things to build/bin/sage-build-env-config directly?

Just a suggestion, to keep the analogy to src/bin/sage-env (which implements some logic) and src/bin/sage-env-config (which purely sets some variables). Not very important.

[sagetrac-git]

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

b6cf7c1

Merge branch 'u/gh-kliem/gather_and_clean_up_compile_flags' of git://trac.sagemath.org/sage into u/gh-kliem/gather_and_clean_up_compile_flags_new

[sagetrac-git]

Changed commit from 2ff5630 to b6cf7c1

[sagetrac-git]

Changed commit from b6cf7c1 to 5f4d0b1

[sagetrac-git]

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

5f4d0b1

bash syntax again

[kliem]

comment:8

I think I have mostly figured it out.

But it seems that this is no longer applied. If I configure with some CFLAGS, it seems that when making still the standard CFLAGS are applied. It is consistent with the CFLAGS in sage -sh at least.

[sagetrac-git]

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

a913bbd

correct detextion of empty values

[sagetrac-git]

Changed commit from 5f4d0b1 to a913bbd

[kliem]

comment:10

Ok, seems to work now.

And in the sage -buildsh this thing is run correctly.

[mkoeppe]

comment:11

Looking good; but I think sage-build-env would ideally not be a generated file but rather get its settings from sage-build-env-config.

[mkoeppe]

comment:12

(If both are generated, there is really no point in splitting into these two files)

[sagetrac-git]

Changed commit from a913bbd to 3b82fe9

[sagetrac-git]

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

3b82fe9

Merge branch 'u/gh-kliem/gather_and_clean_up_compile_flags' of git://trac.sagemath.org/sage into u/gh-kliem/gather_and_clean_up_compile_flags_new

[sagetrac-git]

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

d979c22

do not generate `sage-build-env`; respect environment settings at make time

[sagetrac-git]

Changed commit from 3b82fe9 to d979c22

[mkoeppe]

comment:15

Would it not be cleaner if instead of

+# The configured compilation flags.
+if [ "x$CFLAGS" == "x" ]; then
+    export CFLAGS="@ORIGINAL_CFLAGS@"
+fi

in build-env-config, you would unconditionally set a variable ORIGINAL_CFLAGS?

[sagetrac-git]

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

e898e33

more natural naming

[sagetrac-git]

Changed commit from d979c22 to e898e33

[kliem]

comment:17

I don't understand anything at this point anymore.

$ export CFLAGS='-march=native'
$ export CXXFLAGS='-O1'
$ ./configure
$ sage --buildsh
echo $CFLAGS
-march=native
echo $CXXFLAGS
-O1

Okay, this is exactly what we want. But

export CFLAGS='-march=native'
./configure CXXFLAGS='-O1'
sage --buildsh
echo $CFLAGS
-march=native
echo $CXXFLAGS
-march=native

Why? build/bin/sage-build-env-config was configured exactly the same. However, if you echo $CXXFLAGS at the start of build/bin/sage-build-env/config, you see that CXXFLAGS was already set to -march=native. Were does this come from?

• At the moment I overwrite CFLAGS etc. conditionally so that a user may run make CFLAGS='-Og' <some package>. ORIGINAL_CFLAGS should be the same as this value in this case, shouldn't it (and all other related values as well: CFLAGS_O3 etc). Any package may choose any such environment variable and append flags, but should never discard flags the user handed in.

  It is really confusing however, that the exported stuff at configure time is called ORIGINAL_CFLAGS, as @CFLAGS@ already tells us that this is from configure time. I changed that.

---

New commits:

e898e33

more natural naming

[mkoeppe]

comment:18

Note that also sage-env and sage-env-config are sourced.

[mkoeppe]

comment:55

Replying to @kliem:

From https://www.gnu.org/software/autoconf/manual/autoconf-2.66/html_node/Fortran-Compiler.html

The AC_FC_FREEFORM tries to ensure that the Fortran compiler ($FC) allows free-format source code (as opposed to the older fixed-format style from Fortran 77). If necessary, it may add some additional flags to FCFLAGS.

So, AC_FC_FREEFORM has added -ffree-form to my flags, because it feels that this is needed. So I think, we should either respect that or not call AC_FC_FREEFORM.

We run this to find out whether the configured Fortran compiler has a free form mode.

We do this for our decision whether we have to install gfortran or not.

But it must be up to the individual packages how they invoke the Fortran compiler.

[sagetrac-git]

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

87dae61

keep FCFLAGS during gfortran configure

[sagetrac-git]

Changed commit from 94326db to 87dae61

[kliem]

comment:57

Ok. I'm storing and restoring the FCFLAGS during gfortran configure now.
Defining a macro was the only way, I could enforce the order. But maybe there is a better way of doing this.

[mkoeppe]

comment:58

That's fine, yes, AC_FC_FREEFORM is executed out of order.

These lines:

+    sage_saved_fcflags=$FCFLAGS
+    FCFLAGS=$sage_saved_fcflags

need quoting around the assigned value.

[mkoeppe]

comment:59

Also these macro calls are better without the use of the second argument (look at the generated shell code)

+# Save compiler flags as configured by the user.
+AC_SUBST(CFLAGS, "$CFLAGS")
+AC_SUBST(CXXFLAGS, "$CXXFLAGS")
+AC_SUBST(FCFLAGS, "$FCFLAGS")
+AC_SUBST(F77FLAGS, "$F77FLAGS")

[kliem]

comment:60

Replying to @mkoeppe:

Also these macro calls are better without the use of the second argument (look at the generated shell code)

+# Save compiler flags as configured by the user.
+AC_SUBST(CFLAGS, "$CFLAGS")
+AC_SUBST(CXXFLAGS, "$CXXFLAGS")
+AC_SUBST(FCFLAGS, "$FCFLAGS")
+AC_SUBST(F77FLAGS, "$F77FLAGS")

Your proposed change does not work. AC_PROG_CC() changes the value of CFLAGS if not set. AC_SUBST(CFLAGS, "$CFLAGS") will implicitly set CFLAGS (even though possibly to the empty value), but prevents AC_PROG_CC from messing with it.

[sagetrac-git]

Changed commit from 87dae61 to 7577dd4

[sagetrac-git]

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

7577dd4

added quotes

[kliem]

comment:62

Replying to @kliem:

Replying to @mkoeppe:

Also these macro calls are better without the use of the second argument (look at the generated shell code)

+# Save compiler flags as configured by the user.
+AC_SUBST(CFLAGS, "$CFLAGS")
+AC_SUBST(CXXFLAGS, "$CXXFLAGS")
+AC_SUBST(FCFLAGS, "$FCFLAGS")
+AC_SUBST(F77FLAGS, "$F77FLAGS")

Your proposed change does not work. AC_PROG_CC() changes the value of CFLAGS if not set. AC_SUBST(CFLAGS, "$CFLAGS") will implicitly set CFLAGS (even though possibly to the empty value), but prevents AC_PROG_CC from messing with it.

I could deal with it the same way, I did with the FCFLAGS and save all of those flags beforehand. Maybe that is cleaner.

[kliem]

comment:63

Maybe we can take care of this.

[kliem]

Changed keywords from compile flags to compile flags, sd111

[kliem]

Work Issues: make variables SAGE_DEBUG and SAGE_FAT_BINARY precious

[sagetrac-git]

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

3bb3099

prepare preciouos variables

[sagetrac-git]

Changed commit from 7577dd4 to 3bb3099

[kliem]

comment:66

Ok, at least it is prepared for precious variables.

#29507 would have to take care of using this.

[kliem]

Changed work issues from make variables SAGE_DEBUG and SAGE_FAT_BINARY precious to none

[kliem]

comment:67

Actually, it kind of works.

During make, reconfiguration will be done using ./config.status and this one does not alter the variables.

[mkoeppe]

comment:68

Looking great, let's get this in.

[kliem]

comment:69

Thanks again.

[vbraun]

Changed branch from u/gh-kliem/gather_and_clean_up_compile_flags to 3bb3099