Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable OpenMP with Apple Clang (Mac default compiler) #5146

Merged
merged 14 commits into from
Dec 26, 2019
Merged
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 -j2
trivialfis marked this conversation as resolved.
Show resolved Hide resolved
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 -j2
./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