kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [kudu] 01/04: build: restrict clang version, prefer lld, enable thinlto
Date Wed, 22 Jan 2020 19:24:26 GMT
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 638d95d295da0bddf3fa157792ec7de8889ac919
Author: Todd Lipcon <todd@apache.org>
AuthorDate: Thu Jan 16 22:29:29 2020 -0800

    build: restrict clang version, prefer lld, enable thinlto
    
    * Restricts clang to at least version 6.0, which gets rid of a couple
      older special cases.
    
    * We now loop through linkers trying to prefer lld where available,
      either using the version provided by the compiler with -fuse-lld or
      explicitly passing the path to the toolchain lld. lld should be faster
      than gold and GNU ld, and also has the advantage of being a known
      quantity within our toolchain.
    
      This fixes ASAN builds on my Ubuntu 18 machine, where the version of
      gold that ships with the system can't seem to link ASAN executables.
    
    * Switches the LTO build to use ThinLTO, which provides most of the
      performance benefits but at much lower cost.
    
    Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0
    Reviewed-on: http://gerrit.cloudera.org:8080/15058
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <adar@cloudera.com>
---
 CMakeLists.txt                 | 157 ++++++++++-------------------------------
 cmake_modules/KuduLinker.cmake | 154 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 191 insertions(+), 120 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0b355c1..8df04a8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -82,6 +82,7 @@ include(CMakeParseArguments)
 
 set(BUILD_SUPPORT_DIR ${CMAKE_CURRENT_SOURCE_DIR}/build-support)
 set(THIRDPARTY_DIR ${CMAKE_CURRENT_SOURCE_DIR}/thirdparty)
+set(THIRDPARTY_TOOLCHAIN_DIR ${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/clang-toolchain)
 set(THIRDPARTY_INSTALL_DIR ${THIRDPARTY_DIR}/installed)
 set(THIRDPARTY_INSTALL_COMMON_DIR ${THIRDPARTY_INSTALL_DIR}/common)
 set(THIRDPARTY_INSTALL_UNINSTRUMENTED_DIR ${THIRDPARTY_INSTALL_DIR}/uninstrumented)
@@ -122,6 +123,10 @@ endif()
 # Compiler flags
 ############################################################
 
+# Determine compiler version
+include(CompilerInfo)
+
+
 # compiler flags that are common across debug/release builds
 #  -msse4.2: Enable sse4.2 compiler intrinsics.
 execute_process(COMMAND uname -p OUTPUT_VARIABLE ARCH_NAME)
@@ -185,12 +190,6 @@ set(CXX_FLAGS_DEBUG "-ggdb")
 set(CXX_FLAGS_FASTDEBUG "-ggdb -O1 -fno-omit-frame-pointer")
 set(CXX_FLAGS_RELEASE "-O3 -g -DNDEBUG -fno-omit-frame-pointer")
 
-if (NOT "${KUDU_USE_LTO}" STREQUAL "")
-  set(CXX_FLAGS_RELEASE "${CXX_FLAGS_RELEASE} -flto -fuse-ld=lld")
-  set(CMAKE_AR "llvm-ar")
-  set(CMAKE_RANLIB "llvm-ranlib")
-endif()
-
 set(CXX_FLAGS_PROFILE_GEN "${CXX_FLAGS_RELEASE} -fprofile-generate")
 set(CXX_FLAGS_PROFILE_BUILD "${CXX_FLAGS_RELEASE} -fprofile-use")
 
@@ -227,10 +226,17 @@ endif ()
 # Add common flags
 set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
 
-# Determine compiler version
-include(CompilerInfo)
-
 if ("${COMPILER_FAMILY}" STREQUAL "clang")
+  # Ensure that we don't use old versions of clang. There isn't anything
+  # particularly wrong with those, but no reason to support going back
+  # this far.
+  if ("${COMPILER_VERSION}" VERSION_LESS "6.0")
+    message(FATAL_ERROR "Clang version ${COMPILER_VERSION} too old. "
+      "Consider using the version of clang in ${THIRDPARTY_TOOLCHAIN_DIR}:\n"
+      ""
+      "  CC=${BUILD_SUPPORT_DIR}/ccache-clang/clang CXX=$CC++ cmake <args>")
+  endif()
+
   # Using Clang with ccache causes a bunch of spurious warnings that are
   # purportedly fixed in the next version of ccache. See the following for details:
   #
@@ -320,18 +326,6 @@ if (${KUDU_USE_ASAN})
     message(SEND_ERROR "Cannot use ASAN without clang or gcc >= 4.8")
   endif()
 
-  # If UBSAN is also enabled, and we're on clang < 3.5, ensure static linking is
-  # enabled. Otherwise, we run into https://llvm.org/bugs/show_bug.cgi?id=18211
-  if("${KUDU_USE_UBSAN}" AND
-      "${COMPILER_FAMILY}" STREQUAL "clang" AND
-      "${COMPILER_VERSION}" VERSION_LESS "3.5")
-    if("${KUDU_LINK}" STREQUAL "a")
-      message("Using static linking for ASAN+UBSAN build")
-      set(KUDU_LINK "s")
-    elseif("${KUDU_LINK}" STREQUAL "d")
-      message(SEND_ERROR "Cannot use dynamic linking when ASAN and UBSAN are both enabled")
-    endif()
-  endif()
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -DADDRESS_SANITIZER")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DLEAK_SANITIZER")
 endif()
