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.

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

[cuda] Port over allocator and buffer implementation #13985

Merged
merged 3 commits into from
Jun 8, 2023

Conversation

antiagainst
Copy link
Contributor

@antiagainst antiagainst commented Jun 7, 2023

This commit ports over existing CUDA driver allocator and buffer
implementation. The main logic is kept as-is, with one noticeable
changes--context wrapper is dropped and fields in it are directly
put in various API calls. This is to make supporting multiple
device and stream easier later. Other changes are just polishing
on comments and errors.

Progress towards #13245

@antiagainst
Copy link
Contributor Author

@benvanik: The first commit is just copying code. My modification happens in the second commit. I figured this way it might make the review easiser.

@benvanik
Copy link
Collaborator

benvanik commented Jun 7, 2023

Nice! If you could, I'd recommend taking #13440 as the base instead - that has some non-trivial changes that will be harder to port on top of your changes here without just deleting these files. You could replace your base commit doing the copy with the files from the PR and then try rebasing your second commit on top of it (or just find/replace again).

That PR is ready to land if we disabled the two tests suites testing graphs - the followup that hacks the memsets will happen tonight but neither should block you taking a copy of them.

@antiagainst
Copy link
Contributor Author

SG! Let me include the new changes you made there then.

@antiagainst
Copy link
Contributor Author

@benvanik: Switched to be based on your #13440.

This commit ports over existing CUDA driver allocator and buffer
implementation. The main logic is kept as-is, with one noticeable
changes--context wrapper is dropped and fields in it are directly
put in various API calls. This is to make supporting multiple
device and stream easier later. Other changes are just polishing
on comments and errors.
Copy link
Collaborator

@benvanik benvanik left a comment

Choose a reason for hiding this comment

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

style nits but otherwise lgtm!

experimental/cuda2/cuda_allocator.c Outdated Show resolved Hide resolved
experimental/cuda2/cuda_allocator.c Outdated Show resolved Hide resolved
experimental/cuda2/cuda_allocator.c Outdated Show resolved Hide resolved
experimental/cuda2/cuda_allocator.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@benvanik benvanik left a comment

Choose a reason for hiding this comment

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

style nits but otherwise lgtm!

@antiagainst antiagainst requested a review from benvanik June 8, 2023 01:15
@antiagainst antiagainst merged commit 1299742 into iree-org:main Jun 8, 2023
40 checks passed
NatashaKnk pushed a commit to NatashaKnk/iree that referenced this pull request Jul 6, 2023
This commit ports over existing CUDA driver allocator and buffer
implementation. The main logic is kept as-is, with one noticeable
changes--context wrapper is dropped and fields in it are directly
put in various API calls. This is to make supporting multiple
device and stream easier later. Other changes are just polishing
on comments and errors.

Progress towards iree-org#13245
nhasabni pushed a commit to plaidml/iree that referenced this pull request Aug 24, 2023
This commit ports over existing CUDA driver allocator and buffer
implementation. The main logic is kept as-is, with one noticeable
changes--context wrapper is dropped and fields in it are directly
put in various API calls. This is to make supporting multiple
device and stream easier later. Other changes are just polishing
on comments and errors.

Progress towards iree-org#13245
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hal/cuda Runtime CUDA HAL backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants