Add new launch property to support work_group_scratch_memory #1968
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kbenzie
reviewed
Nov 4, 2024
kbenzie
approved these changes
Nov 4, 2024
Naghasan
changed the title
Naghasan
force-pushed
the
work_group_static
branch
2 times, most recently
from
55f2d02 to
ae9804c
Compare
|
@oneapi-src/unified-runtime-cuda-write @oneapi-src/unified-runtime-level-zero-write please review, this is intended for the next release. |
Naghasan
force-pushed
the
work_group_static
branch
from
ae9804c to
44bcff4
Compare
frasercrmck
reviewed
Nov 19, 2024
frasercrmck
reviewed
Nov 19, 2024
frasercrmck
reviewed
Nov 19, 2024
Naghasan
force-pushed
the
work_group_static
branch
from
44bcff4 to
2afce8e
Compare
Naghasan
force-pushed
the
work_group_static
branch
2 times, most recently
from
a64d649 to
bc9eef9
Compare
|
ping @oneapi-src/unified-runtime-level-zero-write please review this is intended for the next release. |
Actually latest testing shows some related failures in the cuda build |
as far as I can tell it is because needs the DPC++ change (function signature change). The related PR on the dpc++ side is green, same reason for the level 0 failure. the other failures are unrelated to this patch (failure to fetch xpti) |
pbalcer
approved these changes
Nov 25, 2024
intel/llvm#15061 introduces a new property work_group_scratch_memory which allow the user to set a given amount of local memory to be used. In order to pass this information to the adaptor, the patch adds a new launch property to urEnqueueKernelLaunchCustomExp. The patch also changes the signature of urEnqueueKernelLaunchCustomExp to add global offset in order to maintain features when using this extension. Signed-off-by: Victor Lomuller <[email protected]>
Naghasan
force-pushed
the
work_group_static
branch
from
bc9eef9 to
7222f79
Compare
|
Hi @Naghasan, unfortunately I had to revert this PR in #2400 due to build failures in intel/llvm#15061 - I was looking at the wrong results when I hit merge. That said I'm not sure why the adapters built ok here but not in intel/llvm. I updated the tag there to point at |
|
@callumfare the build failure was due to the fact you haven't bumped the tag, only updated the comment ;) |
martygrant
added a commit
to intel/llvm
that referenced
this pull request
Dec 4, 2024
The patch partially implements `work_group_static` and update proposal. Implemented: - `work_group_static` to handle static allocation in kernel. - `get_dynamic_work_group_memory` to handle runtime allocation, but only on CUDA `work_group_static` is implemented by exposing `SYCLScope(WorkGroup)`, allowing the class to be decorated by the attribute and uses the same mechanism during lowering to place the variable in local memory. `get_dynamic_work_group_memory` uses a new builtin function, `__sycl_dynamicLocalMemoryPlaceholder `, which is lowered into referencing a 0 sized array GV when targeting NVPTX. The approach for SPIR will need to differ from this lowering. UR change oneapi-src/unified-runtime#1968, oneapi-src/unified-runtime#2403 --------- Signed-off-by: Lukas Sommer <[email protected]> Signed-off-by: Victor Lomuller <[email protected]> Co-authored-by: Lukas Sommer <[email protected]> Co-authored-by: Atharva Dubey <[email protected]> Co-authored-by: Marcos Maronas <[email protected]> Co-authored-by: Callum Fare <[email protected]> Co-authored-by: Martin Morrison-Grant <[email protected]>
KornevNikita
pushed a commit
to intel/llvm
that referenced
this pull request
Feb 25, 2025
The patch partially implements `work_group_static` and update proposal. Implemented: - `work_group_static` to handle static allocation in kernel. - `get_dynamic_work_group_memory` to handle runtime allocation, but only on CUDA `work_group_static` is implemented by exposing `SYCLScope(WorkGroup)`, allowing the class to be decorated by the attribute and uses the same mechanism during lowering to place the variable in local memory. `get_dynamic_work_group_memory` uses a new builtin function, `__sycl_dynamicLocalMemoryPlaceholder `, which is lowered into referencing a 0 sized array GV when targeting NVPTX. The approach for SPIR will need to differ from this lowering. UR change oneapi-src/unified-runtime#1968, oneapi-src/unified-runtime#2403 --------- Signed-off-by: Lukas Sommer <[email protected]> Signed-off-by: Victor Lomuller <[email protected]> Co-authored-by: Lukas Sommer <[email protected]> Co-authored-by: Atharva Dubey <[email protected]> Co-authored-by: Marcos Maronas <[email protected]> Co-authored-by: Callum Fare <[email protected]> Co-authored-by: Martin Morrison-Grant <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
intel/llvm#15061 introduce a new property
work_group_scratch_sizewhich allow the user to set a given amount of local memory to be used.In order to pass this information to the adaptor, the patch adds a new launch property to
urEnqueueKernelLaunchCustomExp.The patch also changes the signature of
urEnqueueKernelLaunchCustomExpto addglobal offsetin order to maintain features when using this extension.