-
Notifications
You must be signed in to change notification settings - Fork 608
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] Move to hal/drivers and wire up BUILD files #14620
Conversation
ff8ae89
to
fadf0c8
Compare
option(IREE_HAL_DRIVER_CUDA "Enables the 'cuda' runtime HAL driver" ${IREE_HAL_DRIVER_CUDA_DEFAULT}) | ||
option(IREE_HAL_DRIVER_CUDA2 "Enables the 'cuda2' runtime HAL driver" ${IREE_HAL_DRIVER_CUDA2_DEFAULT}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we come up with a plan for switching from the old cuda
to the new cuda2
? Maybe post an RFC and an update to #13245, and could link to one or both of those here.
Some ideas:
- Drive to feature complete
- All CTS tests and execution tests we care about pass using
cuda2
- Figure out the story for flags like on Disable CUDA memset emulation when cuGraphAddMemsetNode fix is deployed. #13984
- All CTS tests and execution tests we care about pass using
- Switch default in-repo
- All in-repo benchmarks using
cuda2
, without egregious regressions
- All in-repo benchmarks using
- Delete (old)
cuda
and renamecuda2
tocuda
Ideally we'd have a timeline in mind so we don't stay in this state of having two similar paths for long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SG. I've commented on #13245 with #13245 (comment). I'll send out a mail later too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard for me to give a precise timeline though; given I'm multiplexing over quite a few work streams. But yeah agreed we don't want to be in a hybrid state for too long; I'm hoping to get feature parity down soon.
c2a2284
to
5dcb81f
Compare
a5c70a0
to
5009f18
Compare
5009f18
to
c17e227
Compare
5903d84
to
104cbcd
Compare
This commit moves the CUDA HAL driver rewrite to the `hal/drivers` directory given it's functional and ready for normal usage. By this we can start run tests with CI to make sure it does not regress. Further improvements can happen directly in this directory. This provides an easy route for trying out the rewrite before eventually replace the existing HAL driver. Along the way wired up BUILD configurations.
104cbcd
to
f441834
Compare
Hi @benvanik, I have addressed all known issues so this is ready for another review now. After moving I'd be switching the tests/benchmarks to use the new impl to test out. |
woo excited to get rid of the old impl! |
This commit moves the CUDA HAL driver rewrite to the
hal/drivers
directory given it's functional and ready fornormal usage. By this we can start run tests with CI
to make sure it does not regress. Further improvements
can happen directly in this directory.
This provides an easy route for trying out the rewrite before
eventually replace the existing HAL driver.
Along the way wired up BUILD configurations.
Progress towards #13245