[SYCL] Allow work group scratch memory to be used with free function kernels #19837
Conversation
…mark them with the appropriate attribute
lbushi25
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
| if (F.getCallingConv() == CallingConv::SPIR_KERNEL) { | ||
| int ArgPos = GetArgumentPos(F); | ||
| SPIRKernelNames.emplace_back(F.getName(), ArgPos); | ||
| if (ArgPos >= 0 || F.hasFnAttribute(WORK_GROUP_STATIC_ATTR)) | ||
| SPIRKernelNames.emplace_back(F.getName(), ArgPos); |
There was a problem hiding this comment.
I think that these changes can be reverted.
With the changes below we mark all kernels using the feature automatically if they weren't marked by headers, so there is no need to analyze arguments of kernels without the attribute
| void double_kernel(float *src, float *dst) { | ||
| size_t lid = syclext::this_work_item::get_nd_item<1>().get_local_linear_id(); | ||
|
|
||
| float *local_mem = (float *)syclexp::get_work_group_scratch_memory(); |
There was a problem hiding this comment.
While we are it, we need to make sure that other extensions for local/work-group memory are covered as well (i.e. work with free function kernels):
sycl_ext_oneapi_work_group_scratch_memoryis covered by this new testsycl_ext_oneapi_work_group_memoryis covered byreduction_free_function.cppsycl_ext_oneapi_work_group_static- brief look says that this one is not tested with free function kernels yet
There was a problem hiding this comment.
While we are it, we need to make sure that other extensions for local/work-group memory are covered as well (i.e. work with free function kernels):
sycl_ext_oneapi_work_group_scratch_memoryis covered by this new testsycl_ext_oneapi_work_group_memoryis covered byreduction_free_function.cppsycl_ext_oneapi_work_group_static- brief look says that this one is not tested with free function kernels yet
I have added the test.
|
@jzc, @asudarsa, @maarquitos14, @intel/dpcpp-tools-reviewers, could you please take a look as well? Notable portion of the change is actually authored by me, so I don't think that I should be the one approving it |
…com/lbushi25/llvm into free_function_kernel_scratch_memory
|
|
||
| #include "helpers.hpp" | ||
|
|
||
| #include <cassert> |
There was a problem hiding this comment.
I think this one should be the last one.
There was a problem hiding this comment.
The formatter disagrees, I have made the change manually so hopefully it wont fail the formatter pre-commit check.
There was a problem hiding this comment.
Yeah, the formatter doesn't accept this so I am reverting it.
|
|
||
| SYCL_EXT_ONEAPI_FUNCTION_PROPERTY((syclexp::nd_range_kernel<1>)) | ||
| void scratch_kernel(float *src, float *dst) { | ||
| size_t lid = syclext::this_work_item::get_nd_item<1>().get_local_linear_id(); |
There was a problem hiding this comment.
Note that variable names should start with an upper-case letter.
maarquitos14
left a comment
maarquitos14
left a comment
There was a problem hiding this comment.
Last few nits! Otherwise LGTM.
sycl/test-e2e/FreeFunctionKernels/free_function_kernel_local_memory.cpp
Outdated
Show resolved
Hide resolved
sycl/test-e2e/FreeFunctionKernels/free_function_kernel_local_memory.cpp
Outdated
Show resolved
Hide resolved
sycl/test-e2e/FreeFunctionKernels/free_function_kernel_local_memory.cpp
Outdated
Show resolved
Hide resolved
sycl/test-e2e/FreeFunctionKernels/free_function_kernel_local_memory.cpp
Outdated
Show resolved
Hide resolved
sycl/test-e2e/FreeFunctionKernels/free_function_kernel_local_memory.cpp
Outdated
Show resolved
Hide resolved
…com/lbushi25/llvm into free_function_kernel_scratch_memory
|
Failures unrelated, see #19767 |
@intel/llvm-gatekeepers , Friendly ping for merge. |
…kernels (intel#19837) When a kernel uses implicit local memory such as by way of the `get_work_group_scratch_memory` function, the library is supposed to mark the kernel with the appropriate attribute `WORK_GROUP_STATIC_ATTR` to get things to work at runtime. This is done through the properties passed to the kernel invocation call. For free function kernels however, the infrastructure is not there to do this marking process and usage of the above mentioned function typically results in a UR error. This PR makes some changes at the middle-end level to traverse the call graph wherever the compiler built-in functions `__sycl_allocateLocalMemory` and `__sycl_dynamicLocalMemoryPlaceholder` are used and mark each of the kernels found during this traversal , including free function kernels, with the `WORK_GROUP_STATIC_ATTR` attribute if not already present.
…kernels (#19978) This is a cherry-pick of #19837 When a kernel uses implicit local memory such as by way of the `get_work_group_scratch_memory` function, the library is supposed to mark the kernel with the appropriate attribute `WORK_GROUP_STATIC_ATTR` to get things to work at runtime. This is done through the properties passed to the kernel invocation call. For free function kernels however, the infrastructure is not there to do this marking process and usage of the above mentioned function typically results in a UR error. This PR makes some changes at the middle-end level to traverse the call graph wherever the compiler built-in functions `__sycl_allocateLocalMemory` and `__sycl_dynamicLocalMemoryPlaceholder` are used and mark each of the kernels found during this traversal , including free function kernels, with the `WORK_GROUP_STATIC_ATTR` attribute if not already present. Patch-by: Lorenc Bushi <[email protected]>
4 participants
When a kernel uses implicit local memory such as by way of the
get_work_group_scratch_memoryfunction, the library is supposed to mark the kernel with the appropriate attributeWORK_GROUP_STATIC_ATTRto get things to work at runtime. This is done through the properties passed to the kernel invocation call. For free function kernels however, the infrastructure is not there to do this marking process and usage of the above mentioned function typically results in a UR error.This PR makes some changes at the middle-end level to traverse the call graph wherever the compiler built-in functions
__sycl_allocateLocalMemoryand__sycl_dynamicLocalMemoryPlaceholderare used and mark each of the kernels found during this traversal , including free function kernels, with theWORK_GROUP_STATIC_ATTRattribute if not already present.