-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL][ROCm] Add HIP NVIDIA support #4049
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
Conversation
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.
Changes in buildbot/configure.py and sycl/doc/GetStartedGuide.md look good to me.
@malixian, FYI. |
@smaslov-intel, @intel/llvm-reviewers-runtime, ping. |
@npmiller, how can I run I would expect that there should be CMake target like this for running lit tests with ROCm plug-in on AMD/NVIDIA HW. Right? |
This is not wired up for the ROCm backend yet. It's definitely something we need however this PR was already getting pretty massive, so I was thinking about adding it in a follow up PR. It'll take a bit of time but if you prefer I can add it in here instead. |
Separate PR sounds good to me. I just wanted to make sure that there is some way to test the ROCm backend. Thanks! |
Just curious. @npmiller would it make sense to build rocm plugin for both platforms at the same time and present them as different backends? |
It could be, although I think that might make the CMake more awkward and require a bunch of glue in the C++ as well, like all the Ultimately the NVIDIA support through ROCm is mostly useful for debugging and testing at the moment, for running actual applications on NVIDIA GPUs the CUDA backend should definitely be preferred, HIP goes right back to CUDA anyway. And building both the CUDA and ROCm backend (for either platform) works fine, so you can have a build that can target both platforms already. I guess being able to build both of the platforms at the same time would make it easier to validate that a patch doesn't break building either, but overall I'm not too sure it's worth the extra complexity in the plugin or in the CMake. |
@smaslov-intel, @intel/llvm-reviewers-runtime, ping. |
Co-authored-by: vladimirlaz <[email protected]>
The double quotes around AMD and NVIDIA were incorrect, they were getting processed as part of the string thus breaking the string equality check.
@smaslov-intel, ping. |
I approve the changes as is, but would prefer these be broken into multiple change-sets. |
I'm not sure why these were left to `1` but this patch fixes some of the tests in [oneAPI-DirectProgramming ](https://github.com/zjin-lcf/oneAPI-DirectProgramming), such as the matrix multiply and mandelbrot samples. With this patch the samples are now giving correct results on both AMD GPU, and on Nvidia GPU with the ROCm backend (using #4049).
This PR enables building the PI ROCm plugin for NVIDIA rather than AMD, has ROCm has some NVIDIA support.
It should allow doing some testing of the ROCm PI plugin on NVIDIA hardware.
This PR includes:
--rocm-platform
flagWe are still running some testing on this, however it looks to be similar to the current ROCm plugin on AMD, and a bit better on NVIDIA since the SYCL compiler for it is more mature.