Bug 360249 - Optimize compile flag is used without concern for the variable CMAKE_BUILD_TYPE
Summary: Optimize compile flag is used without concern for the variable CMAKE_BUILD_TYPE
Status: RESOLVED INTENTIONAL
Alias: None
Product: trojita
Classification: Unmaintained
Component: Other (show other bugs)
Version: git
Platform: unspecified All
: NOR wishlist
Target Milestone: ---
Assignee: Trojita default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-08 11:16 UTC by Hohyeis
Modified: 2016-03-09 09:51 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hohyeis 2016-03-08 11:16:38 UTC
It is desirable that it be possible to build without optimisation for compilation speed and predictability of generated code, such as for testing.

Reproducible: Always

Steps to Reproduce:
Build with
cmake -DCMAKE_BUILD_TYPE=None
observe commands used by setting the VERBOSE environment variable to some value for 'make', such as
make VERBOSE=1

Actual Results:  
The compiler uses optimisation

Expected Results:  
whereas it should not.
Comment 1 Jan Kundrát 2016-03-08 11:28:47 UTC
This is on purpose, see https://gerrit.vesnicky.cesnet.cz/r/317 . If you want to override CXXFLAGS, just pass a generic CMAKE_CXX_FLAGS, it will override whatever optimization levels the build system decided to use.

However, please note that there is a reason to not skip optimizations; there are plenty of bits which make a lot of difference (commit 588b1237381641b524b79a611fb3625407d04bfc is just one example I believe, I've seen a *huge* speed difference in operator[](int),...).

Do you have any measurements about the build speed, btw?
Comment 2 Hohyeis 2016-03-08 13:41:11 UTC
The default can still be with optimization. The user's selection of CMAKE_BUILD_TYPE should have effect. The CMake file ideally checks whether CMAKE_BUILD_TYPE is set and can choose a default build type if it is not.

I propose the following optimization settings
CMAKE_CXX_FLAGS_RELEASE=-Ofast
CMAKE_CXX_FLAGS_RELWITHDEBINFO=-Og

I have written CMake code to work with CMAKE_BUILD_TYPE.
I am willing to provide such code to Trojita if it wishes to use it.

I am timing builds with sundry optimization settings.
Comment 3 Jan Kundrát 2016-03-08 14:06:23 UTC
CMake's selection of build type is, unfortunately, only half-baked. In 
particular, in the range of cmake versions, build types and compilers we 
support, plenty of them do not ship with sufficient flags. I would love to 
not mess with the build flags, but that's unfortunately just not possible.

There are several toggles which are of interest, and which are currently 
being toggle by the rather high-level abstraction of a build type. One of 
them is a decision to enable or disable assertions (Q_ASSERT), the other 
decisions affect the availability of the debug symbols, yet others control 
code optimization levels.

I decided that I want to have a convenient way to let other contributors 
build in a way which is suitable for Trojita development. To me, 
CMAKE_BUILD_TYPE=Debug is a nice approximation of this. I need this mode to 
enable assetions and to provide debug symbols. At the same time, because 
this is a version of Trojita which I (and other contributors, presumably) 
end up running on a daily basis, it's desirable to let the compiler do its 
job and still produce an optimized build -- simply because that's what the 
most people are going to use, and to make sure that profiling still 
produces usable data.

At the same time, the code already supports users to override any buid 
parameters by passing appropriate CXXFLAGS. If you are bitten by, for 
example, a compiler optimizing away variables in the optimized debug build, 
you can ask it not to perform any optimizations through, for example, -O0 
or -Og.

This bugreport says that you expected no optimizations to be active when 
doing the debug build type. This won't happen for the reasons outlined 
above. I'm, unfortunately, still doing many rebuilds a day, and I find the 
associated drawbacks to be acceptable -- especially considering that I once 
spent several days optimizing an apparent hotspot which was completely 
elliminated through -O2 "in a real world", which is why the *default* of 
-O2 in debug builds was introduced. Just set your CXXFLAGS to whatever 
value you prefer if it bothers you.
Comment 4 Hohyeis 2016-03-08 15:01:46 UTC
Surprise: CXXFLAGS does not override all compiler parameters, including the optimization.

cmake -DCMAKE_CXX_FLAGS=-O0 ../trojita
make -j1 CXXFLAGS=-O0 VERBOSE=1

There is a second '-O2' which overrides the '-O0'.
There are compiler parameters which come after the CXXFLAGS.

>This bugreport says that you expected no optimizations to be active when doing the debug build type.

I do expect that when I do a debug build type no optimization is done, but I don't see that I wrote that anywhere.
This report proposes that CMAKE_BUILD_TYPE=None would not perform any optimization.
Earlier in this report, I proposed optimizing the debug builds with '-Og'.
Comment 5 Hohyeis 2016-03-08 15:03:02 UTC
Build time for a single process 'make' build was 27 minutes, not including 'make install', which compiled and linked things for more than 1 minute.
Comment 6 Jan Kundrát 2016-03-08 15:14:32 UTC
(In reply to Hohyeis from comment #4)
> Surprise: CXXFLAGS does not override all compiler parameters, including the
> optimization.
> 
> cmake -DCMAKE_CXX_FLAGS=-O0 ../trojita
> make -j1 CXXFLAGS=-O0 VERBOSE=1

I am not sure what is the best way, cmake-wise, but I suspect it is this:

1) start from scratch
2) pass the CXXFLAGS as an env var to cmake, like this: `CXXFLAGS="-march=native -Og" cmake /path/to/source`
3) just run `make` with no additional parameters

