Skip to content

Commit

Permalink
Enable OpenMP with Apple Clang (Mac default compiler) (#5146)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
hcho3 authored and trivialfis committed Dec 26, 2019
1 parent f3d7877 commit 9b0af6e
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 45 deletions.
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ env:
addons:
homebrew:
packages:
- gcc@9
- cmake
- libomp
- graphviz
- openssl
- libgit2
- cmake
- wget
- r
update: true
Expand Down
22 changes: 10 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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")
Expand All @@ -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!")
Expand Down Expand Up @@ -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})
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion demo/c-api/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
17 changes: 5 additions & 12 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
$<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=${OpenMP_CXX_FLAGS}>
)
Expand Down Expand Up @@ -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 $<$<COMPILE_LANGUAGE:CXX>:${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
Expand Down
2 changes: 1 addition & 1 deletion tests/ci_build/Dockerfile.cpu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
FROM ubuntu:18.04
ARG CMAKE_VERSION=3.3
ARG CMAKE_VERSION=3.12

# Environment
ENV DEBIAN_FRONTEND noninteractive
Expand Down
8 changes: 1 addition & 7 deletions tests/cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 $<$<COMPILE_LANGUAGE:CXX>:${OpenMP_CXX_FLAGS}>)
endif (USE_OPENMP)
set_output_directory(testxgboost ${xgboost_BINARY_DIR})

# This grouping organises source files nicely in visual studio
Expand Down
18 changes: 11 additions & 7 deletions tests/travis/run_test.sh
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/travis/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9b0af6e

Please sign in to comment.