kat-2.1.1/lib: Makefile injects -O3

[tgac]

kat-2.1.1/lib: Makefile injects -O3

[mmokrejs]

I attach a full build.log here.

build.txt

Maybe my boost library needs re-linking so maybe ignore the final linking issue and boost symbols unresolved. Just focus on the "-O3" issue.

[maplesond]

Yes, I try to pass through the environment variables where possible. I inject -O3 as I believe -O0 is the default setting and wanted to ensure optimisations are used by default. I'm open to changing this though. Just to clarify what behaviour you'd like, are you saying you'd prefer -O0 as default, -O2, or that the user environment variables should override any auto-injected settings?

Regarding boost, we static link this into the KAT exe, so just make sure you have all the .a files available and those errors should go away.

[maplesond]

How are you getting on here? Did you get boost linking correctly?

Looking at your log again, I note while the -O3 is injected by default, the -O2 is also added to the GCC arguments from your environment. Does the -O3 take precedence over -O2? Also I don't fully understand if this causing you a serious problem or not. If so let me know and I'll take a look.

[mmokrejs]

Hi Daniel,
I am sorry for my late answer.

I inject -O3 as I believe -O0 is the default setting and wanted to ensure optimisations are used by default.

You should not touch the optimizations by default in general. Compiles have enabled some optimizations by default so there is no need to re-inforce the same thing. In any case, the "proper" way would be only to inject "-O" with no number afterwards. Just drop your -O3 altogether.

Just to clarify what behaviour you'd like, are you saying you'd prefer -O0 as default, -O2, or that the user environment variables should override any auto-injected settings?

The latter. If user has CFLAGS/CXXFLAGS variables set (non-empty) then do not overwrite them, at worst append to them whatever you really need, like path to headers but that is all.

This would be the preferred way in a Makefile if you insist on forcing at least some optimization:

CFLAGS += -O
CXXFLAGS += -O

[maplesond]

Ok fair enough. I'll take a look at this and make the changes for the next release.

[maplesond]

I've been looking into this in more detail. I'll try and break down the pros/cons or each optimisation level as a default for KAT makefiles:

-O0 - I would definitely want the average user to have an optimised binary unless they say otherwise. The whole point of KAT is that it's fast, so I don't want an unoptimised version on the users system unless they explicitly say so.

-O1 - Your suggestion (-O / -O1) will enable optimisation level 1 by default. However, from what I've read there seems to be no reason to prefer level 1 over level 2 AFAIK.

-O2 - This is the default level used for most linux / GNU projects. I'm open to being convinced that the default to -O2 might make more sense than -O3 if there's a compelling reason I haven't thought of.

-O3 - For many projects this is considered too aggressive due to increase in memory usage during compilation and increase in binary size. However, due to the KAT system requirements I doubt this would be an issue for anyone.

So the main issues here for me are whether or not the user can override the default settings. Which they already can: make CXXFLAGS="-O0" for example.

[mmokrejs]

> -O2 - This is the default level used for most linux / GNU projects. I'm open to being convinced that the default to -O2 might make more sense than -O3 if there's a compelling reason I haven't thought of.

Definitely, in gcc manpage there is somewhere stated that -O3 is not guaranteed to be any faster than -O2. It is generally considered too aggressive and also giving more numerically inexact results.

Further to say, other compilers have different number of -O levels, for example icc has one more.

If you want speed with gcc, add -O2 -march=tune option. Adding it straight to Makefile is however a bad idea as it will not work with icc, clang, etc. This should be tested first by a configure macro. I am fine with these but please always use in the Makefile.in in the end

CXXFLAGS += $whatever_you_like

