diff --git a/.clang-tidy b/.clang-tidy index 959b0a438..1708dcf52 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,4 +1,4 @@ -Checks: 'modernize-*,-modernize-make-*,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming' +Checks: 'modernize-*,-modernize-make-*,-modernize-use-auto,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming' CheckOptions: - { key: readability-identifier-naming.ClassCase, value: CamelCase } - { key: readability-identifier-naming.StructCase, value: CamelCase } diff --git a/CMakeLists.txt b/CMakeLists.txt index aec9c0664..4a7ebdf25 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -123,8 +123,6 @@ file(GLOB_RECURSE SOURCES # Only add main function for executable target list(REMOVE_ITEM SOURCES ${PROJECT_SOURCE_DIR}/src/cli_main.cc) -file(GLOB_RECURSE TEST_SOURCES "tests/cpp/*.cc") - file(GLOB_RECURSE CUDA_SOURCES src/*.cu src/*.cuh @@ -140,7 +138,7 @@ if(PLUGIN_DENSE_PARSER) endif() # rabit -# TODO: Create rabit cmakelists.txt +# TODO: Use CMakeLists.txt from rabit. set(RABIT_SOURCES rabit/src/allreduce_base.cc rabit/src/allreduce_robust.cc @@ -151,6 +149,7 @@ set(RABIT_EMPTY_SOURCES rabit/src/engine_empty.cc rabit/src/c_api.cc ) + if(MINGW OR R_LIB) # build a dummy rabit library add_library(rabit STATIC ${RABIT_EMPTY_SOURCES}) @@ -158,7 +157,11 @@ else() add_library(rabit STATIC ${RABIT_SOURCES}) endif() -if(USE_CUDA) +if (GENERATE_COMPILATION_DATABASE) + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +endif (GENERATE_COMPILATION_DATABASE) + +if(USE_CUDA AND (NOT GENERATE_COMPILATION_DATABASE)) find_package(CUDA 8.0 REQUIRED) cmake_minimum_required(VERSION 3.5) @@ -168,7 +171,7 @@ if(USE_CUDA) if(USE_NCCL) find_package(Nccl REQUIRED) - include_directories(${NCCL_INCLUDE_DIR}) + cuda_include_directories(${NCCL_INCLUDE_DIR}) add_definitions(-DXGBOOST_USE_NCCL) endif() @@ -188,6 +191,39 @@ if(USE_CUDA) target_link_libraries(gpuxgboost ${NCCL_LIB_NAME}) endif() list(APPEND LINK_LIBRARIES gpuxgboost) + +elseif (USE_CUDA AND GENERATE_COMPILATION_DATABASE) + # Enable CUDA language to generate a compilation database. + cmake_minimum_required(VERSION 3.8) + + find_package(CUDA 8.0 REQUIRED) + enable_language(CUDA) + set(CMAKE_CUDA_COMPILER clang++) + set(CUDA_SEPARABLE_COMPILATION ON) + if (NOT CLANG_CUDA_GENCODE) + set(CLANG_CUDA_GENCODE "--cuda-gpu-arch=sm_35") + endif (NOT CLANG_CUDA_GENCODE) + set(CMAKE_CUDA_FLAGS " -Wno-deprecated ${CLANG_CUDA_GENCODE} -fPIC ${GENCODE} -std=c++11 -x cuda") + message(STATUS "CMAKE_CUDA_FLAGS: ${CMAKE_CUDA_FLAGS}") + + add_library(gpuxgboost STATIC ${CUDA_SOURCES}) + + if(USE_NCCL) + find_package(Nccl REQUIRED) + target_include_directories(gpuxgboost PUBLIC ${NCCL_INCLUDE_DIR}) + target_compile_definitions(gpuxgboost PUBLIC -DXGBOOST_USE_NCCL) + target_link_libraries(gpuxgboost PUBLIC ${NCCL_LIB_NAME}) + endif() + + target_compile_definitions(gpuxgboost PUBLIC -DXGBOOST_USE_CUDA) + # A hack for CMake to make arguments valid for clang++ + string(REPLACE "-x cu" "-x cuda" CMAKE_CUDA_COMPILE_PTX_COMPILATION + ${CMAKE_CUDA_COMPILE_PTX_COMPILATION}) + string(REPLACE "-x cu" "-x cuda" CMAKE_CUDA_COMPILE_WHOLE_COMPILATION + ${CMAKE_CUDA_COMPILE_WHOLE_COMPILATION}) + string(REPLACE "-x cu" "-x cuda" CMAKE_CUDA_COMPILE_SEPARABLE_COMPILATION + ${CMAKE_CUDA_COMPILE_SEPARABLE_COMPILATION}) + target_include_directories(gpuxgboost PUBLIC cub) endif() @@ -203,7 +239,6 @@ endif() add_library(objxgboost OBJECT ${SOURCES}) - # building shared library for R package if(R_LIB) find_package(LibR REQUIRED) @@ -211,13 +246,13 @@ if(R_LIB) list(APPEND LINK_LIBRARIES "${LIBR_CORE_LIBRARY}") MESSAGE(STATUS "LIBR_CORE_LIBRARY " ${LIBR_CORE_LIBRARY}) - include_directories( + # Shared library target for the R package + add_library(xgboost SHARED $) + include_directories(xgboost "${LIBR_INCLUDE_DIRS}" "${PROJECT_SOURCE_DIR}" ) - # Shared library target for the R package - add_library(xgboost SHARED $) target_link_libraries(xgboost ${LINK_LIBRARIES}) # R uses no lib prefix in shared library names of its packages set_target_properties(xgboost PROPERTIES PREFIX "") @@ -229,7 +264,7 @@ if(R_LIB) # use a dummy location for any other remaining installs set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/dummy_inst") -# main targets: shared library & exe + # main targets: shared library & exe else() # Executable add_executable(runxgboost $ src/cli_main.cc) @@ -252,20 +287,20 @@ else() add_dependencies(xgboost runxgboost) endif() - # JVM if(JVM_BINDINGS) find_package(JNI QUIET REQUIRED) - include_directories(${JNI_INCLUDE_DIRS} jvm-packages/xgboost4j/src/native) - add_library(xgboost4j SHARED - $ - jvm-packages/xgboost4j/src/native/xgboost4j.cpp) - set_output_directory(xgboost4j ${PROJECT_SOURCE_DIR}/lib) + $ + jvm-packages/xgboost4j/src/native/xgboost4j.cpp) + target_include_directories(xgboost4j + PRIVATE ${JNI_INCLUDE_DIRS} + PRIVATE jvm-packages/xgboost4j/src/native) target_link_libraries(xgboost4j - ${LINK_LIBRARIES} - ${JAVA_JVM_LIBRARY}) + ${LINK_LIBRARIES} + ${JAVA_JVM_LIBRARY}) + set_output_directory(xgboost4j ${PROJECT_SOURCE_DIR}/lib) endif() @@ -274,18 +309,31 @@ if(GOOGLE_TEST) enable_testing() find_package(GTest REQUIRED) + file(GLOB_RECURSE TEST_SOURCES "tests/cpp/*.cc") auto_source_group("${TEST_SOURCES}") - include_directories(${GTEST_INCLUDE_DIRS}) - if(USE_CUDA) + if(USE_CUDA AND (NOT GENERATE_COMPILATION_DATABASE)) file(GLOB_RECURSE CUDA_TEST_SOURCES "tests/cpp/*.cu") + cuda_include_directories(${GTEST_INCLUDE_DIRS}) cuda_compile(CUDA_TEST_OBJS ${CUDA_TEST_SOURCES}) + elseif (USE_CUDA AND GENERATE_COMPILATION_DATABASE) + file(GLOB_RECURSE CUDA_TEST_SOURCES "tests/cpp/*.cu") else() set(CUDA_TEST_OBJS "") endif() - add_executable(testxgboost ${TEST_SOURCES} ${CUDA_TEST_OBJS} $) + if (USE_CUDA AND GENERATE_COMPILATION_DATABASE) + add_executable(testxgboost ${TEST_SOURCES} ${CUDA_TEST_SOURCES} + $) + target_include_directories(testxgboost PRIVATE cub) + else () + add_executable(testxgboost ${TEST_SOURCES} ${CUDA_TEST_OBJS} + $) + endif () + set_output_directory(testxgboost ${PROJECT_SOURCE_DIR}) + target_include_directories(testxgboost + PRIVATE ${GTEST_INCLUDE_DIRS}) target_link_libraries(testxgboost ${GTEST_LIBRARIES} ${LINK_LIBRARIES}) add_test(TestXGBoost testxgboost) diff --git a/Jenkinsfile b/Jenkinsfile index 143cba596..215e9102e 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -53,7 +53,7 @@ pipeline { parallel (buildMatrix.findAll{it['enabled']}.collectEntries{ c -> def buildName = utils.getBuildName(c) utils.buildFactory(buildName, c, false, this.&buildPlatformCmake) - }) + } + [ "clang-tidy" : { buildClangTidyJob() } ]) } } } @@ -106,3 +106,22 @@ def buildPlatformCmake(buildName, conf, nodeReq, dockerTarget) { } } } + +/** + * Run a clang-tidy job on a GPU machine + */ +def buildClangTidyJob() { + def nodeReq = "linux && gpu && unrestricted" + node(nodeReq) { + unstash name: 'srcs' + echo "Running clang-tidy job..." + // Invoke command inside docker + // Install Google Test and Python yaml + dockerTarget = "clang_tidy" + dockerArgs = "--build-arg CUDA_VERSION=9.2" + sh """ + ${dockerRun} ${dockerTarget} ${dockerArgs} tests/ci_build/clang_tidy.sh + """ + } + } + diff --git a/doc/contribute.rst b/doc/contribute.rst index 6335a6498..cde56efe0 100644 --- a/doc/contribute.rst +++ b/doc/contribute.rst @@ -19,6 +19,7 @@ Everyone is more than welcome to contribute. It is a way to make the project bet * `Documents`_ * `Testcases`_ * `Sanitizers`_ +* `clang-tidy`_ * `Examples`_ * `Core Library`_ * `Python Package`_ @@ -169,6 +170,31 @@ environment variable: For details, please consult `official documentation `_ for sanitizers. +********** +clang-tidy +********** +To run clang-tidy on both C++ and CUDA source code, run the following command +from the top level source tree: + + .. code-black:: bash + cd /path/to/xgboost/ + python3 tests/ci_build/tidy.py --gtest-path=/path/to/google-test + +The script requires the full path of Google Test library via the ``--gtest-path`` argument. + +Also, the script accepts two optional integer arguments, namely ``--cpp`` and ``--cuda``. +By default they are both set to 1. If you want to exclude CUDA source from +linting, use: + + .. code-black:: bash + cd /path/to/xgboost/ + python3 tests/ci_build/tidy.py --cuda=0 + +Similarly, if you want to exclude C++ source from linting: + + .. code-black:: bash + cd /path/to/xgboost/ + python3 tests/ci_build/tidy.py --cpp=0 ******** Examples diff --git a/tests/ci_build/Dockerfile.clang_tidy b/tests/ci_build/Dockerfile.clang_tidy new file mode 100644 index 000000000..1d09fce96 --- /dev/null +++ b/tests/ci_build/Dockerfile.clang_tidy @@ -0,0 +1,45 @@ +ARG CUDA_VERSION +FROM nvidia/cuda:$CUDA_VERSION-devel-ubuntu18.04 + +# Environment +ENV DEBIAN_FRONTEND noninteractive + +# Install all basic requirements +RUN \ + apt-get update && \ + apt-get install -y tar unzip wget git build-essential cmake python3 python3-pip llvm-7 clang-tidy-7 clang-7 + +# Set default clang-tidy version +RUN \ + update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-7 100 && \ + update-alternatives --install /usr/bin/clang clang /usr/bin/clang-7 100 + +# NCCL2 (License: https://docs.nvidia.com/deeplearning/sdk/nccl-sla/index.html) +RUN \ + export CUDA_SHORT=`echo $CUDA_VERSION | egrep -o '[0-9]+\.[0-9]'` && \ + if [ "${CUDA_SHORT}" != "10.0" ]; then \ + wget https://developer.download.nvidia.com/compute/redist/nccl/v2.2/nccl_2.2.13-1%2Bcuda${CUDA_SHORT}_x86_64.txz && \ + tar xf "nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64.txz" && \ + cp nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64/include/nccl.h /usr/include && \ + cp nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64/lib/* /usr/lib && \ + rm -f nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64.txz && \ + rm -r nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64; fi + +# Install Python packages +RUN \ + pip3 install pyyaml + +ENV GOSU_VERSION 1.10 + +# Install lightweight sudo (not bound to TTY) +RUN set -ex; \ + wget -O /usr/local/bin/gosu "https://github.com/tianon/gosu/releases/download/$GOSU_VERSION/gosu-amd64" && \ + chmod +x /usr/local/bin/gosu && \ + gosu nobody true + +# Default entry-point to use if running locally +# It will preserve attributes of created files +COPY entrypoint.sh /scripts/ + +WORKDIR /workspace +ENTRYPOINT ["/scripts/entrypoint.sh"] diff --git a/tests/ci_build/clang_tidy.sh b/tests/ci_build/clang_tidy.sh new file mode 100755 index 000000000..392d910e4 --- /dev/null +++ b/tests/ci_build/clang_tidy.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +rm -rf gtest googletest-release-1.7.0 +wget -nc https://github.com/google/googletest/archive/release-1.7.0.zip +unzip -n release-1.7.0.zip +mv googletest-release-1.7.0 gtest && cd gtest +cmake . && make +mkdir lib && mv libgtest.a lib +cd .. +rm -rf release-1.7.0.zip* + +python3 tests/ci_build/tidy.py --gtest-path=${PWD}/gtest diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py new file mode 100755 index 000000000..76a044228 --- /dev/null +++ b/tests/ci_build/tidy.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python +import subprocess +import yaml +import json +from multiprocessing import Pool, cpu_count +import shutil +import os +import re +import argparse + + +def call(args): + '''Subprocess run wrapper.''' + completed = subprocess.run(args, stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL) + error_msg = completed.stdout.decode('utf-8') + matched = re.match('.*xgboost.*warning.*', error_msg, + re.MULTILINE | re.DOTALL) + if matched is None: + return_code = 0 + else: + print(error_msg, '\n') + return_code = 1 + return completed.returncode | return_code + + +class ClangTidy(object): + ''' + clang tidy wrapper. + Args: + gtest_path: Full path of Google Test library. + cpp_lint: Run linter on C++ source code. + cuda_lint: Run linter on CUDA source code. + ''' + def __init__(self, gtest_path, cpp_lint, cuda_lint): + self.gtest_path = gtest_path + self.cpp_lint = cpp_lint + self.cuda_lint = cuda_lint + print('Using Google Test from {}'.format(self.gtest_path)) + print('Run linter on CUDA: ', self.cuda_lint) + print('Run linter on C++:', self.cpp_lint) + if not self.cpp_lint and not self.cuda_lint: + raise ValueError('Both --cpp and --cuda are set to 0.') + self.root_path = os.path.abspath(os.path.curdir) + print('Project root:', self.root_path) + self.cdb_path = os.path.join(self.root_path, 'cdb') + + def __enter__(self): + if os.path.exists(self.cdb_path): + shutil.rmtree(self.cdb_path) + self._generate_cdb() + return self + + def __exit__(self, *args): + if os.path.exists(self.cdb_path): + shutil.rmtree(self.cdb_path) + + def _generate_cdb(self): + '''Run CMake to generate compilation database.''' + os.mkdir(self.cdb_path) + os.chdir(self.cdb_path) + cmake_args = ['cmake', '..', '-DGENERATE_COMPILATION_DATABASE=ON', + '-DGOOGLE_TEST=ON', '-DGTEST_ROOT={}'.format( + self.gtest_path)] + if self.cuda_lint: + cmake_args.extend(['-DUSE_CUDA=ON', '-DUSE_NCCL=ON']) + subprocess.run(cmake_args) + os.chdir(self.root_path) + + def _configure_flags(self, path, command): + common_args = ['clang-tidy', + # "-header-filter='(xgboost\\/src|xgboost\\/include)'", + '-config='+str(self.clang_tidy)] + common_args.append(path) + common_args.append('--') + + command = command.split()[1:] # remove clang/c++/g++ + # filter out not used flags + if '-fuse-ld=gold' in command: + command.remove('-fuse-ld=gold') + if '-rdynamic' in command: + command.remove('-rdynamic') + if '-Xcompiler=-fPIC' in command: + command.remove('-Xcompiler=-fPIC') + if '-Xcompiler=-fPIE' in command: + command.remove('-Xcompiler=-fPIE') + if '-c' in command: + index = command.index('-c') + del command[index+1] + command.remove('-c') + if '-o' in command: + index = command.index('-o') + del command[index+1] + command.remove('-o') + + common_args.extend(command) + + # Two passes, one for device code another for host code. + if path.endswith('cu'): + args = [common_args.copy(), common_args.copy()] + args[0].append('--cuda-host-only') + args[1].append('--cuda-device-only') + else: + args = [common_args.copy()] + for a in args: + a.append('-Wno-unused-command-line-argument') + return args + + def _configure(self): + '''Load and configure compile_commands and clang_tidy.''' + + def should_lint(path): + if not self.cpp_lint and path.endswith('.cc'): + return False + return True + + cdb_file = os.path.join(self.cdb_path, 'compile_commands.json') + with open(cdb_file, 'r') as fd: + self.compile_commands = json.load(fd) + tidy_file = os.path.join(self.root_path, '.clang-tidy') + with open(tidy_file) as fd: + self.clang_tidy = yaml.load(fd) + all_files = [] + for entry in self.compile_commands: + path = entry['file'] + if should_lint(path): + print(path) + args = self._configure_flags(path, entry['command']) + all_files.extend(args) + return all_files + + def run(self): + '''Run clang-tidy.''' + all_files = self._configure() + with Pool(cpu_count()) as pool: + results = pool.map(call, all_files) + passed = True + if 1 in results: + print('Please correct clang-tidy warnings.') + passed = False + return passed + + +if __name__ == '__main__': + parser = argparse.ArgumentParser(description='Run clang-tidy.') + parser.add_argument('--cpp', type=int, default=1) + parser.add_argument('--cuda', type=int, default=1) + parser.add_argument('--gtest-path', required=True, + help='Full path of Google Test library directory') + args = parser.parse_args() + with ClangTidy(args.gtest_path, args.cpp, args.cuda) as linter: + passed = linter.run() + # Uncomment it once the code base is clang-tidy conformant. + # if not passed: + # sys.exit(1)