Add tgt_target_kernel_async and helper - #212862
Conversation
|
Hello @ZuseZ4 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
jdoerfert
left a comment
jdoerfert
left a comment
There was a problem hiding this comment.
I thought we had helpers to create and sync async info objects already, did I imagine that?
| } | ||
|
|
||
| struct __tgt_async_info_handle { | ||
| int64_t DeviceId; |
There was a problem hiding this comment.
An AsyncInfoTy has the device ID and you probably don't need another type to wrap it at all.
| return; | ||
|
|
||
| Handle->AsyncInfo.synchronize(); | ||
| delete Handle; |
|
|
||
| handleTargetOutcome(Rc == OFFLOAD_SUCCESS, Loc); | ||
| return Rc; | ||
| } |
There was a problem hiding this comment.
This function looks copied, can we reuse the original instead?
2 participants
On the Rust side, we want to have more control over how we launch our kernel.
The implicit sync at the normal
__tgt_target_kernellaunch caused a 6x perf overhead on a set of (shorter) kernels which we were benchmarking. Rust usage: rust-lang/rust#156620Wip till I get to add tests/fix codestyle/etc.
cc @jdoerfert @kevinsala
Disclaimer: I used an LLM for the interface.cpp changes