Skip to content

Commit

Permalink
ARROW-18231: [C++][CMake] Add support for overriding optimization lev…
Browse files Browse the repository at this point in the history 
…el (#15022)

We can overriding optimization level by
`-DCMAKE_CXX_FLAGS_${CONFIG}=-OX` such as
`-DCMAKE_CXX_FLAGS_RELEASE=-O0` with this change.

Note that we can't use CXXFLAGS environment variable because CMake uses
`${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CONFIG}}` order and optimization flags are specified
in `CMAKE_CXX_FLAGS_${CONFIG}` not `CMAKE_CXX_FLAGS`. (CMAKE_CXX_FLAGS CMake variable is
initialized by CXXFLAGS environment variable.)

See also: https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS.html

> The flags in this variable will be passed to the compiler before
> those in the per-configuration `CMAKE_<LANG>_FLAGS_<CONFIG>` variant,
> and before flags added by the `add_compile_options()` or
> `target_compile_options()` commands.

This change also cleans up the followings:

* C/CXX flags related codes
  * Use `CMAKE_C_FLAGS_${BUILD_TYPE}`/`CMAKE_CXX_FLAGS_${BUILD_TYPE}` instead of putting all flags to `CMAKE_C_FLAGS`/`CMAKE_CXX_FLAGS`
* `externalproject_add()` related codes
  * Extract common options as `EP_XXX`
* Use `string(APPEND)`/`list(APPEND)` as much as possible instead of `set(XXX "${XXX} ...")`/`set(XXX ${XXX} ...)`

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
@kou
kou authored Dec 24, 2022
1 parent 5feabc5 commit 474b1a5
Show file tree
Hide file tree
Showing 7 changed files with 236 additions and 279 deletions.
4 changes: 4 additions & 0 deletions cpp/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,10 @@ endif()
# CMAKE_CXX_FLAGS now fully assembled
message(STATUS "CMAKE_C_FLAGS: ${CMAKE_C_FLAGS}")
message(STATUS "CMAKE_CXX_FLAGS: ${CMAKE_CXX_FLAGS}")
message(STATUS "CMAKE_C_FLAGS_${UPPERCASE_BUILD_TYPE}: ${CMAKE_C_FLAGS_${UPPERCASE_BUILD_TYPE}}"
)
message(STATUS "CMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}: ${CMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}}"
)

include_directories(${CMAKE_CURRENT_BINARY_DIR}/src)
include_directories(src)
Expand Down
96 changes: 31 additions & 65 deletions cpp/cmake_modules/SetupCxxFlags.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,13 +283,13 @@ string(TOUPPER ${BUILD_WARNING_LEVEL} BUILD_WARNING_LEVEL)
message(STATUS "Arrow build warning level: ${BUILD_WARNING_LEVEL}")

macro(arrow_add_werror_if_debug)
if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
# Treat all compiler warnings as errors
if(MSVC)
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /WX")
else()
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Werror")
endif()
# Treat all compiler warnings as errors
if(MSVC)
string(APPEND CMAKE_C_FLAGS_DEBUG " /WX")
string(APPEND CMAKE_CXX_FLAGS_DEBUG " /WX")
else()
string(APPEND CMAKE_C_FLAGS_DEBUG " -Werror")
string(APPEND CMAKE_CXX_FLAGS_DEBUG " -Werror")
endif()
endmacro()

Expand Down Expand Up @@ -604,76 +604,42 @@ endif()
# For all builds:
# For CMAKE_BUILD_TYPE=Debug
# -ggdb: Enable gdb debugging
# For CMAKE_BUILD_TYPE=FastDebug
# Same as Debug, except with some optimizations on.
# For CMAKE_BUILD_TYPE=Release
# -O2: Enable all compiler optimizations
# -O2 (not -O3): Enable compiler optimizations
# Debug symbols are stripped for reduced binary size.
# For CMAKE_BUILD_TYPE=RelWithDebInfo
# Same as Release, except with debug symbols enabled.

if(NOT MSVC)
string(REPLACE "-O3" "" CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}")
string(REPLACE "-O3" "" CMAKE_CXX_FLAGS_RELWITHDEBINFO
"${CMAKE_CXX_FLAGS_RELWITHDEBINFO}")

set(RELEASE_FLAGS "-O2 -DNDEBUG")
set(C_RELEASE_FLAGS "")
if(CMAKE_C_FLAGS_RELEASE MATCHES "-O3")
string(APPEND C_RELEASE_FLAGS " -O2")
endif()
set(CXX_RELEASE_FLAGS "")
if(CMAKE_CXX_FLAGS_RELEASE MATCHES "-O3")
string(APPEND CXX_RELEASE_FLAGS " -O2")
endif()
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
set(RELEASE_FLAGS "${RELEASE_FLAGS} -ftree-vectorize")
string(APPEND C_RELEASE_FLAGS " -ftree-vectorize")
string(APPEND CXX_RELEASE_FLAGS " -ftree-vectorize")
endif()

if(ARROW_GGDB_DEBUG)
set(ARROW_DEBUG_SYMBOL_TYPE "gdb")
set(C_FLAGS_DEBUG "-g${ARROW_DEBUG_SYMBOL_TYPE} -O0")
set(C_FLAGS_FASTDEBUG "-g${ARROW_DEBUG_SYMBOL_TYPE} -O1")
set(C_FLAGS_RELWITHDEBINFO "-g${ARROW_DEBUG_SYMBOL_TYPE} ${RELEASE_FLAGS}")
set(CXX_FLAGS_DEBUG "-g${ARROW_DEBUG_SYMBOL_TYPE} -O0")
set(CXX_FLAGS_FASTDEBUG "-g${ARROW_DEBUG_SYMBOL_TYPE} -O1")
set(CXX_FLAGS_RELWITHDEBINFO "-g${ARROW_DEBUG_SYMBOL_TYPE} ${RELEASE_FLAGS}")
set(DEBUG_FLAGS "")
if(MSVC)
string(APPEND DEBUG_FLAGS " /Od")
else()
set(C_FLAGS_DEBUG "-g -O0")
set(C_FLAGS_FASTDEBUG "-g -O1")
set(C_FLAGS_RELWITHDEBINFO "-g ${RELEASE_FLAGS}")
set(CXX_FLAGS_DEBUG "-g -O0")
set(CXX_FLAGS_FASTDEBUG "-g -O1")
set(CXX_FLAGS_RELWITHDEBINFO "-g ${RELEASE_FLAGS}")
string(APPEND DEBUG_FLAGS " -O0")
endif()
if(ARROW_GGDB_DEBUG)
string(APPEND DEBUG_FLAGS " -ggdb")
endif()

set(C_FLAGS_RELEASE "${RELEASE_FLAGS}")
set(CXX_FLAGS_RELEASE "${RELEASE_FLAGS}")
endif()

set(C_FLAGS_PROFILE_GEN "${CXX_FLAGS_RELEASE} -fprofile-generate")
set(C_FLAGS_PROFILE_BUILD "${CXX_FLAGS_RELEASE} -fprofile-use")
set(CXX_FLAGS_PROFILE_GEN "${CXX_FLAGS_RELEASE} -fprofile-generate")
set(CXX_FLAGS_PROFILE_BUILD "${CXX_FLAGS_RELEASE} -fprofile-use")

# Set compile flags based on the build type.
message(STATUS "Configured for ${CMAKE_BUILD_TYPE} build (set with cmake -DCMAKE_BUILD_TYPE={release,debug,...})"
)
if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_FLAGS_DEBUG}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_FLAGS_DEBUG}")
elseif("${CMAKE_BUILD_TYPE}" STREQUAL "RELWITHDEBINFO")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_FLAGS_RELWITHDEBINFO}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_FLAGS_RELWITHDEBINFO}")
elseif("${CMAKE_BUILD_TYPE}" STREQUAL "FASTDEBUG")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_FLAGS_FASTDEBUG}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_FLAGS_FASTDEBUG}")
elseif("${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_FLAGS_RELEASE}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_FLAGS_RELEASE}")
elseif("${CMAKE_BUILD_TYPE}" STREQUAL "PROFILE_GEN")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_FLAGS_PROFILE_GEN}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_FLAGS_PROFILE_GEN}")
elseif("${CMAKE_BUILD_TYPE}" STREQUAL "PROFILE_BUILD")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_FLAGS_PROFILE_BUILD}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_FLAGS_PROFILE_BUILD}")
elseif("${CMAKE_BUILD_TYPE}" STREQUAL "MINSIZEREL")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_FLAGS_MINSIZEREL}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CXX_FLAGS_MINSIZEREL}")
else()
message(FATAL_ERROR "Unknown build type: ${CMAKE_BUILD_TYPE}")
string(APPEND CMAKE_C_FLAGS_RELEASE "${C_RELEASE_FLAGS}")
string(APPEND CMAKE_CXX_FLAGS_RELEASE "${CXX_RELEASE_FLAGS}")
string(APPEND CMAKE_C_FLAGS_DEBUG "${DEBUG_FLAGS}")
string(APPEND CMAKE_CXX_FLAGS_DEBUG "${DEBUG_FLAGS}")
string(APPEND CMAKE_C_FLAGS_RELWITHDEBINFO "${C_RELEASE_FLAGS} ${DEBUG_FLAGS}")
string(APPEND CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CXX_RELEASE_FLAGS} ${DEBUG_FLAGS}")
endif()

message(STATUS "Build Type: ${CMAKE_BUILD_TYPE}")
Expand Down
Loading

0 comments on commit 474b1a5

Please sign in to comment.