@@ -400,23 +394,36 @@ if (${KUDU_USE_TSAN})
   endif()
 endif()
 
+include("KuduLinker")
+APPEND_LINKER_FLAGS()
 
 if ("${KUDU_USE_ASAN}" OR "${KUDU_USE_TSAN}" OR "${KUDU_USE_UBSAN}")
   # GCC 4.8 and 4.9 (latest as of this writing) don't allow you to specify a
   # sanitizer blacklist.
   if("${COMPILER_FAMILY}" STREQUAL "clang")
-    # Require clang 3.4 or newer; clang 3.3 has issues with TSAN and pthread
-    # symbol interception.
-    if("${COMPILER_VERSION}" VERSION_LESS "3.4")
-      message(SEND_ERROR "Must use clang 3.4 or newer to run a sanitizer build."
-        " Try using clang from thirdparty/")
-    endif()
     add_definitions("-fsanitize-blacklist=${BUILD_SUPPORT_DIR}/sanitize-blacklist.txt")
   else()
     message(WARNING "GCC does not support specifying a sanitizer blacklist. Known sanitizer
check failures will not be suppressed.")
   endif()
 endif()
 
+if (KUDU_USE_LTO)
+  if(NOT "${CMAKE_EXE_LINKER_FLAGS}" MATCHES "-fuse-ld=lld" OR
+     NOT "${COMPILER_FAMILY}" STREQUAL "clang")
+    message(FATAL_ERROR "must use clang and lld for LTO build")
+  endif()
+  if (NOT "${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE")
+    # Doesn't make much sense to do LTO on a non-release build -- it's slow
+    # and the only reason you'd want to do it is high performance.
+    message(FATAL_ERROR "LTO only supported for release builds")
+  endif()
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto=thin")
+  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--thinlto-cache-dir=${CMAKE_CURRENT_BINARY_DIR}/lto.cache")
+  set(CMAKE_AR "${THIRDPARTY_TOOLCHAIN_DIR}/bin/llvm-ar")
+  set(CMAKE_RANLIB "${THIRDPARTY_TOOLCHAIN_DIR}/bin/llvm-ranlib")
+endif()
+
+
 # Code coverage
 if ("${KUDU_GENERATE_COVERAGE}")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} --coverage -DCOVERAGE_BUILD")
@@ -442,93 +449,6 @@ if ("${KUDU_LINK}" STREQUAL "a")
   endif()
 endif()
 
