-
Notifications
You must be signed in to change notification settings - Fork 11
Support for SYCL #230
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
Support for SYCL #230
Changes from 6 commits
5fbeaec
04dccff
308401e
8e30813
21cd89c
4b90527
cf3bd85
2a23c5b
b95d0eb
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 |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Load gen9 arch with SYCL backend for kokkos | ||
| set(CMAKE_CXX_STANDARD 17) | ||
| include (${CMAKE_CURRENT_LIST_DIR}/kokkos/intel-gen9.cmake) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # Load XEHP arch with SYCL backend for kokkos | ||
| include (${CMAKE_CURRENT_LIST_DIR}/kokkos/intel-xehp.cmake) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| include (${CMAKE_CURRENT_LIST_DIR}/generic.cmake) | ||
|
|
||
| option(Kokkos_ARCH_INTEL_GEN9 "" ON) | ||
| set(Kokkos_ENABLE_SYCL TRUE CACHE BOOL "") | ||
| set(Kokkos_ENABLE_DEPRECATED_CODE_3 FALSE CACHE BOOL "") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| include (${CMAKE_CURRENT_LIST_DIR}/generic.cmake) | ||
|
|
||
| option(Kokkos_ARCH_INTEL_XEHP "" ON) | ||
| set(Kokkos_ENABLE_SYCL TRUE CACHE BOOL "") | ||
| set(Kokkos_ENABLE_DEPRECATED_CODE_3 FALSE CACHE BOOL "") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,7 +71,12 @@ set(EKAT_HEADERS | |
| add_library(ekat ${EKAT_SOURCES}) | ||
|
|
||
| # Link MPI | ||
| target_link_libraries (ekat PUBLIC MPI::MPI_C) | ||
| if (Kokkos_ROOT) | ||
|
Contributor
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. This if statement is not necessary if we do the mod to |
||
| # Link Kokkos in-addition if not using the submodule | ||
| target_link_libraries (ekat PUBLIC Kokkos::kokkos MPI::MPI_C) | ||
| else () | ||
| target_link_libraries (ekat PUBLIC MPI::MPI_C) | ||
| endif () | ||
|
|
||
| target_include_directories(ekat PUBLIC | ||
| $<BUILD_INTERFACE:${EKAT_SOURCE_DIR}/src> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,7 +57,11 @@ namespace impl { | |
| * detail, and, normally, should not be used by customer apps | ||
| */ | ||
| template <bool Serialize, typename TeamMember, typename Lambda, typename ValueType> | ||
| #ifdef KOKKOS_ENABLE_SYCL | ||
| KOKKOS_INLINE_FUNCTION SYCL_EXTERNAL | ||
|
Contributor
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. We do have lots of KOKKOS_INLINE_FUNCTION in EKAT. Why does this particular case require a special handling? And what does this macro do differently from KOKKOS_INLINE_FUNCTION?
Collaborator
Author
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. There is a bit of limitation on how static device-side functions and linking can be setup with SYCL. There are majorly two changes suggested for this function alone and has nothing to do with Regarding the need of I have made some changes to cleanup this. Thanks.
Contributor
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. Ah, I misread the two macros as just a single one.
Contributor
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. But still, why do we need external here? The
Collaborator
Author
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. That is exactly right. The definition available to all other translation units during linking. Mostly a temporary one, this will be improved in the future revisions of the specifications and the compiler implementations.
Contributor
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. But then we should not need
Collaborator
Author
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. Making the function template <bool Serialize, typename TeamMember, typename Lambda, typename ValueType>
#ifdef KOKKOS_ENABLE_SYCL
SYCL_EXTERNAL
#else
static
#endif
KOKKOS_INLINE_FUNCTION
void parallel_reduce (const TeamMember& team,
const int& begin, // pack index
const int& end, // pack index
const Lambda& lambda,
ValueType& result)
Contributor
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. Are you saying that SYCL complains about the function being static, and requires the external keyword to compile? Either I am not understanding SYCL, or this seems wrong. The "inline" and "extern" keywords are somewhat mutually exclusive, so I am surprised that SYCL needs that macro. The documentation suggests that it is needed if the definition is not available to other TU's, but since it's inlined, it should be available as soon as this header is included. Anyhow, if you say that it does not compile without that macro, then it's fine. I'm just not really happy with this confusing detail.
Collaborator
Author
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. Sorry for the confusion. The need for SYCL_EXTERNAL is not required any more. There has been a mix-up with compiler versions at my end. Thanks for pointing this out. |
||
| #else | ||
| static KOKKOS_INLINE_FUNCTION | ||
| #endif | ||
| void parallel_reduce (const TeamMember& team, | ||
| const int& begin, // pack index | ||
| const int& end, // pack index | ||
|
|
@@ -88,7 +92,7 @@ void parallel_reduce (const TeamMember& team, | |
| #ifdef EKAT_ENABLE_GPU | ||
| // Broadcast result to all threads by doing sum of one thread's | ||
| // non-0 value and the rest of the 0s. | ||
| TeamMember::vector_reduce(Kokkos::Sum<ValueType>(local_tmp)); | ||
| team.vector_reduce(Kokkos::Sum<ValueType>(local_tmp)); | ||
| #endif | ||
|
|
||
| result = local_tmp; | ||
|
|
@@ -525,7 +529,7 @@ class TeamUtils<ValueType, Kokkos::OpenMP> : public TeamUtilsCommonBase<ValueTyp | |
| #endif | ||
|
|
||
| /* | ||
| * Specialization for Cuda execution space. | ||
| * Specialization for CUDA, HIP and SYCL execution space. | ||
| */ | ||
| #ifdef EKAT_ENABLE_GPU | ||
| template <typename ValueType> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.