From 9b0af6e88246bfea58db71dce881bb3bdcdaecd3 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Thu, 26 Dec 2019 00:53:12 -0800 Subject: [PATCH] Enable OpenMP with Apple Clang (Mac default compiler) (#5146) * Add OpenMP as CMake target * Require CMake 3.12, to allow linking OpenMP target to objxgboost * Specify OpenMP compiler flag for CUDA host compiler * Require CMake 3.16+ if the OS is Mac OSX * Use AppleClang in Mac tests. * Update dmlc-core --- .travis.yml | 4 ++-- CMakeLists.txt | 22 ++++++++++------------ demo/c-api/CMakeLists.txt | 2 +- dmlc-core | 2 +- src/CMakeLists.txt | 17 +++++------------ tests/ci_build/Dockerfile.cpu | 2 +- tests/cpp/CMakeLists.txt | 8 +------- tests/travis/run_test.sh | 18 +++++++++++------- tests/travis/setup.sh | 4 ++-- 9 files changed, 34 insertions(+), 45 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2516a17e4de8..1be62ccb7cc7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,11 +21,11 @@ env: addons: homebrew: packages: - - gcc@9 + - cmake + - libomp - graphviz - openssl - libgit2 - - cmake - wget - r update: true diff --git a/CMakeLists.txt b/CMakeLists.txt index dedae4f06b30..c7075928e577 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.3) +cmake_minimum_required(VERSION 3.12) project(xgboost LANGUAGES CXX C VERSION 1.0.0) include(cmake/Utils.cmake) list(APPEND CMAKE_MODULE_PATH "${xgboost_SOURCE_DIR}/cmake/modules") @@ -9,9 +9,6 @@ if ((${CMAKE_VERSION} VERSION_GREATER 3.13) OR (${CMAKE_VERSION} VERSION_EQUAL 3 endif ((${CMAKE_VERSION} VERSION_GREATER 3.13) OR (${CMAKE_VERSION} VERSION_EQUAL 3.13)) message(STATUS "CMake version ${CMAKE_VERSION}") -if (MSVC) - cmake_minimum_required(VERSION 3.11) -endif (MSVC) if (CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 5.0) message(FATAL_ERROR "GCC version must be at least 5.0!") @@ -80,14 +77,11 @@ endif (USE_AVX) #-- Sanitizer if (USE_SANITIZER) - # Older CMake versions have had troubles with Sanitizer - cmake_minimum_required(VERSION 3.12) include(cmake/Sanitizer.cmake) enable_sanitizers("${ENABLED_SANITIZERS}") endif (USE_SANITIZER) if (USE_CUDA) - cmake_minimum_required(VERSION 3.12) SET(USE_OPENMP ON CACHE BOOL "CUDA requires OpenMP" FORCE) # `export CXX=' is ignored by CMake CUDA. set(CMAKE_CUDA_HOST_COMPILER ${CMAKE_CXX_COMPILER}) @@ -99,6 +93,15 @@ if (USE_CUDA) message(STATUS "CUDA GEN_CODE: ${GEN_CODE}") endif (USE_CUDA) +if (USE_OPENMP) + if (APPLE) + # Require CMake 3.16+ on Mac OSX, as previous versions of CMake had trouble locating + # OpenMP on Mac. See https://github.com/dmlc/xgboost/pull/5146#issuecomment-568312706 + cmake_minimum_required(VERSION 3.16) + endif (APPLE) + find_package(OpenMP REQUIRED) +endif (USE_OPENMP) + # dmlc-core msvc_use_static_runtime() add_subdirectory(${xgboost_SOURCE_DIR}/dmlc-core) @@ -146,11 +149,6 @@ endif (JVM_BINDINGS) #-- CLI for xgboost add_executable(runxgboost ${xgboost_SOURCE_DIR}/src/cli_main.cc ${XGBOOST_OBJ_SOURCES}) -# For cli_main.cc only -if (USE_OPENMP) - find_package(OpenMP REQUIRED) - target_compile_options(runxgboost PRIVATE ${OpenMP_CXX_FLAGS}) -endif (USE_OPENMP) target_include_directories(runxgboost PRIVATE diff --git a/demo/c-api/CMakeLists.txt b/demo/c-api/CMakeLists.txt index 308ac4a2c992..20a8158af252 100644 --- a/demo/c-api/CMakeLists.txt +++ b/demo/c-api/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.3) +cmake_minimum_required(VERSION 3.12) find_package(xgboost REQUIRED) add_executable(api-demo c-api-demo.c) target_link_libraries(api-demo xgboost::xgboost) diff --git a/dmlc-core b/dmlc-core index 20676bbc413a..61bb900ff925 160000 --- a/dmlc-core +++ b/dmlc-core @@ -1 +1 @@ -Subproject commit 20676bbc413a7d1f0229ec32701743cd5c205360 +Subproject commit 61bb900ff92545cd59b613331a38aff08577f318 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index b11d6cf16d70..ad11d892f14d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -29,8 +29,6 @@ if (USE_CUDA) target_compile_definitions(objxgboost PRIVATE -DXGBOOST_USE_NVTX=1) endif (USE_NVTX) - # OpenMP is mandatory for cuda version - find_package(OpenMP REQUIRED) target_compile_options(objxgboost PRIVATE $<$:-Xcompiler=${OpenMP_CXX_FLAGS}> ) @@ -78,17 +76,12 @@ if (XGBOOST_BUILTIN_PREFETCH_PRESENT) -DXGBOOST_BUILTIN_PREFETCH_PRESENT=1) endif (XGBOOST_BUILTIN_PREFETCH_PRESENT) -if (USE_OPENMP) +if (USE_OPENMP OR USE_CUDA) # CUDA requires OpenMP find_package(OpenMP REQUIRED) - if (OpenMP_CXX_FOUND OR OPENMP_FOUND) - target_compile_options(objxgboost PRIVATE $<$:${OpenMP_CXX_FLAGS}>) - if ((NOT OpenMP_CXX_LIBRARIES) AND (NOT MSVC)) # old CMake doesn't define this variable - set(OpenMP_CXX_LIBRARIES "gomp;pthread") - endif ((NOT OpenMP_CXX_LIBRARIES) AND (NOT MSVC)) - list(APPEND SRC_LIBS ${OpenMP_CXX_LIBRARIES}) - set(LINKED_LIBRARIES_PRIVATE "${LINKED_LIBRARIES_PRIVATE};${SRC_LIBS}" PARENT_SCOPE) - endif (OpenMP_CXX_FOUND OR OPENMP_FOUND) -endif (USE_OPENMP) + list(APPEND SRC_LIBS OpenMP::OpenMP_CXX) + set(LINKED_LIBRARIES_PRIVATE "${LINKED_LIBRARIES_PRIVATE};${SRC_LIBS}" PARENT_SCOPE) + target_link_libraries(objxgboost PRIVATE OpenMP::OpenMP_CXX) +endif (USE_OPENMP OR USE_CUDA) # For MSVC: Call msvc_use_static_runtime() once again to completely # replace /MD with /MT. See https://github.com/dmlc/xgboost/issues/4462 diff --git a/tests/ci_build/Dockerfile.cpu b/tests/ci_build/Dockerfile.cpu index 56f740c63482..7f85010466d0 100644 --- a/tests/ci_build/Dockerfile.cpu +++ b/tests/ci_build/Dockerfile.cpu @@ -1,5 +1,5 @@ FROM ubuntu:18.04 -ARG CMAKE_VERSION=3.3 +ARG CMAKE_VERSION=3.12 # Environment ENV DEBIAN_FRONTEND noninteractive diff --git a/tests/cpp/CMakeLists.txt b/tests/cpp/CMakeLists.txt index 09a40deee852..a4ddf33a7f47 100644 --- a/tests/cpp/CMakeLists.txt +++ b/tests/cpp/CMakeLists.txt @@ -55,18 +55,12 @@ set_target_properties( testxgboost PROPERTIES CXX_STANDARD 11 CXX_STANDARD_REQUIRED ON) -if ((NOT OpenMP_CXX_LIBRARIES) AND (NOT MSVC)) # old CMake doesn't define this variable - set(OpenMP_CXX_LIBRARIES "gomp;pthread") -endif ((NOT OpenMP_CXX_LIBRARIES) AND (NOT MSVC)) target_link_libraries(testxgboost PRIVATE ${GTEST_LIBRARIES} ${LINKED_LIBRARIES_PRIVATE} - ${OpenMP_CXX_LIBRARIES}) + OpenMP::OpenMP_CXX) target_compile_definitions(testxgboost PRIVATE ${XGBOOST_DEFINITIONS}) -if (USE_OPENMP) - target_compile_options(testxgboost PRIVATE $<$:${OpenMP_CXX_FLAGS}>) -endif (USE_OPENMP) set_output_directory(testxgboost ${xgboost_BINARY_DIR}) # This grouping organises source files nicely in visual studio diff --git a/tests/travis/run_test.sh b/tests/travis/run_test.sh index 0266ef36d30a..fe40987b92d6 100755 --- a/tests/travis/run_test.sh +++ b/tests/travis/run_test.sh @@ -1,14 +1,18 @@ #!/bin/bash -cp make/travis.mk config.mk make -f dmlc-core/scripts/packages.mk lz4 -if [ ${TRAVIS_OS_NAME} == "osx" ]; then - echo 'USE_OPENMP=0' >> config.mk -fi +source $HOME/miniconda/bin/activate if [ ${TASK} == "python_test" ]; then - make all || exit -1 + set -e + # Build/test + rm -rf build + mkdir build && cd build + cmake .. -DUSE_OPENMP=ON -DCMAKE_VERBOSE_MAKEFILE=ON + make -j$(nproc) + cd .. + echo "-------------------------------" conda activate python3 python --version @@ -42,8 +46,8 @@ if [ ${TASK} == "cmake_test" ]; then rm -rf build mkdir build && cd build PLUGINS="-DPLUGIN_LZ4=ON -DPLUGIN_DENSE_PARSER=ON" - CC=gcc-9 CXX=g++-9 cmake .. -DGOOGLE_TEST=ON -DUSE_OPENMP=ON -DUSE_DMLC_GTEST=ON ${PLUGINS} - make + cmake .. -DCMAKE_VERBOSE_MAKEFILE=ON -DGOOGLE_TEST=ON -DUSE_OPENMP=ON -DUSE_DMLC_GTEST=ON ${PLUGINS} + make -j$(nproc) ./testxgboost cd .. rm -rf build diff --git a/tests/travis/setup.sh b/tests/travis/setup.sh index bac51b697b99..34a3844fa036 100755 --- a/tests/travis/setup.sh +++ b/tests/travis/setup.sh @@ -4,11 +4,11 @@ if [ ${TASK} == "python_test" ]; then if [ ${TRAVIS_OS_NAME} == "osx" ]; then wget -O conda.sh https://repo.anaconda.com/miniconda/Miniconda3-latest-MacOSX-x86_64.sh else - echo "We are no longer running Linux test on Travis." + echo "We are no longer running Linux test on Travis." exit 1 fi bash conda.sh -b -p $HOME/miniconda - export PATH="$HOME/miniconda/bin:$PATH" + source $HOME/miniconda/bin/activate hash -r conda config --set always_yes yes --set changeps1 no conda update -q conda