-# Interrogates the linker version via the C++ compiler to determine whether
-# we're using the gold linker, and if so, extracts its version.
-#
-# If the gold linker is being used, sets GOLD_VERSION in the parent scope with
-# the extracted version.
-#
-# Any additional arguments are passed verbatim into the C++ compiler invocation.
-function(GET_GOLD_VERSION)
-  # The gold linker is only for ELF binaries, which macOS doesn't use.
-  if (APPLE)
-    return()
-  endif()
-
-  execute_process(COMMAND ${CMAKE_CXX_COMPILER} "-Wl,--version" ${ARGN}
-    ERROR_QUIET
-    OUTPUT_VARIABLE LINKER_OUTPUT)
-  # We're expecting LINKER_OUTPUT to look like one of these:
-  #   GNU gold (version 2.24) 1.11
-  #   GNU gold (GNU Binutils for Ubuntu 2.30) 1.15
-  if (LINKER_OUTPUT MATCHES "GNU gold")
-    string(REGEX MATCH "GNU gold \\([^\\)]*\\) (([0-9]+\\.?)+)" _ "${LINKER_OUTPUT}")
-    if (NOT CMAKE_MATCH_1)
-      message(SEND_ERROR "Could not extract GNU gold version. "
-        "Linker version output: ${LINKER_OUTPUT}")
-    endif()
-    set(GOLD_VERSION "${CMAKE_MATCH_1}" PARENT_SCOPE)
-  endif()
-endfunction()
-
-# Is the compiler hard-wired to use the gold linker?
-GET_GOLD_VERSION()
-if (GOLD_VERSION)
-  set(MUST_USE_GOLD 1)
-else()
-  # Can the compiler optionally enable the gold linker?
-  GET_GOLD_VERSION("-fuse-ld=gold")
-
-  # We can't use the gold linker if it's inside devtoolset because the compiler
-  # won't find it when invoked directly from make/ninja (which is typically
-  # done outside devtoolset).
-  execute_process(COMMAND which ld.gold
-    OUTPUT_VARIABLE GOLD_LOCATION
-    OUTPUT_STRIP_TRAILING_WHITESPACE
-    ERROR_QUIET)
-  if ("${GOLD_LOCATION}" MATCHES "^/opt/rh/devtoolset")
-    message("Skipping optional gold linker (version ${GOLD_VERSION}) because "
-      "it's in devtoolset")
-    set(GOLD_VERSION)
-  endif()
-endif()
-if (GOLD_VERSION)
-  # Older versions of the gold linker are vulnerable to a bug [1] which
-  # prevents weak symbols from being overridden properly. This leads to
-  # omitting of Kudu's tcmalloc dependency if using dynamic linking.
-  #
-  # How we handle this situation depends on other factors:
-  # - If gold is optional, we won't use it.
-  # - If gold is required and we're using dynamic linking, we'll either:
-  #   - Raise an error in RELEASE builds (we shouldn't release such a product), or
-  #   - Drop tcmalloc in all other builds.
-  #
-  # 1. https://sourceware.org/bugzilla/show_bug.cgi?id=16979.
-  if ("${GOLD_VERSION}" VERSION_LESS "1.12")
-    set(KUDU_BUGGY_GOLD 1)
-  endif()
-  if (MUST_USE_GOLD)
-    message("Using hard-wired gold linker (version ${GOLD_VERSION})")
-    if (KUDU_BUGGY_GOLD AND "${KUDU_LINK}" STREQUAL "d")
-      if ("${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE")
-        message(SEND_ERROR "Configured to use buggy gold with dynamic linking "
-          "in a RELEASE build; this would cause tcmalloc symbols to get dropped")
-      endif()
-      message("Hard-wired gold linker is buggy, dropping tcmalloc dependency")
-      set(GOLD_INCOMPATIBLE_WITH_TCMALLOC 1)
-    endif()
-  elseif (NOT KUDU_BUGGY_GOLD)
-    # The Gold linker must be manually enabled.
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fuse-ld=gold")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fuse-ld=gold")
-    message("Using optional gold linker (version ${GOLD_VERSION})")
-  else()
-    message("Optional gold linker is buggy, using ld linker instead")
-  endif()
-else()
-  message("Using ld linker")
-endif()
-
 # Having set KUDU_LINK due to build type and/or sanitizer, it's now safe to
 # act on its value.
 if ("${KUDU_LINK}" STREQUAL "d")
@@ -1130,11 +1050,9 @@ endif()
 
 ## Google PerfTools
 ##