The end result which you'll get is something like compiler being executed with options such as `g++ -O2 -I/something -isystem /path/to/qt -Og -c foo.cpp -o CMakeFiles/foo.o`. Note that in this compiler's invocation, flags which come later on take precedence, so even though -O2 appears here, it will be built with -Og.

> There is a second '-O2' which overrides the '-O0'.

Please show us a full, unredacted compiler invocation if you think that there's a bug. It would be interesting to know if you start by `cmake -DCMAKE_CXX_FLAGS=...` or by `CXXFLAGS=... cmake ...`.

> Build time for a single process 'make' build was 27 minutes, not including
> 'make install', which compiled and linked things for more than 1 minute.

Just out of curiosity, what platform and what sort of hardware are you on? If you have multiple CPU cores, there's also the option of using the -j parameter for make.

Also, the build can be sped up quite a bit by passing -GNinja to cmake and running `ninja` instead of `make` if it's available on your platform.
Comment 7 Thomas Lübking 2016-03-08 15:51:36 UTC
If this is about speeding up (re)builds, disable test builds - they consume quite some time and the vast majority if you apply only minor updates to the code.

Building -O0 instead of -02 will hardly gain you /that/ much and unlike -O3 will NOT produce binaries which are completely useless for debugging.

You must set CFLAGS as well as CPP flags and the relevant part is where you feed this into cmake, however

@Jan "Debug" has only "-g" here? (no -O whatsoever)
Comment 8 Jan Kundrát 2016-03-08 16:27:56 UTC
> You must set CFLAGS as well as CPP flags and the relevant part is where you
> feed this into cmake, however

$CFLAGS are flags for the C compiler, and are ignored when building Trojita 
(maybe except one trivial .so for the GPG unit tests in an unmerged 
branch).

$CPPFLAGS are flags for the C and C++ preprocessors, and should not be used 
for specifying these options.

$CXXFLAGS is what matters as they affect flags which are passed to the C++ 
compiler.

> @Jan "Debug" has only "-g" here? (no -O whatsoever)

I'm not sure if this is a question or a bugreport. I see (both in code and 
in the actual compiler invocation here) that our configuration passes -O2 
to all build types.
Comment 9 Thomas Lübking 2016-03-08 16:43:56 UTC
question, i might have altered it somewhen in the past and just wanted to be sure.

RelWithDebInfo puts two -O2 into this, not sure where the  other one comes from unless inherited from CPPFLAGS or CFLAGS *shrug*
Comment 10 Hohyeis 2016-03-08 18:00:41 UTC
I wish to know how to disable tests. I sought the information but did not find it.

I also had CXXFLAGS set and exported when I ran CMake, 

'-Og' is a fairly new debugging level -- only a few years old.
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
>enables optimizations that do not interfere with debugging. It should be the optimization level of choice for the standard edit-compile-debug cycle, offering a reasonable level of optimization while maintaining fast compilation and a good debugging experience. 

I have a multi-core processor of about 4 years ago, but compile with only 1 core so that it is well measurable. I did 'time make -j1' .

I've used Google Ninja because CMake generates Makefiles which are broken for multi-process/threaded compilation, depending on what is in the script. I have not observed that problem in the generated Makefiles for Trojita.
A disadvantage of Google Ninja is that doesn't use environment variables, such as CFLAGS, CXXFLAGS, LDFLAGS.

Here is a full execution line from the build with 'make'.

In a new directory with
export CXXFLAGS='-pipe -march=native -O0'
cmake -DCMAKE_CXX_FLAGS=-O0 ../trojita
make -j1 VERBOSE=1

/usr/bin/c++   -DQT_CORE_LIB -DQT_NETWORK_LIB -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_STRICT_ITERATORS -DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS -DQT_USE_QSTRINGBUILDER -I/Source/trojita/src -I/Build -isystem /usr/include/qt -isystem /usr/include/qt/QtNetwork -isystem /usr/include/qt/QtCore -isystem /usr/lib/qt/mkspecs/linux-g++  -Wall -Wsign-compare -O2 -O0 -std=c++11 -fvisibility=hidden -fvisibility-inlines-hidden -O2 -g -DNDEBUG -fPIC   -fPIC -o CMakeFiles/Streams.dir/src/Streams/IODeviceSocket.cpp.o -c /Source/trojita/src/Streams/IODeviceSocket.cpp

