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

CMakeLists.txt

@@ -683,7 +683,8 @@ endif (UNIX)
 # 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)

cmake/CeresConfig.cmake.in

@@ -258,6 +258,36 @@ endif (NOT TARGET ceres AND NOT Ceres_BINARY_DIR)
 # 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})
 

docs/source/faqs.rst

@@ -33,6 +33,34 @@ Building
    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
 ========

internal/ceres/CMakeLists.txt

@@ -183,6 +183,7 @@ add_library(ceres ${CERES_LIBRARY_SOURCE})
 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 @@ if (NOT WIN32 AND NOT BUILD_SHARED_LIBS)
   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)