Skip to content

Add HIP-Clang CMake changes #25

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.

Sign up for GitHub

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

Merged
jszuppe merged 4 commits into from
Aug 28, 2018
Merged

Add HIP-Clang CMake changes #25

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1461db3
Add HIP-Clang CMake changes
aaronenyeshi Aug 24, 2018
ffd862a
Create rocprim_hc if hcc PACKAGE / targets found
aaronenyeshi Aug 27, 2018
1dc3cb1
Clean up HCC check on HIP-Clang
aaronenyeshi Aug 27, 2018
b191bdd
Disable hc examples for HIP-Clang
aaronenyeshi Aug 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,21 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror")

# Determine if CXX Compiler is hcc, hip-clang or other

@jszuppe jszuppe Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to move it to cmake/VerifyCompiler.cmake after line 37, so this code it not run if HIP_PLATFORM is nvcc and only hipCUB will be built.

@jszuppe jszuppe Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I made that comment in the first "big" PR, but I forgot about it here when I did the first review of this PR.

@aaronenyeshi aaronenyeshi Aug 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a good point

execute_process(COMMAND ${CMAKE_CXX_COMPILER} "--version" OUTPUT_VARIABLE CXX_OUTPUT
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_STRIP_TRAILING_WHITESPACE)
string(REGEX MATCH "[A-Za-z]* ?clang version" TMP_CXX_VERSION ${CXX_OUTPUT})
string(REGEX MATCH "[A-Za-z]+" CXX_VERSION_STRING ${TMP_CXX_VERSION})
if(CXX_VERSION_STRING MATCHES "HCC")
set(HIP_COMPILER "hcc" CACHE STRING "HIP Compiler")
elseif(CXX_VERSION_STRING MATCHES "clang")
set(HIP_COMPILER "clang" CACHE STRING "HIP Compiler")
else()
message(FATAL_ERROR "CXX Compiler version ${CXX_VERSION_STRING} unsupported.")
endif()
message(STATUS "HIP Compiler: " ${HIP_COMPILER})

# Build options
option(BUILD_TEST "Build tests (requires googletest)" ON)
option(BUILD_BENCHMARK "Build benchmarks" OFF)
Expand Down
2 changes: 2 additions & 0 deletions benchmark/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ add_rocprim_benchmark_hip(benchmark_hip_warp_sort.cpp)
add_rocprim_benchmark_hip(benchmark_hip_device_memory.cpp)

# rocPRIM HC benchmarks
if(HIP_COMPILER STREQUAL "HCC")

@jszuppe jszuppe Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should also disable HC example in https://github.com/ROCmSoftwarePlatform/rocPRIM/blob/master/example/CMakeLists.txt

@aaronenyeshi aaronenyeshi Aug 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I had examples OFF during my compiles, I'll add this

