[SYCL][PI][L0] Fixes to make sure kernels and programs cannot be destroyed before they have finished execution. #2710
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
Pull to update my local fork
updating fork from main
update fork to sycl head
Update my fork
Updating fork to head of sycl.
Update to sycl head.
…royed before the have finished execution.
smaslov-intel
suggested changes
Oct 30, 2020
|
I think I have addressed all your review comments and I have added the basic design notes in the original comment. I still need to add a test, but please review the revised code while I work on adding a test. Thank you. |
smaslov-intel
approved these changes
Nov 3, 2020
alexbatashev
pushed a commit
to alexbatashev/llvm
that referenced
this pull request
Nov 6, 2020
* upstream/sycl: [SYCL] Move tests with dependencies to on-device directory (intel#2732) [SYCL][Test] Remove leftovers for FPGA archives (intel#2735) [SYCL][NFC] Extend ABI tests to cover device code (intel#2725) [SYCL] Fix link to ESIMD tests (intel#2736) Added the SYCL_INTEL_mem_channel_property extension specification (intel#2688) [SYCL] Add support for new FPGA loop attribute nofusion (intel#2715) [SYCL] Remove host-task-dependency test added to llvm-test-suite (intel#2720) [SYCL] Remove warning about SYCL_EXTERNAL with pointers (intel#2722) [SYCL] dot_product support. (intel#2609) [SYCL][PI][L0] Fix a problem with kernels and programs destruction while they can be used (intel#2710) [SYCL] Fix the check for read-only host pointer during memobj creation (intel#2697)
kbenzie
pushed a commit
to kbenzie/intel-llvm
that referenced
this pull request
Feb 18, 2025
…pare_baseline add comparing to baseline in benchmark workflow
Chenyang-L
pushed a commit
that referenced
this pull request
Feb 18, 2025
…baseline add comparing to baseline in benchmark workflow
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.
So, here’s requirements from summary of level zero spec:
1 – cannot do zeKernelDestroy until all commands using the kernel have completed execution as measured by their event or fence being signaled.
2 – The application must destroy all kernel and build log handles created from the module before destroying the module itself (zeModuleDestroy)
So, here is a design I think will work.
1 – We want to do a piKernelRetain sometime before kernel is appended to command-list in piEnqueueKernelLaunch. The intent is that we will do a piKernelRelease once the Event that we create to be associated with the Enqueued Kernel is signaled/released. The handle of kernel to be released will be stored in the event.
2 – Event currently contains fields for CommandType and CommandData, and for the CommandType that piEnqueueKernelLaunch uses, CommandData is currently unused. So, for this case, we will set CommandData to be the Kernel, so then when the event is signaled as done, then we can use the kernel we stored in CommandData, we can do piKernelRelease(CommandData).
3 – I think in piKernelRetain, we also need to call piProgramRetain on the program that the kernel is part of, and we need piKernelRelease to call piProgramRelease. This is necessary to make sure we do not destroy the program before destroying all the kernels that may be in use from the program.
4 – in the destructor for pi_program, we need to destroy the build log before we destroy the program itself.