Merge lp:~brandontschaefer/unity/support-Og-and-add-O2-compiler-option into lp:unity

[Brandon Schaefer]

Add -O2 compiler support for RELEASED versions of unity.

Add -Og support when the compiler supports this option.

Add -O2 compiler support for RELEASED versions of unity.

Add -Og support when the compiler supports this option.

For reference on the -Og option.
http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

[Marco Trevisan (Treviño) (3v1n0)]

+1 for the -Og!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3300
http://jenkins.qa.ubuntu.com/job/unity-ci/2/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/2
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/2/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/2

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/2/rebuild

review: Needs Fixing (continuous-integration)

[Jussi Pakkanen (jpakkane)]

There are some issues here (mainly because the existing CMake stuff in Unity is not polished).

You should not overwrite the ???_CXX_FLAGS (yes, debug mode does that, and it really should not): http://voices.canonical.com/jussi.pakkanen/2013/03/26/a-list-of-common-cmake-antipatterns/

Release target type already has -O3 why would you want to replace it with -O2? Note that release mode is not used in package builds, they set their build flags manually.

Build type should be lower case or, at the very least, should work with both upper and lower case.

Does -Og work reliably enough to be enabled by default? If it may lead to "value optimized out" notifications then we might consider putting it behind an option.

review: Needs Fixing

[Brandon Schaefer (brandontschaefer)]

Hey Jussi,

Thanks for the comment and blog post. That was very informative.

I put the -O2, cause I couldn't find any optimizations for the RELEASE build. I figured it would be best to have at lease -O2 if someone built it locally. If its added in manually later then I can remove it. (I wasn't aware of this).

Right, a while ago when they had instructions for building unity they set the build mode to "Debug"... so it should be either "debug" or "DEBUG" which makes sense.

As for the -Og, im not actually sure how reliably it is. Its a new option being added in gcc and from reading about it looked like something that would be nice to include for debug more only. It shouldn't clober anything, at lease from what it said... but yes we should look at how reliable this is!

-Og
    Optimize debugging experience. -Og 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.

Thanks again, Ill look at fixing some of these anti patterns in unity as well.

[Jussi Pakkanen (jpakkane)]

If you want to examine what flags each mode sets, install cmake-curses-gui, run ccmake <your_build_directory> and type 't' to show the advanced options.

[Jussi Pakkanen (jpakkane)]

Sorry, I did not notice that the old code set CMAKE_CXX_FLAGS_RELEASE to empty. There may be a reason for that (at one time Unity included Compiz's CMake macros which did things of unspeakable evil, for instance). If one can't be found the correct fix is to remove the line altogether. That brings in CMake's default flags for release mode.

Unmerged revisions

3300. By Brandon Schaefer on 2013-04-13

* Adds -O2 to the release version
* Adds support for -Og, if the option exists for the CXX compiler

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-04-03 20:28:58 +0000
+++ CMakeLists.txt	2013-04-13 00:44:27 +0000
@@ -17,7 +17,14 @@
 
 set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DGNOME_DESKTOP_USE_UNSTABLE_API -std=c++0x -fno-permissive")
 set (CMAKE_CXX_FLAGS_DEBUG "-g3")
-set (CMAKE_CXX_FLAGS_RELEASE "")
+set (CMAKE_CXX_FLAGS_RELEASE "-O2")
+
+include(CheckCXXCompilerFlag)
+check_cxx_compiler_flag(-Og HAS_OG_FLAG)
+
+if (HAS_OG_FLAG AND CMAKE_BUILD_TYPE MATCHES Debug)
+  set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Og")
+endif (HAS_OG_FLAG AND CMAKE_BUILD_TYPE MATCHES Debug)
 
 option(
   ENABLE_X_SUPPORT