add_rocprim_benchmark_hc(benchmark_hc_block_discontinuity.cpp)
add_rocprim_benchmark_hc(benchmark_hc_block_exchange.cpp)
add_rocprim_benchmark_hc(benchmark_hc_block_histogram.cpp)
Expand All @@ -95,3 +96,4 @@ add_rocprim_benchmark_hc(benchmark_hc_device_select.cpp)
add_rocprim_benchmark_hc(benchmark_hc_device_transform.cpp)
add_rocprim_benchmark_hc(benchmark_hc_warp_scan.cpp)
add_rocprim_benchmark_hc(benchmark_hc_warp_sort.cpp)
endif()
12 changes: 8 additions & 4 deletions cmake/VerifyCompiler.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,21 @@ if(HIP_PLATFORM STREQUAL "nvcc")
include(cmake/SetupNVCC.cmake)
message(STATUS "rocPRIM does not support NVCC. Only hipCUB will be available.")
elseif(HIP_PLATFORM STREQUAL "hcc")
if(NOT (CMAKE_CXX_COMPILER MATCHES ".*/hcc$"))
message(FATAL_ERROR "On ROCm platform 'hcc' must be used as C++ compiler.")
if(NOT (CMAKE_CXX_COMPILER MATCHES ".*/hcc$" OR CMAKE_CXX_COMPILER MATCHES ".*/hipcc$"))
message(FATAL_ERROR "On ROCm platform 'hcc' or 'clang' must be used as C++ compiler.")
else()
# Workaround until hcc & hip cmake modules fixes symlink logic in their config files.
# (Thanks to rocBLAS devs for finding workaround for this problem.)
list(APPEND CMAKE_PREFIX_PATH /opt/rocm/hcc /opt/rocm/hip)
# Ignore hcc warning: argument unused during compilation: '-isystem /opt/rocm/hip/include'
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-command-line-argument")
find_package(hcc REQUIRED CONFIG PATHS /opt/rocm)
if(HIP_COMPILER STREQUAL "HCC")
find_package(hcc REQUIRED CONFIG PATHS /opt/rocm)
else()
find_package(hcc QUIET CONFIG PATHS /opt/rocm)
endif()
find_package(hip REQUIRED CONFIG PATHS /opt/rocm)
endif()
else()
message(FATAL_ERROR "HIP_PLATFORM must be 'hcc' (AMD ROCm platform) or `nvcc` (NVIDIA CUDA platform).")
message(FATAL_ERROR "HIP_PLATFORM must be 'hcc' or 'clang' (AMD ROCm platform) or `nvcc` (NVIDIA CUDA platform).")
endif()
25 changes: 14 additions & 11 deletions rocprim/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,20 @@ target_include_directories(rocprim

# This target allows using only HC interface, links only
# against HC/HSA library, doesn't require HIP
add_library(rocprim_hc INTERFACE)
target_link_libraries(rocprim_hc
INTERFACE
rocprim
hcc::hccrt
hcc::hc_am
)
target_compile_definitions(rocprim_hc
INTERFACE
ROCPRIM_HC_API=1
)
if(HIP_COMPILER STREQUAL "HCC" OR (hcc_FOUND AND TARGET hcc::hccrt AND TARGET hcc::hc_am))

@jszuppe jszuppe Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you only need to check hcc_FOUND, other conditions must be true if that one is.

Question: If you are using hip-clang to compile HIP code, are hcc::hccrt and hcc::hc_am targets (and generally hcc package) needed at all? I mean: is there dependency like for hcc between those libs and HIP?

@jszuppe jszuppe Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also check line 83 of this file. rocprim_hc is being used even if it's not created.

@aaronenyeshi aaronenyeshi Aug 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are using HIP-Clang to compile HIP code, you should have zero dependency on HCC (or any of its targets). But there is still work on removing some of those dependencies.

With HIP-Clang, all you need would be llvm/clang/lld/device-libs/and a new HIP runtime. No more dependence on HCC rt.

@jszuppe jszuppe Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, so it will HCC rt won't be needed at the time HIP-Clang is ready. Thanks for confirmation.

add_library(rocprim_hc INTERFACE)
target_link_libraries(rocprim_hc
INTERFACE
rocprim
hcc::hccrt
hcc::hc_am
)
target_compile_definitions(rocprim_hc
INTERFACE
ROCPRIM_HC_API=1
)
endif()


# This target allows using both HIP and HC interfaces,
# links against HIP library (which depends on HC)
Expand Down
25 changes: 17 additions & 8 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,19 @@ function(add_hc_test TEST_NAME TEST_SOURCES)
PUBLIC
${GTEST_INCLUDE_DIRS}
)
target_link_libraries(${TEST_TARGET}
PRIVATE
hcc::hccrt
hcc::hc_am
${GTEST_BOTH_LIBRARIES}
)
if(HIP_COMPILER STREQUAL "HCC")
target_link_libraries(${TEST_TARGET}
PRIVATE
hcc::hccrt
hcc::hc_am
${GTEST_BOTH_LIBRARIES}
)
elseif(HIP_COMPILER STREQUAL "clang")
target_link_libraries(${TEST_TARGET}
PRIVATE
${GTEST_BOTH_LIBRARIES}
)
endif()
foreach(amdgpu_target ${AMDGPU_TARGETS})
target_link_libraries(${TEST_TARGET}
PRIVATE
Expand All @@ -86,8 +93,10 @@ endfunction()

# HC and HIP tests without using rocPRIM
if(HIP_PLATFORM STREQUAL "hcc")
add_hip_test("hc.device_api" test_hip_api.cpp)
add_hc_test ("hip.device_api" test_hc_api.cpp)
add_hip_test("hip.device_api" test_hip_api.cpp)
if(HIP_COMPILER STREQUAL "HCC")
add_hc_test ("hc.device_api" test_hc_api.cpp)
endif()
endif()

# rocPRIM test (run only on ROCm/hcc)
Expand Down
2 changes: 2 additions & 0 deletions test/rocprim/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ endfunction()
#

# HCP basic test, which also checks if there are no linkage problems when there are multiple sources
if(HIP_COMPILER STREQUAL "HCC")
add_rocprim_test_hc("rocprim.hc.basic_test" "test_hc_basic.cpp;detail/get_rocprim_version_hc.cpp")

add_rocprim_test_hc("rocprim.hc.arg_index_iterator" test_hc_arg_index_iterator.cpp)
Expand Down Expand Up @@ -98,6 +99,7 @@ add_rocprim_test_hc("rocprim.hc.warp_reduce" test_hc_warp_reduce.cpp)
add_rocprim_test_hc("rocprim.hc.warp_scan" test_hc_warp_scan.cpp)
add_rocprim_test_hc("rocprim.hc.warp_sort" test_hc_warp_sort.cpp)
add_rocprim_test_hc("rocprim.hc.zip_iterator" test_hc_zip_iterator.cpp)
endif()

#
# rocPRIM HIP API tests
Expand Down