Revert increased inline threshold (iff Clang) to exported Ceres target. - Increasing the inline threshold results in very variable performance improvements, and could potentially confuse users if they are trying to set the inline threshold themselves. - As such, we no longer export our inline threshold configuration for Clang, but instead document how to change it in the FAQs. Change-Id: I88e2e0001e4586ba2718535845ed1e4b1a5b72bc
diff --git a/cmake/CeresConfig.cmake.in b/cmake/CeresConfig.cmake.in index 9468279..9e29d7a 100644 --- a/cmake/CeresConfig.cmake.in +++ b/cmake/CeresConfig.cmake.in
@@ -278,14 +278,6 @@ "${CERES_LOCATION} was built with C++11. Ceres target will add " "C++11 flags to compile options for targets using it.") endif() - # Check for -inline-threshold Clang-specific flags. - if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND - CERES_INTERFACE_COMPILE_OPTIONS MATCHES ".*inline\\-threshold.*") - message(STATUS "Ceres version ${CERES_VERSION} detected here: " - "${CERES_LOCATION} will add flags to increase inline threshold " - "to compile options for targets using it and are compiled with " - "Clang (improves Eigen performance).") - endif() endif() # Set legacy include directories variable for backwards compatibility.
diff --git a/docs/source/faqs.rst b/docs/source/faqs.rst index 81bd9b7..badf90a 100644 --- a/docs/source/faqs.rst +++ b/docs/source/faqs.rst
@@ -40,19 +40,29 @@ poor performance. Ceres itself will always be compiled with an increased inline threshold - (currently 600) when compiled with Clang. This increased threshold is also - added to the interface flags for the exported Ceres CMake target provided - that the CMake version is >= 2.8.12 (from which this was supported). This - means that **any user code that links against Ceres using CMake >= 2.8.12 - (and is compiled with Clang, irrespective of what Ceres was compiled with) - will automatically be compiled with the same increased inlining threshold - used to compile Ceres**. - - If you are using CMake < 2.8.12 and Clang in your own code which uses Ceres - we recommend that you increase the inlining threshold yourself using: + (currently 600) when compiled with Clang. To experiment with this in your + own code, you can use the following: .. code-block:: cmake + # If using CMake >= 2.8.12: + # We recommend you use target_compile_options() to add the inlining flags + # to specific targets for fine grained control: + # + # Use a larger inlining threshold for Clang, since it can hobble Eigen, + # resulting in reduced performance. The -Qunused-arguments is needed because + # CMake passes the inline threshold to the linker and clang complains about + # it (and dies, if compiling with -Werror). + target_compile_options(<YOUR_TARGET_NAME> PUBLIC + $<$<CXX_COMPILER_ID:Clang>:-Qunused-arguments -mllvm -inline-threshold=600>) + +.. code-block:: cmake + + # If using CMake < 2.8.12: + # On CMake < 2.8.12 target_compile_options() is not available, so you + # cannot add the flags only on a per-target level and must instead set them + # for all targets declared after the flags are updated: + # # Use a larger inlining threshold for Clang, since it can hobble Eigen, # resulting in reduced performance. The -Qunused-arguments is needed because # CMake passes the inline threshold to the linker and clang complains about
diff --git a/internal/ceres/CMakeLists.txt b/internal/ceres/CMakeLists.txt index 49f4de3..55643de 100644 --- a/internal/ceres/CMakeLists.txt +++ b/internal/ceres/CMakeLists.txt
@@ -209,13 +209,6 @@ "compiler requires -std=c++11. The client is responsible for adding " "-std=c++11 when using Ceres.") endif() - - message("-- Warning: Detected CMake version: ${CMAKE_VERSION} < 2.8.12, " - "which is the minimum required for compile options to be included in an " - "exported CMake target, and the detected compiler is Clang. Cannot add " - "increased -inline-threshold compile flag to exported Ceres target, but " - "this is recommended to improve Eigen's performance on Clang. You will " - "have to add this manually to targets using Ceres if desired.") else() # CMake version >= 2.8.12 supports target_compile_options(). if (CXX11 AND COMPILER_HAS_CXX11_FLAG) @@ -228,30 +221,6 @@ target_compile_options(ceres PUBLIC $<$<NOT:$<STREQUAL:$<TARGET_PROPERTY:LINKER_LANGUAGE>,C>>:-std=c++11>) endif() - - # Export the use of a larger inlining threshold for Clang into the - # exported Ceres target. The default setting for Clang hobbles Eigen and - # increasing this can result in very significant performance improvements, - # particularly in auto-diffed CostFunctions. Exporting this setting into - # the Ceres target means any user code linking against Ceres (via CMake) - # will use it. The -Qunused-arguments is needed because CMake passes the - # inline threshold to the linker and clang complains about it and dies. - # - # We use a generator expression such that irrespective of whether Ceres was - # compiled using Clang, if user code using Ceres is compiled with Clang - # it will still inherit the flags (e.g. if Ceres compiled with GCC to get - # OpenMP, but user code compiled with Clang). - # - # We use INTERFACE (not PUBLIC) here, and add the same flags to - # CMAKE_CXX_FLAGS in the root Ceres CMakeLists such that even if - # CMake < 2.8.12 is being used to compile Ceres with Clang, we will compile - # Ceres, and all of the examples with the increased inline threshold. - message("-- Adding compile flags to increase inline threshold to exported " - "Ceres CMake target (improves Eigen performance). These will *only* be " - "used if Clang is used to compile client code linking against Ceres (using " - "CMake). They are not required, and will not be used, for GCC or MSVC.") - target_compile_options(ceres INTERFACE - $<$<CXX_COMPILER_ID:Clang>:-Qunused-arguments -mllvm -inline-threshold=600>) endif() if (BUILD_SHARED_LIBS)