So, the optimization level is '-O2' even though I specified '-O0'. The CXXFLAGS variable in the shell when cmake was called also had '-pipe -march=native' which are missing, so we see that the variable from the shell passed to CMake was not used.

I don't see where it is getting double '-O2' from. It is not coming from my CXXFLAGS or CFLAGS environment variable.  It could be
- the line adding -O2 to CMAKE_CXX_FLAGS is being called twice.
- there is some rule where CMAKE_CXX_FLAGS is used twice.
This could happen by being used once directly as CMAKE_CXX_FLAGS and another being appended to CXXFLAGS.
Comment 11 Jan Kundrát 2016-03-08 23:23:26 UTC
> I wish to know how to disable tests. I sought the information 
> but did not find it.

Probably something like -DCMAKE_DISABLE_FIND_PACKAGE_Qt5Test=true (or 
Qt5Tests?) should do the trick.

Or just do not run `make all` (which is a default target executed by 
`make`), but `make trojita`, that's even easier.

> -fvisibility=hidden -fvisibility-inlines-hidden -O2 -g -DNDEBUG 
> -fPIC   -fPIC

Trojita's build system never specifies this particular chunk of CXXFLAGS 
directly. I've tried grepping my /usr/lib64/cmake, but I don't see an 
obvious culprit in there.

We might workaround by carefuly placing user's CXXFLAGS after any 
library-provided CXXFLAGS, but in order to fix this, we need to know which 
module injects these flags.

Anyway, this worked for me:

  $ cd clean/build/dir
  $ cmake -DCMAKE_CXX_FLAGS="-O0 -march=native" -DCMAKE_BUILD_TYPE=Debug 
~/work/prog/trojita
  $ make -j4 VERBOSE=1
  ...
  /usr/bin/c++   -DQT_CORE_LIB -DQT_NETWORK_LIB -DQT_NO_CAST_FROM_ASCII 
-DQT_NO_CAST_TO_ASCII -DQT_STRICT_ITERATORS -DQT_USE_FAST_CONCATENATION 
-DQT_USE_FAST_OPERATOR_PLUS -DQT_USE_QSTRINGBUILDER -Wall -Wsign-compare 
-O2 -O0 -march=native -std=c++11 -fvisibility=hidden 
-fvisibility-inlines-hidden -Werror -g -fPIC 
-I/home/jkt/work/prog/trojita/src 
-I/home/jkt/work/prog/_trojita-build/cxxflags -isystem /usr/include/qt5 
-isystem /usr/include/qt5/QtNetwork -isystem /usr/include/qt5/QtCore 
-isystem /usr/lib64/qt5/mkspecs/linux-g++    -fPIC -o 
CMakeFiles/Streams.dir/src/Streams/DeletionWatcher.cpp.o -c 
/home/jkt/work/prog/trojita/src/Streams/DeletionWatcher.cpp

> So, the optimization level is '-O2' even though I specified 
> '-O0'. The CXXFLAGS
> variable in the shell when cmake was called also had '-pipe -march=native'
> which are missing, so we see that the variable from the shell 
> passed to CMake was not used.

Considering that you specified both an env var and an explicit option to 
cmake when configuring, it makes sense that the -DCMAKE_CXX_FLAGS has won. 
That's not a bug.

> I don't see where it is getting double '-O2' from. It is not coming from my
> CXXFLAGS or CFLAGS environment variable.  It could be
> - the line adding -O2 to CMAKE_CXX_FLAGS is being called twice.
> - there is some rule where CMAKE_CXX_FLAGS is used twice.
> This could happen by being used once directly as CMAKE_CXX_FLAGS and another
> being appended to CXXFLAGS.

I'm willing to bet that this comes from some external library cmake config. 
As I wrote above, it's apparently build-type-dependant, so add a 
-DCMAKE_BUILD_TYPE=Debug and be done with it.
Comment 12 paalsteek 2016-03-09 09:51:45 UTC
(In reply to Jan Kundrát from comment #11)
> > I wish to know how to disable tests. I sought the information 
> > but did not find it.
> 
> Probably something like -DCMAKE_DISABLE_FIND_PACKAGE_Qt5Test=true (or 
> Qt5Tests?) should do the trick.
> 
> Or just do not run `make all` (which is a default target executed by 
> `make`), but `make trojita`, that's even easier.

Shouldn't -DWITH_TESTS=OFF or similar do the trick? Can't test right now.