Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle RelWithDebInfo in CMake #751

Merged
merged 4 commits into from
Jun 22, 2021
Merged

Handle RelWithDebInfo in CMake #751

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28dc254
Hanlde RelWithDebInfo in CMake
simone-gaia Jun 22, 2021
5923f9d
Update CMakeLists.txt
simone-gaia Jun 22, 2021
3af6d55
Update CMakeLists.txt
simone-gaia Jun 22, 2021
41a0537
Update CMakeLists.txt
simone-gaia Jun 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion production/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ if(BUILD_GAIA_RELEASE OR BUILD_GAIA_LLVM_TESTS)
# LLD/libc++ are necessary for MSan.
set(LLVM_ENABLE_LLD ON CACHE BOOL "")
set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
if(CMAKE_BUILD_TYPE STREQUAL "Debug")
if(CMAKE_BUILD_TYPE STREQUAL "Debug" OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it too much to ask for the name to beReleaseWithDebugInfo?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes :) This is a standard CMake build type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Tobin said.. https://stackoverflow.com/questions/24767450/using-cmake-how-do-you-determine-the-default-compiler-flags-per-build-type

# This speeds up linking by separating debug info.
set(LLVM_USE_SPLIT_DWARF ON CACHE BOOL "")
# This makes debugger startup faster.
Expand All @@ -106,6 +106,15 @@ if(CMAKE_BUILD_TYPE STREQUAL "Debug")
set_property(CACHE SANITIZER PROPERTY STRINGS "ASAN")
endif()

# Release with DebugInfo build-specific options.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all these options are duplicated between Debug and RelWithDebInfo. Can we coalesce all of them except the the optimization level into one if() block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I expect to get rid of those if as soon as we get rid of GAIA_COMPILE_FLAGS/GAIA_LINK_FLAGS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I've already gotten rid of these in the example app cmake files but had a little trouble with the production cmake files (quoting issues), and didn't want to spend more time on it.

if(CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
# Don't omit debug symbols from any built binaries.
add_compile_options(-fno-limit-debug-info)

set(GAIA_COMPILE_FLAGS "-c -Wall -Wextra -O2 -g3 -ggdb -fno-omit-frame-pointer -fno-optimize-sibling-calls -ggnu-pubnames -gsplit-dwarf -ftime-trace")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't want to bikeshed but would -Og be more appropriate here? I don't think it's a big deal either way; -O2 seems to be the default level for RelWithDebInfo anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't want to bikeshed but would -Og be more appropriate here?

Probably yes.

set(GAIA_LINK_FLAGS "-Wl,--gdb-index")
endif()

# Sanitizer-specific compiler/linker flags.
if(DEFINED CACHE{SANITIZER})
message(VERBOSE "Sanitizer=$CACHE{SANITIZER}")
Expand Down