Add increased inline threshold (iff Clang) to exported Ceres target. - When compiled with Clang, Ceres and all of the examples are compiled with an increased inlining-threshold, as the default value can result in poor Eigen performance. - Previously, client code using Ceres would typically not use an increased inlining-threshold (unless the user has specifically added it themselves). However, increasing the inlining threshold can result in significant performance improvements in auto-diffed CostFunctions. - This patch adds the inlining-threshold flags to the interface flags for the Ceres CMake target s/t any client code using Ceres (via CMake), and compiled with Clang, will now be compiled with the same increased inlining threshold as used by Ceres itself. Change-Id: I31e8f1abfda140d22e85bb48aa57f028a68a415e
diff --git a/CMakeLists.txt b/CMakeLists.txt index 20e7cac..af8f341 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt
@@ -683,7 +683,8 @@ # threshold to the linker and clang complains about it and dies. if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") set(CMAKE_CXX_FLAGS - "${CMAKE_CXX_FLAGS} -Qunused-arguments -mllvm -inline-threshold=600") + "${CMAKE_CXX_FLAGS} -Qunused-arguments -mllvm -inline-threshold=600") + # Older versions of Clang (<= 2.9) do not support the 'return-type-c-linkage' # option, so check for its presence before adding it to the default flags set. include(CheckCXXCompilerFlag)
diff --git a/cmake/CeresConfig.cmake.in b/cmake/CeresConfig.cmake.in index bcda9a7..9468279 100644 --- a/cmake/CeresConfig.cmake.in +++ b/cmake/CeresConfig.cmake.in
@@ -258,6 +258,36 @@ # Set the expected XX_LIBRARIES variable for FindPackage(). set(CERES_LIBRARIES ceres) +# Make user aware of any compile flags that will be added to their targets +# which use Ceres (i.e. flags exported in the Ceres target). Only CMake +# versions >= 2.8.12 support target_compile_options(). +if (TARGET ${CERES_LIBRARIES} AND + NOT CMAKE_VERSION VERSION_LESS "2.8.12") + get_target_property(CERES_INTERFACE_COMPILE_OPTIONS + ${CERES_LIBRARIES} INTERFACE_COMPILE_OPTIONS) + + if (CERES_WAS_INSTALLED) + set(CERES_LOCATION "${CURRENT_ROOT_INSTALL_DIR}") + else() + set(CERES_LOCATION "${CERES_EXPORTED_BUILD_DIR}") + endif() + + # Check for -std=c++11 flags. + if (CERES_INTERFACE_COMPILE_OPTIONS MATCHES ".*std=c\\+\\+11.*") + message(STATUS "Ceres version ${CERES_VERSION} detected here: " + "${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. set(CERES_INCLUDES ${CERES_INCLUDE_DIRS})
diff --git a/docs/source/faqs.rst b/docs/source/faqs.rst index a4737d6..f6dbdbb 100644 --- a/docs/source/faqs.rst +++ b/docs/source/faqs.rst
@@ -33,6 +33,34 @@ NDK. It has worse performance than the full fledged glog library and is much harder to control and use. +#. Increase the inlining threshold in Clang for Eigen. + + By default, the inlining threshold (evaluated on a heuristic used by LLVM to + guess how expensive a function will be to inline) can hobble Eigen, resulting in + 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: + +.. code-block:: cmake + + # 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). + if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + set(CMAKE_CXX_FLAGS + "${CMAKE_CXX_FLAGS} -Qunused-arguments -mllvm -inline-threshold=600") + endif() Modeling ========
diff --git a/internal/ceres/CMakeLists.txt b/internal/ceres/CMakeLists.txt index 2ae05b7..49f4de3 100644 --- a/internal/ceres/CMakeLists.txt +++ b/internal/ceres/CMakeLists.txt
@@ -183,6 +183,7 @@ set_target_properties(ceres PROPERTIES VERSION ${CERES_VERSION} SOVERSION ${CERES_VERSION_MAJOR}) + # Always build position-independent code (PIC), even when building Ceres as a # static library so that shared libraries can link against it, not just # executables (PIC does not apply on Windows). @@ -197,23 +198,60 @@ endif() endif() -if (CXX11 AND COMPILER_HAS_CXX11_FLAG) - if (CMAKE_VERSION VERSION_LESS "2.8.12") - message("-- Warning: detected CMake version: ${CMAKE_VERSION} < 2.8.12, " +if (CMAKE_VERSION VERSION_LESS "2.8.12") + # CMake version < 2.8.12 does not support target_compile_options(), warn + # user that they will have to add compile flags to their own projects + # manually if required. + if (CXX11 AND COMPILER_HAS_CXX11_FLAG) + 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, but CERES_USE_CXX11 is enabled and the detected. " - "compiler requires -std=c++11. The client is responsible for adding " + "compiler requires -std=c++11. The client is responsible for adding " "-std=c++11 when using Ceres.") - else () + 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) # If Ceres is compiled using C++11, and the compiler requires -std=c++11 # to be set, then ensure that this requirement is rolled into the exported - # CMake target, s/t client code which uses Ceres will inherit it (if the - # CMake version supports it), iff they are NOT compiling for C. We check - # for not C, rather than C++ as LINKER_LANGUAGE is often NOTFOUND and then - # uses the default (C++). + # CMake target, such that client code which uses Ceres will inherit it (if + # the CMake version supports it), iff they are NOT compiling for C. We + # check for not C, rather than C++ as LINKER_LANGUAGE is often NOTFOUND and + # then uses the default (C++). 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)