-## Disabled with TSAN/ASAN as well as with gold+dynamic linking (see
-## earlier comment).
+## Disabled with TSAN/ASAN.
 if (NOT "${KUDU_USE_ASAN}" AND
-    NOT "${KUDU_USE_TSAN}" AND
-    NOT "${GOLD_INCOMPATIBLE_WITH_TCMALLOC}")
+    NOT "${KUDU_USE_TSAN}")
   find_package(GPerf REQUIRED)
   ADD_THIRDPARTY_LIB(tcmalloc
     STATIC_LIB "${TCMALLOC_STATIC_LIB}"
@@ -1267,8 +1185,7 @@ if (APPLE)
   endif()
 else()
   if (KUDU_TCMALLOC_AVAILABLE AND
-      (("${COMPILER_FAMILY}" STREQUAL "clang" AND
-        "${COMPILER_VERSION}" VERSION_GREATER "3.7") OR
+      (("${COMPILER_FAMILY}" STREQUAL "clang") OR
        ("${COMPILER_FAMILY}" STREQUAL "gcc" AND
         "${COMPILER_VERSION}" VERSION_GREATER "5.0")))
     set(COMPILER_SUPPORTS_SIZED_DEALLOCATION TRUE)
diff --git a/cmake_modules/KuduLinker.cmake b/cmake_modules/KuduLinker.cmake
new file mode 100644
index 0000000..f2b2ac4
--- /dev/null
+++ b/cmake_modules/KuduLinker.cmake
@@ -0,0 +1,154 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# The Gold linker must be manually enabled.
+function(APPEND_LINKER_FLAGS)
+  # Search candidate linkers in the order of decreasing speed and functionality.
+  # In particular, lld is the best choice since it's quite fast and supports
+  # ThinLTO, etc.
+  foreach(candidate_linker lld "${THIRDPARTY_TOOLCHAIN_DIR}/bin/ld.lld" gold default)
+    if(candidate_linker STREQUAL "default")
+      set(candidate_flag "")
+    else()
+      set(candidate_flag "-fuse-ld=${candidate_linker}")
+    endif()
+    GET_LINKER_VERSION("${candidate_flag}")
+    if(NOT LINKER_FOUND)
+      continue()
+    endif()
+
+    # Older versions of the gold linker are vulnerable to a bug [1] which
+    # prevents weak symbols from being overridden properly. This leads to
+    # omitting of Kudu's tcmalloc dependency if using dynamic linking.
+    #
+    # We'll skip gold in our list of candidate linkers if this bug is relevant.
+    #
+    # 1. https://sourceware.org/bugzilla/show_bug.cgi?id=16979.
+    if ("${LINKER_FAMILY}" STREQUAL "gold")
+      if("${LINKER_VERSION}" VERSION_LESS "1.12" AND
+         "${KUDU_LINK}" STREQUAL "d")
+        message(WARNING "Skipping gold <1.12 with dynamic linking.")
+        continue()
+      endif()
+
+      # We can't use the gold linker if it's inside devtoolset because the compiler
+      # won't find it when invoked directly from make/ninja (which is typically
+      # done outside devtoolset). This seems to be fixed in devtoolset-6 or later
+      # by having the gcc/clang binary search the devtoolset bin directory even
+      # if it's not on $PATH.
+      execute_process(COMMAND which ld.gold
+        OUTPUT_VARIABLE GOLD_LOCATION
+        OUTPUT_STRIP_TRAILING_WHITESPACE
+        ERROR_QUIET)
+
+        if ("${GOLD_LOCATION}" MATCHES "^/opt/rh/devtoolset-[3-5]/")
+          message(WARNING "Cannot use gold with devtoolset < 6, skipping...")
+          continue()
+        endif()
+    endif()
+
+    # LLD < 10.0.0 has a bug[1] which causes the __tsan_default_options (and similar)
+    # symbols to be defined as WEAK instead of DEFINED in the resulting binary. This
+    # causes our suppressions to not work properly, resulting in failed tests.
+    #
+    # [1] https://reviews.llvm.org/D63974
+    if ("${LINKER_FAMILY}" STREQUAL "lld" AND
+        "${LINKER_VERSION}" VERSION_LESS "10.0.0" AND
+        ("${KUDU_USE_TSAN}" OR "${KUDU_USE_ASAN}" OR "${KUDU_USE_UBSAN}"))
+      message(WARNING "Cannot use lld < 10.0.0 with TSAN/ASAN/UBSAN. Skipping...")
+      continue()
+    endif()
+    set(linker_flags "${candidate_flag}")
+    break()
+  endforeach()
+
+  if(NOT DEFINED linker_flags)
+    message(SEND_ERROR "Could not find a suitable linker")
+  endif()
+  message(STATUS "Using linker flags: ${linker_flags}")
+  foreach(var CMAKE_SHARED_LINKER_FLAGS CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS)
+    set(${var} "${${var}} ${linker_flags}" PARENT_SCOPE)
+  endforeach()
+endfunction()
+
+# Interrogates the linker version via the C++ compiler to determine which linker
+# we're using, along with its version.
+#
+# Sets the following variables:
+#   LINKER_FOUND: true/false
+#   LINKER_FAMILY: lld/ld/gold
+#   LINKER_VERSION: version number of the linker
+# Any additional arguments are passed verbatim into the C++ compiler invocation.
+function(GET_LINKER_VERSION)
+  if(ARGN)
+    message(STATUS "Checking linker version with cflags: ${ARGN}")
+  else()
+    message(STATUS "Checking linker version when not specifying any flags")
+  endif()
+  execute_process(COMMAND ${CMAKE_CXX_COMPILER} "-Wl,--version" ${ARGN}
+    ERROR_QUIET
+    OUTPUT_VARIABLE LINKER_OUTPUT)
+  if (LINKER_OUTPUT MATCHES "GNU gold")
+    # We're expecting LINKER_OUTPUT to look like one of these:
+    #   GNU gold (version 2.24) 1.11
+    #   GNU gold (GNU Binutils for Ubuntu 2.30) 1.15
+    if (NOT "${LINKER_OUTPUT}" MATCHES "GNU gold \\([^\\)]*\\) (([0-9]+\\.?)+)")
+      message(SEND_ERROR "Could not extract GNU gold version. "
+        "Linker version output: ${LINKER_OUTPUT}")
+    endif()
+    set(LINKER_FAMILY "gold")
+    set(LINKER_VERSION "${CMAKE_MATCH_1}")
+    set(LINKER_FOUND TRUE)
+  elseif (LINKER_OUTPUT MATCHES "GNU ld")
+    # We're expecting LINKER_OUTPUT to look like one of these:
+    #   GNU ld (GNU Binutils for Ubuntu) 2.30
+    #   GNU ld version 2.20.51.0.2-5.42.el6 20100205
+    #   GNU ld version 2.25.1-22.base.el7
+    if (NOT "${LINKER_OUTPUT}" MATCHES "GNU ld version (([0-9]+\\.?)+)" AND
+        NOT "${LINKER_OUTPUT}" MATCHES "GNU ld \\([^\\)]*\\) (([0-9]+\\.?)+)")
+      message(SEND_ERROR "Could not extract GNU ld version. "
+        "Linker version output: ${LINKER_OUTPUT}")
+    endif()
+    set(LINKER_FAMILY "ld")
+    set(LINKER_VERSION "${CMAKE_MATCH_1}")
+    set(LINKER_FOUND TRUE)
+  elseif (LINKER_OUTPUT MATCHES "LLD")
+    # Sample:
+    #   LLD 9.0.0 (example.com:kudu.git a5848a4c3c8c72a1ac823182e87cd1f6c31ddc15) (compatible
with GNU linkers)
+    if (NOT "${LINKER_OUTPUT}" MATCHES "LLD (([0-9]+\\.?)+)")
+      message(SEND_ERROR "Could not extract lld version. "
+        "Linker version output: ${LINKER_OUTPUT}")
+    endif()
+    set(LINKER_FAMILY "lld")
+    set(LINKER_VERSION "${CMAKE_MATCH_1}")
+    set(LINKER_FOUND TRUE)
+  else()
+    set(LINKER_FAMILY "")
+    set(LINKER_VERSION "")
+    set(LINKER_FOUND FALSE)
+  endif()
+
+  if(LINKER_FOUND)
+    message(STATUS "Found linker: ${LINKER_FAMILY} version ${LINKER_VERSION}")
+  else()
+    message(STATUS "Linker not found")
+  endif()
+  # Propagate the results to the caller.
+  set(LINKER_FOUND "${LINKER_FOUND}" PARENT_SCOPE)
+  set(LINKER_FAMILY "${LINKER_FAMILY}" PARENT_SCOPE)
+  set(LINKER_VERSION "${LINKER_VERSION}" PARENT_SCOPE)
+endfunction()


Mime
View raw message