Skip to content

Add gpu_driven_amd with AMD GPU support #217

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

CalebZ9909
Copy link
Collaborator

Add AMD HIP for gpu_driven on AMD GPUs

@YangZhou1997
Copy link
Member

YangZhou1997 commented Jul 28, 2025

@CalebZ9909 Looks great. Can you put your code also in the gpu_driven folder so that we can track the diff and do the merging?

@YangZhou1997
Copy link
Member

YangZhou1997 commented Jul 29, 2025

Nice. I think my past wording was confusing. You should also do git rm -r gpu_driven_amd.

@YangZhou1997 YangZhou1997 requested a review from MaoZiming July 29, 2025 03:19
@CalebZ9909
Copy link
Collaborator Author

Yes, I am done on my side. Ziming and you can check the code now. I am working on testing the code and will update next version soon.

@MaoZiming
Copy link
Member

Thanks @CalebZ9909 , ideally we can define a flag (e.g. HIP_PLATFORM_AMD in rdma folder) so that we can toggle whether to run on AMD platform from the makefile. It should not overwrite all existing cuda commands with hip but keeping the option to toggle between the two

Maybe we can define some function name to either refer to the cuda version or the hip version depending on the macro. e.g. (either hipMemcpyPeerAsync or cudaMemcpyPeerAsync).

@YangZhou1997
Copy link
Member

@CalebZ9909
Copy link
Collaborator Author

Sounds good! Yes, I can set up a flag and keep all CUDA-related files along with AMD files, which can be chosen to be executed by the flag in the makefile.

@YangZhou1997
Copy link
Member

I guess a better way is to use the unified name like gpuMemcpy in https://github.com/uccl-project/uccl/blob/main/include/util/gpu_rt.h, so that we just need to write a new MakefileHip for AMD. You can find an example in https://github.com/uccl-project/uccl/tree/main/p2p

@CalebZ9909
Copy link
Collaborator Author

Sure, I will look into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants