-
Notifications
You must be signed in to change notification settings - Fork 1
Add optional add_sycl_to_target to cmake #2
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
ed505da
4c34df8
f51ff59
01452f1
59c3b76
337e9ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,6 +68,10 @@ endif() | |
option(ENABLE_CUBLAS_BACKEND "" OFF) | ||
option(ENABLE_CURAND_BACKEND "" OFF) | ||
option(ENABLE_NETLIB_BACKEND "" OFF) | ||
option(DISABLE_HALF_RUTINES "" OFF) | ||
option(ONEMKL_SYCL_IMPLEMENTATION "" "dpc++") | ||
|
||
|
||
|
||
## Domains | ||
set(DOMAINS_LIST "") | ||
|
@@ -124,7 +128,20 @@ list(APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake") | |
if(WIN32) | ||
add_library(ONEMKL::SYCL::SYCL INTERFACE IMPORTED) | ||
else() | ||
find_package(Compiler REQUIRED) | ||
# Find necessary packages | ||
if (${ONEMKL_SYCL_IMPLEMENTATION} STREQUAL "hipSYCL") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd prefer to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm inclined to disagree about hipSYCL being lowercase. SYCL is a term that is inherently and strictly defined as all capital. DPC++ does not have such strong capitalization definitions. We also pretty consistently use the hipSYCL capitalization in all our stuff - the only exception being C++ source code inside hipSYCL. In any case, actively turning everything lower case and hence accepting any capitalization seems like the best way to go. |
||
message(STATUS "Looking for hipSYCL") | ||
find_package(hipSYCL CONFIG REQUIRED) | ||
set(USE_ADD_SYCL_TO_TARGET_INTEGRATION true) | ||
set(DISABLE_HALF_RUTINES ON) | ||
add_library(ONEMKL::SYCL::SYCL INTERFACE IMPORTED) | ||
elseif(${ONEMKL_SYCL_IMPLEMENTATION} STREQUAL "dpc++") | ||
message(STATUS "Looking for dpc++") | ||
set(USE_ADD_SYCL_TO_TARGET_INTEGRATION false) | ||
find_package(Compiler REQUIRED) | ||
else() | ||
message(FATAL_ERROR "SYCL implementation ${ONEMKL_SYCL_IMPLEMENTATION} is not known") | ||
endif() | ||
endif() | ||
|
||
# Add source directory and output to bin/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,17 +20,16 @@ | |
set(LIB_NAME onemkl_blas_cublas) | ||
set(LIB_OBJ ${LIB_NAME}_obj) | ||
find_package(cuBLAS REQUIRED) | ||
|
||
set(SOURCES cublas_level1.cpp | ||
cublas_level2.cpp | ||
cublas_level3.cpp | ||
cublas_batch.cpp | ||
cublas_extensions.cpp | ||
cublas_scope_handle.cpp | ||
$<$<BOOL:${BUILD_SHARED_LIBS}>: mkl_blas_cublas_wrappers.cpp>) | ||
add_library(${LIB_NAME}) | ||
add_library(${LIB_OBJ} OBJECT | ||
cublas_level1.cpp | ||
cublas_level2.cpp | ||
cublas_level3.cpp | ||
cublas_batch.cpp | ||
cublas_extensions.cpp | ||
cublas_scope_handle.cpp | ||
$<$<BOOL:${BUILD_SHARED_LIBS}>: mkl_blas_cublas_wrappers.cpp> | ||
) | ||
add_library(${LIB_OBJ} OBJECT ${SOURCES}) | ||
|
||
target_include_directories(${LIB_OBJ} | ||
PRIVATE ${PROJECT_SOURCE_DIR}/include | ||
${PROJECT_SOURCE_DIR}/src | ||
|
@@ -42,7 +41,9 @@ set_target_properties(${LIB_OBJ} PROPERTIES | |
POSITION_INDEPENDENT_CODE ON) | ||
|
||
target_link_libraries(${LIB_NAME} PUBLIC ${LIB_OBJ}) | ||
|
||
if (USE_ADD_SYCL_TO_TARGET_INTEGRATION) | ||
add_sycl_to_target(TARGET ${LIB_OBJ} SOURCES ${SOURCES}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we even have to specify the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hipSYCL doesn't need it, but it's the correct interface for portability, as e.g. ComputeCpp needs it AFAIK. |
||
endif() | ||
# Add major version to the library | ||
set_target_properties(${LIB_NAME} PROPERTIES | ||
SOVERSION ${PROJECT_VERSION_MAJOR} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,13 +21,14 @@ set(LIB_NAME onemkl_blas_mklcpu) | |
set(LIB_OBJ ${LIB_NAME}_obj) | ||
|
||
find_package(MKL REQUIRED) | ||
|
||
add_library(${LIB_NAME}) | ||
add_library(${LIB_OBJ} OBJECT | ||
fp16.hpp mklcpu_common.hpp | ||
set(SOURCES fp16.hpp mklcpu_common.hpp | ||
mklcpu_level1.cpp mklcpu_level2.cpp mklcpu_level3.cpp mklcpu_batch.cpp mklcpu_extensions.cpp | ||
$<$<BOOL:${BUILD_SHARED_LIBS}>: mklcpu_wrappers.cpp> | ||
) | ||
$<$<BOOL:${BUILD_SHARED_LIBS}>: mklcpu_wrappers.cpp>) | ||
add_library(${LIB_NAME}) | ||
add_library(${LIB_OBJ} OBJECT ${SOURCES}) | ||
if (USE_ADD_SYCL_TO_TARGET_INTEGRATION) | ||
add_sycl_to_target(TARGET ${LIB_OBJ} ${SOURCES}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you specify the sources here, don't you need the |
||
endif() | ||
|
||
target_include_directories(${LIB_OBJ} | ||
PRIVATE ${PROJECT_SOURCE_DIR}/include | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,9 @@ foreach(domain ${TARGET_DOMAINS}) | |
add_executable(test_main_${domain}_ct main_test.cpp) | ||
target_include_directories(test_main_${domain}_ct PUBLIC ${GTEST_INCLUDE_DIR}) | ||
target_compile_options(test_main_${domain}_ct PRIVATE -fsycl) | ||
if (USE_ADD_SYCL_TO_TARGET_INTEGRATION) | ||
add_sycl_to_target(TARGET test_main_${domain}_ct SOURCES t main_test.cpp) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
also, shouldn't the setting of the |
||
endif() | ||
|
||
if(BUILD_SHARED_LIBS) | ||
add_executable(test_main_${domain}_rt main_test.cpp) | ||
|
@@ -73,6 +76,9 @@ foreach(domain ${TARGET_DOMAINS}) | |
onemkl | ||
${${domain}_TEST_LIST_RT} | ||
) | ||
if (USE_ADD_SYCL_TO_TARGET_INTEGRATION) | ||
add_sycl_to_target(TARGET test_main_${domain}_rt SOURCES main_test.cpp) | ||
endif() | ||
endif() | ||
|
||
if(ENABLE_MKLCPU_BACKEND) | ||
|
@@ -109,7 +115,6 @@ foreach(domain ${TARGET_DOMAINS}) | |
ONEMKL::SYCL::SYCL | ||
${${domain}_TEST_LIST_CT} | ||
) | ||
|
||
string(TOUPPER ${domain} DOMAIN_PREFIX) | ||
|
||
if(BUILD_SHARED_LIBS) | ||
|
@@ -130,5 +135,8 @@ foreach(domain ${TARGET_DOMAINS}) | |
PROPERTIES TEST_PREFIX ${DOMAIN_PREFIX}/CT/ | ||
DISCOVERY_TIMEOUT 30 | ||
) | ||
if (USE_ADD_SYCL_TO_TARGET_INTEGRATION) | ||
add_sycl_to_target(TARGET test_main_${domain}_rt) | ||
endif() | ||
|
||
endforeach() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,11 @@ if(BUILD_SHARED_LIBS) | |
PUBLIC ${CBLAS_INCLUDE} | ||
) | ||
target_link_libraries(blas_batch_rt PUBLIC ONEMKL::SYCL::SYCL) | ||
if (USE_ADD_SYCL_TO_TARGET_INTEGRATION) | ||
add_sycl_to_target(TARGET blas_batch_rt SOURCES ${BATCH_SOURCES}) | ||
else() | ||
target_link_libraries(blas_batch_rt PUBLIC ONEMKL::SYCL::SYCL) | ||
endif() | ||
endif() | ||
|
||
add_library(blas_batch_ct OBJECT ${BATCH_SOURCES}) | ||
|
@@ -45,3 +50,8 @@ target_include_directories(blas_batch_ct | |
PUBLIC ${CBLAS_INCLUDE} | ||
) | ||
target_link_libraries(blas_batch_ct PUBLIC ONEMKL::SYCL::SYCL) | ||
if (USE_ADD_SYCL_TO_TARGET_INTEGRATION) | ||
add_sycl_to_target(TARGET blas_batch_ct SOURCES ${BATCH_SOURCES}) | ||
else() | ||
target_link_libraries(blas_batch_ct PUBLIC ONEMKL::SYCL::SYCL) | ||
endif() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing line at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah here it is... an alternative naming could be
ENABLE_HALF_DATA_TYPE
or similar (inspired by SYCL-BLAS..)