So the main issues here for me are whether or not the user can override the default settings. Which they already can: make CXXFLAGS="-O0" for example.`

Yes but that is unexpected and considered forcing users too much, or at least IMHO.
What works in the most cases is
CXXFLAGS="-O0" make
but that is prevented because of the
CXX = $whatever_you_like
lines.

[maplesond]

I think we may have to agree to disagree on this point. I'm using autotools to manage the make structure of the project so don't really want to commit .in files to the repository unless absolutely necessary. autotools will automatically add the user CXXFLAGS to the Makefiles: https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html

So the user CXXFLAGS gets embedded into the makefiles during configuration-time. If the user wants to override the CXXFLAGS at make-time then they need to add the options as I originally posted: make CXXFLAGS="-O0". So I don't think this approach is non-standard (not for autotools anyhow).

I agree that with regards to optimisations, -O3 may not speed things up much, and there are certainly additional optimisations that could manually be added, and it will increase the binary sizes. However, -O3 is pretty portable AFAIK, and the binary sizes shouldn't be an issue for any KAT users. I think it's fine to leave it at this level.

[mmokrejs]

$ git clone https://github.com/TGAC/KAT.git
Cloning into 'KAT'...
remote: Counting objects: 6970, done.
remote: Compressing objects: 100% (56/56), done.
remote: Total 6970 (delta 21), reused 0 (delta 0), pack-reused 6914
Receiving objects: 100% (6970/6970), 39.46 MiB | 4.96 MiB/s, done.
Resolving deltas: 100% (4518/4518), done.
cd KAT/
$ grep "\-O3" src/*
src/Makefile.am:kat_CXXFLAGS = -g -O3 -fwrapv -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -ansi -pedantic -std=c++11 @AM_CXXFLAGS@
$ export CXXFLAGS="-O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native"
$ export CFLAGS="-O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native"
$ ./autogen.sh 
autoreconf-2.69: Entering directory `.'
autoreconf-2.69: configure.ac: not using Gettext
autoreconf-2.69: running: aclocal --force -I m4 ${ACLOCAL_FLAGS}
autoreconf-2.69: configure.ac: tracing
autoreconf-2.69: configure.ac: adding subdirectory deps/jellyfish-2.2.0 to autoreconf
autoreconf-2.69: Entering directory `deps/jellyfish-2.2.0'
...
$ ./configure
checking for g++... g++
checking whether the C++ compiler works... yes
checking for C++ compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C++ compiler... yes
checking whether g++ accepts -g... yes
checking how to run the C++ preprocessor... g++ -E
checking whether we are using the GNU C++ compiler... (cached) yes
checking whether g++ accepts -g... (cached) yes
checking for a BSD-compatible install... /usr/bin/install -c
checking whether g++ supports C++11 features by default... no
checking whether g++ supports C++11 features with -std=gnu++11... yes
...
config.status: creating config.h
config.status: executing depfiles commands
config.status: executing libtool commands
=== configuring in deps/jellyfish-2.2.0 (/scratch/KAT/deps/jellyfish-2.2.0)
configure: running /bin/sh ./configure --disable-option-checking '--prefix=/usr/local'  'CXXFLAGS=-O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native' 'CFLAGS=-O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native' --cache-file=/dev/null --srcdir=.
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
...
$ make
make  all-recursive
make[2]: Entering directory '/scratch/KAT/src'
  CXX      kat-plot_density.o
  CXX      kat-plot_profile.o
  CXX      kat-plot_spectra_cn.o
  CXX      kat-plot_spectra_mx.o
  CXX      kat-plot_spectra_hist.o
  CXX      kat-plot.o
  CXX      kat-filter_kmer.o
  CXX      kat-filter_sequence.o
^Z
[1]+  Stopped                 make
$

$ grep -n "\-O2" src/*
src/Makefile:225:CFLAGS = -O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native
src/Makefile:231:CXXFLAGS = -O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native -std=gnu++11
$ grep -n "\-O3" src/*
src/Makefile:355:kat_CXXFLAGS = -g -O3 -fwrapv -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -ansi -pedantic -std=c++11 -DCPLUSPLUS
src/Makefile.am:8:kat_CXXFLAGS = -g -O3 -fwrapv -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -ansi -pedantic -std=c++11 @AM_CXXFLAGS@
src/Makefile.in:355:kat_CXXFLAGS = -g -O3 -fwrapv -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -ansi -pedantic -std=c++11 @AM_CXXFLAGS@
$

$ ps -ef |  grep g++
mmokrejs  8971  8914  0 16:01 pts/8    00:00:00 /bin/sh -c echo "  CXX     " kat-filter_sequence.o;g++ -DHAVE_CONFIG_H -I. -I..  -isystem ../deps/seqan-library-2.0.0/include -isystem ../deps/jellyfish-2.2.0/include -isystem ../lib/include -I/usr/include   -g -O3 -fwrapv -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -ansi -pedantic -std=c++11 -DCPLUSPLUS -O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native -std=gnu++11 -MT kat-filter_sequence.o -MD -MP -MF .deps/kat-filter_sequence.Tpo -c -o kat-filter_sequence.o `test -f 'filter_sequence.cc' || echo './'`filter_sequence.cc
mmokrejs  8973  8971  0 16:01 pts/8    00:00:00 /usr/x86_64-pc-linux-gnu/gcc-bin/5.3.0/g++ -DHAVE_CONFIG_H -I. -I.. -isystem ../deps/seqan-library-2.0.0/include -isystem ../deps/jellyfish-2.2.0/include -isystem ../lib/include -I/usr/include -g -O3 -fwrapv -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -ansi -pedantic -std=c++11 -DCPLUSPLUS -O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native -std=gnu++11 -MT kat-filter_sequence.o -MD -MP -MF .deps/kat-filter_sequence.Tpo -c -o kat-filter_sequence.o filter_sequence.cc
mmokrejs  9050  8655  0 16:05 pts/9    00:00:00 grep --colour=auto g++
$

$ ps -ef |  grep gcc
mmokrejs  8973  8971  0 16:01 pts/8    00:00:00 /usr/x86_64-pc-linux-gnu/gcc-bin/5.3.0/g++ -DHAVE_CONFIG_H -I. -I.. -isystem ../deps/seqan-library-2.0.0/include -isystem ../deps/jellyfish-2.2.0/include -isystem ../lib/include -I/usr/include -g -O3 -fwrapv -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -ansi -pedantic -std=c++11 -DCPLUSPLUS -O2 -pipe -maes -mpclmul -mpopcnt -mavx -march=native -std=gnu++11 -MT kat-filter_sequence.o -MD -MP -MF .deps/kat-filter_sequence.Tpo -c -o kat-filter_sequence.o filter_sequence.cc
mmokrejs  8974  8973  3 16:01 pts/8    00:00:09 /usr/libexec/gcc/x86_64-pc-linux-gnu/5.3.0/cc1plus -quiet -I . -I .. -I /usr/include -MD kat-filter_sequence.d -MF .deps/kat-filter_sequence.Tpo -MP -MT kat-filter_sequence.o -D_GNU_SOURCE -D HAVE_CONFIG_H -D CPLUSPLUS -isystem ../deps/seqan-library-2.0.0/include -isystem ../deps/jellyfish-2.2.0/include -isystem ../lib/include filter_sequence.cc -march=sandybridge -mmmx -mno-3dnow -msse -msse2 -msse3 -mssse3 -mno-sse4a -mcx16 -msahf -mno-movbe -maes -mno-sha -mpclmul -mpopcnt -mno-abm -mno-lwp -mno-fma -mno-fma4 -mno-xop -mno-bmi -mno-bmi2 -mno-tbm -mavx -mno-avx2 -msse4.2 -msse4.1 -mno-lzcnt -mno-rtm -mno-hle -mno-rdrnd -mno-f16c -mno-fsgsbase -mno-rdseed -mno-prfchw -mno-adx -mfxsr -mxsave -mxsaveopt -mno-avx512f -mno-avx512er -mno-avx512cd -mno-avx512pf -mno-prefetchwt1 -mno-clflushopt -mno-xsavec -mno-xsaves -mno-avx512dq -mno-avx512bw -mno-avx512vl -mno-avx512ifma -mno-avx512vbmi -mno-clwb -mno-pcommit -mno-mwaitx --param l1-cache-size=32 --param l1-cache-line-size=64 --param l2-cache-size=4096 -mtune=sandybridge -quiet -dumpbase filter_sequence.cc -maes -mpclmul -mpopcnt -mavx -auxbase-strip kat-filter_sequence.o -g -O3 -O2 -Wall -Wextra -Wno-deprecated-declarations -Wno-unused-function -Wno-unused-parameter -Wno-unused-variable -Wno-unused-command-line-argument -Wpedantic -ansi -std=c++11 -std=gnu++11 -fwrapv -fstack-protector-strong -o -
mmokrejs  8975  8973  0 16:01 pts/8    00:00:00 /usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/../../../../x86_64-pc-linux-gnu/bin/as -I . -I .. -I /usr/include --64 -msse2avx -o kat-filter_sequence.o
$

So, I am lucky my CXXFLAGS were appended after your so I override your "-O3".

http://stackoverflow.com/questions/32940860/gcc-optimization-levels-which-is-better
http://stackoverflow.com/questions/11546075/is-optimisation-level-o3-dangerous-in-g
https://wiki.gentoo.org/wiki/GCC_optimization#-O

Thank you for your time anyway.

[maplesond]

Yes, that's how it should work. I think if I can summarise the issue here, it's that environment variables won't be respected if there are changes between configuration and make time. That's just how the autotools system works though, and there are workarounds as discussed so I don't think it will be a big problem for users.

Thanks for the interesting discussion and the links. It's always worth refreshing the memory about all the compiler switches. I've read the last link before but the first one has some useful tips for me.