You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2023/06/20 20:30:37 UTC

[arrow] branch main updated: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG (#35924)

This is an automated email from the ASF dual-hosted git repository.

kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new 72343f5399 GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG (#35924)
72343f5399 is described below

commit 72343f53992fe6fd213810fd20f170fca0493ec2
Author: Sutou Kouhei <ko...@clear-code.com>
AuthorDate: Wed Jun 21 05:30:26 2023 +0900

    GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG (#35924)
    
    ### Rationale for this change
    
    `-DCMAKE_CXX_FLAGS_RELEASE=-O...` works with #15022 but `-DCMAKE_CXX_FLAGS_DEBUG=-O...` and
    `-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O...` don't work. Because we always specify `-O0` for Debug and RelWithDebInfo.
    
    ### What changes are included in this PR?
    
    These changes don't specify optimization levels for Debug and RelWithDebInfo unconditionally. We don't specify optimization levels for RelWithDebInfo the default flags include them. We specify `-O0` for Debug only when the default flags don't include any optimization levels. So user specified `CMAKE_CXX_FLAGS_DEBUG=-O...` and `CMAKE_CXX_FLAGS_RELWITHDEBINFO=-O...` are used.
    
    In addition, `ARROW_C_FLAGS_${CONFIG}` and `ARROW_CXX_FLAGS_${CONFIG}` are introduced. Users can also use `-DARROW_CXX_FLAGS_DEBUG=-O...` instead of `-DCMAKE_CXX_FLAGS_DEBUG=-O...`.
    
    ### Are these changes tested?
    
    Yes.
    
    ### Are there any user-facing changes?
    
    Yes.
    * Closes: #35870
    
    Lead-authored-by: Sutou Kouhei <ko...@clear-code.com>
    Co-authored-by: Sutou Kouhei <ko...@cozmixng.org>
    Co-authored-by: Antoine Pitrou <pi...@free.fr>
    Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
 ci/scripts/cpp_build.sh                 |  6 ++++
 cpp/cmake_modules/SetupCxxFlags.cmake   | 50 ++++++++++++++++++++++-----------
 docs/source/developers/cpp/building.rst | 28 ++++++++++++++++++
 3 files changed, 67 insertions(+), 17 deletions(-)

diff --git a/ci/scripts/cpp_build.sh b/ci/scripts/cpp_build.sh
index 4c9f4aac13..b2dd1631cc 100755
--- a/ci/scripts/cpp_build.sh
+++ b/ci/scripts/cpp_build.sh
@@ -84,6 +84,12 @@ cmake \
   -DARROW_CSV=${ARROW_CSV:-ON} \
   -DARROW_CUDA=${ARROW_CUDA:-OFF} \
   -DARROW_CXXFLAGS=${ARROW_CXXFLAGS:-} \
+  -DARROW_CXX_FLAGS_DEBUG=${ARROW_CXX_FLAGS_DEBUG:-} \
+  -DARROW_CXX_FLAGS_RELEASE=${ARROW_CXX_FLAGS_RELEASE:-} \
+  -DARROW_CXX_FLAGS_RELWITHDEBINFO=${ARROW_CXX_FLAGS_RELWITHDEBINFO:-} \
+  -DARROW_C_FLAGS_DEBUG=${ARROW_C_FLAGS_DEBUG:-} \
+  -DARROW_C_FLAGS_RELEASE=${ARROW_C_FLAGS_RELEASE:-} \
+  -DARROW_C_FLAGS_RELWITHDEBINFO=${ARROW_C_FLAGS_RELWITHDEBINFO:-} \
   -DARROW_DATASET=${ARROW_DATASET:-ON} \
   -DARROW_DEPENDENCY_SOURCE=${ARROW_DEPENDENCY_SOURCE:-AUTO} \
   -DARROW_ENABLE_TIMING_TESTS=${ARROW_ENABLE_TIMING_TESTS:-ON} \
diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
index 34c177d213..03124e1b38 100644
--- a/cpp/cmake_modules/SetupCxxFlags.cmake
+++ b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -623,29 +623,45 @@ if(NOT MSVC)
   if(CMAKE_CXX_FLAGS_RELEASE MATCHES "-O3")
     string(APPEND CXX_RELEASE_FLAGS " -O2")
   endif()
+  set(C_RELWITHDEBINFO_FLAGS "")
+  if(CMAKE_C_FLAGS_RELWITHDEBINFO MATCHES "-O3")
+    string(APPEND C_RELWITHDEBINFO_FLAGS " -O2")
+  endif()
+  set(CXX_RELWITHDEBINFO_FLAGS "")
+  if(CMAKE_CXX_FLAGS_RELWITHDEBINFO MATCHES "-O3")
+    string(APPEND CXX_RELWITHDEBINFO_FLAGS " -O2")
+  endif()
   if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
     string(APPEND C_RELEASE_FLAGS " -ftree-vectorize")
     string(APPEND CXX_RELEASE_FLAGS " -ftree-vectorize")
+    string(APPEND C_RELWITHDEBINFO_FLAGS " -ftree-vectorize")
+    string(APPEND CXX_RELWITHDEBINFO_FLAGS " -ftree-vectorize")
   endif()
-
-  set(DEBUG_FLAGS "")
-  if(MSVC)
-    string(APPEND DEBUG_FLAGS " /Od")
-  else()
-    string(APPEND DEBUG_FLAGS " -O0")
-  endif()
-  if(ARROW_GGDB_DEBUG)
-    string(APPEND DEBUG_FLAGS " -ggdb")
+  set(C_DEBUG_FLAGS "")
+  set(CXX_DEBUG_FLAGS "")
+  if(NOT MSVC)
+    if(NOT CMAKE_C_FLAGS_DEBUG MATCHES "-O")
+      string(APPEND C_DEBUG_FLAGS " -O0")
+    endif()
+    if(NOT CMAKE_CXX_FLAGS_DEBUG MATCHES "-O")
+      string(APPEND CXX_DEBUG_FLAGS " -O0")
+    endif()
+    if(ARROW_GGDB_DEBUG)
+      string(APPEND C_DEBUG_FLAGS " -ggdb")
+      string(APPEND CXX_DEBUG_FLAGS " -ggdb")
+      string(APPEND C_RELWITHDEBINFO_FLAGS " -ggdb")
+      string(APPEND CXX_RELWITHDEBINFO_FLAGS " -ggdb")
+    endif()
   endif()
 
-  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}")
-  # We must put release flags after debug flags to use optimization
-  # flags in release flags. RelWithDebInfo must enable optimization.
-  string(APPEND CMAKE_C_FLAGS_RELWITHDEBINFO "${DEBUG_FLAGS} ${C_RELEASE_FLAGS}")
-  string(APPEND CMAKE_CXX_FLAGS_RELWITHDEBINFO "${DEBUG_FLAGS} ${CXX_RELEASE_FLAGS}")
+  string(APPEND CMAKE_C_FLAGS_RELEASE "${C_RELEASE_FLAGS} ${ARROW_C_FLAGS_RELEASE}")
+  string(APPEND CMAKE_CXX_FLAGS_RELEASE "${CXX_RELEASE_FLAGS} ${ARROW_CXX_FLAGS_RELEASE}")
+  string(APPEND CMAKE_C_FLAGS_DEBUG "${C_DEBUG_FLAGS} ${ARROW_C_FLAGS_DEBUG}")
+  string(APPEND CMAKE_CXX_FLAGS_DEBUG "${CXX_DEBUG_FLAGS} ${ARROW_CXX_FLAGS_DEBUG}")
+  string(APPEND CMAKE_C_FLAGS_RELWITHDEBINFO
+         "${C_RELWITHDEBINFO_FLAGS} ${ARROW_C_FLAGS_RELWITHDEBINFO}")
+  string(APPEND CMAKE_CXX_FLAGS_RELWITHDEBINFO
+         "${CXX_RELWITHDEBINFO_FLAGS} ${ARROW_CXX_FLAGS_RELWITHDEBINFO}")
 endif()
 
 message(STATUS "Build Type: ${CMAKE_BUILD_TYPE}")
diff --git a/docs/source/developers/cpp/building.rst b/docs/source/developers/cpp/building.rst
index 909c291b40..aa5dbe2086 100644
--- a/docs/source/developers/cpp/building.rst
+++ b/docs/source/developers/cpp/building.rst
@@ -254,6 +254,34 @@ Several build types are possible:
 * ``Release``: applies compiler optimizations and removes debug information
   from the binary.
 
+.. note::
+
+   These build types provide suitable optimization/debug flags by
+   default but you can change them by specifying
+   ``-DARROW_C_FLAGS_${BUILD_TYPE}=...`` and/or
+   ``-DARROW_CXX_FLAGS_${BUILD_TYPE}=...``. ``${BUILD_TYPE}`` is upper
+   case of build type. For example, ``DEBUG``
+   (``-DARROW_C_FLAGS_DEBUG=...`` / ``-DARROW_CXX_FLAGS_DEBUG=...``) for the
+   ``Debug`` build type and ``RELWITHDEBINFO``
+   (``-DARROW_C_FLAGS_RELWITHDEBINFO=...`` /
+   ``-DARROW_CXX_FLAGS_RELWITHDEBINFO=...``) for the ``RelWithDebInfo``
+   build type.
+
+   For example, you can use ``-O3`` as an optimization flag for the ``Release``
+   build type by passing ``-DARROW_CXX_FLAGS_RELEASE=-O3`` .
+   You can use ``-g3`` as a debug flag for the ``Debug`` build type
+   by passing ``-DARROW_CXX_FLAGS_DEBUG=-g3`` .
+
+   You can also use the standard ``CMAKE_C_FLAGS_${BUILD_TYPE}``
+   and ``CMAKE_CXX_FLAGS_${BUILD_TYPE}`` variables but
+   the ``ARROW_C_FLAGS_${BUILD_TYPE}`` and
+   ``ARROW_CXX_FLAGS_${BUILD_TYPE}`` variables are
+   recommended. The ``CMAKE_C_FLAGS_${BUILD_TYPE}`` and
+   ``CMAKE_CXX_FLAGS_${BUILD_TYPE}`` variables replace all default
+   flags provided by CMake, while ``ARROW_C_FLAGS_${BUILD_TYPE}`` and
+   ``ARROW_CXX_FLAGS_${BUILD_TYPE}`` just append the
+   flags specified, which allows selectively overriding some of the defaults.
+
 You can also run default build with flag ``-DARROW_EXTRA_ERROR_CONTEXT=ON``, see
 :ref:`cpp-extra-debugging`.