Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[CUDA][EXP] CUDA adapter support for command buffers #932

Merged
merged 4 commits into from
Oct 31, 2023
Merged

[CUDA][EXP] CUDA adapter support for command buffers #932

merged 4 commits into from
Oct 31, 2023

Conversation

Bensuo
Copy link
Contributor

@Bensuo Bensuo commented Oct 6, 2023

  • Implement initial subset of command buffer functions for CUDA
  • Report command buffer extension as supported from device query
  • Some additional methods and refactoring to reuse existing adapter code

@Bensuo
Copy link
Contributor Author

Bensuo commented Oct 6, 2023

Linked DPC++ PR: intel/llvm#11133

@Bensuo Bensuo mentioned this pull request Oct 6, 2023
Merged
jchlanda
jchlanda reviewed Oct 11, 2023
View reviewed changes
Copy link
Contributor

@jchlanda jchlanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an in depth view of this functionality, so my review is mostly style/nit, apologies.

source/adapters/cuda/command_buffer.cpp Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.hpp Outdated Show resolved Hide resolved
ldrumm
ldrumm requested changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@ldrumm ldrumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know anything about command buffers, so my review is in the small. I'll leave someone else to think about the higher level flow

source/adapters/cuda/command_buffer.hpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.hpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.hpp Show resolved Hide resolved
source/adapters/cuda/command_buffer.hpp Outdated Show resolved Hide resolved
source/adapters/cuda/device.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/enqueue.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/enqueue.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/enqueue.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Outdated Show resolved Hide resolved
@Bensuo Bensuo requested a review from a team as a code owner October 13, 2023 13:59
@Bensuo Bensuo force-pushed the cuda-cmd-buffers branch 3 times, most recently from f30f960 to 44cb034 Compare October 16, 2023 09:07
@Bensuo Bensuo requested a review from ldrumm October 16, 2023 12:09
@Bensuo Bensuo force-pushed the cuda-cmd-buffers branch 3 times, most recently from 0f0dd7d to b70280f Compare October 17, 2023 15:50
@Bensuo
Copy link
Contributor Author

Bensuo commented Oct 18, 2023

@ldrumm @jchlanda If you're happy with the changes could you please approve the PR?

@kbenzie kbenzie added the ready to merge Added to PR's which are ready to merge label Oct 20, 2023
@Bensuo
[CUDA][EXP] CUDA adapter support for command buffers
74f42f8 
- Implement initial subset of command buffer functions for CUDA
- Report command buffer extension as supported from device query
- Some additional methods and refactoring to reuse existing adapter code
@Bensuo Bensuo requested a review from a team as a code owner October 23, 2023 09:27
@EwanC
Copy link
Contributor

EwanC commented Oct 30, 2023 •
edited
Loading

Based on the CI fail after merging in tip adapters branch, think we need to update our PR with the equivalent changes from #961 to cuda/command_buffer.cpp

EDIT: I've pushed this change

@kbenzie kbenzie merged commit be53fb3 into oneapi-src:adapters Oct 31, 2023
48 checks passed
steffenlarsen pushed a commit to intel/llvm that referenced this pull request Nov 3, 2023
@mfrancepillois @EwanC
@julianmi @Bensuo @kbenzie
[SYCL][Graph] Add support for CUDA backend (#11133)
367b662 
Adds support for enabling CUDA backend with Sycl-Graph:

- Enables Graph e2e tests with CUDA backend.

Companion PR with CUDA UR adapter changes is here:
oneapi-src/unified-runtime#932

---------

Co-authored-by: Ewan Crawford <ewan@codeplay.com>
Co-authored-by: Julian Miller <julian.miller@intel.com>
Co-authored-by: Ben Tracy <ben.tracy@codeplay.com>
Co-authored-by: Kenneth Benzie (Benie) <k.benzie83@gmail.com>
Co-authored-by: Maxime France-Pillois <maxime.francepillois@codeplay.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbenzie kbenzie kbenzie left review comments

@mfrancepillois mfrancepillois mfrancepillois approved these changes

@ldrumm ldrumm ldrumm approved these changes

@jchlanda jchlanda jchlanda approved these changes

@npmiller npmiller Awaiting requested review from npmiller

@JackAKirk JackAKirk Awaiting requested review from JackAKirk

Assignees
No one assigned
Labels
ready to merge Added to PR's which are ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Bensuo @EwanC @jchlanda @kbenzie @ldrumm @mfrancepillois