-
Notifications
You must be signed in to change notification settings - Fork 937
accelerator/cuda: fix typos #12617
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
accelerator/cuda: fix typos #12617
Conversation
|
Running AWS CI |
|
Had to made a couple more adjustments. Requesting review from @devreal |
| result = cuMemcpyAsync((CUdeviceptr) dest, (CUdeviceptr) src, size, | ||
| GET_STREAM(&opal_accelerator_cuda_memcpy_stream.base)); |
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.
@devreal Does this change do what you intended it for?
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.
why do you need the GET_STREAM macro in this particular scenario? I think you can just use the opal_accelerator_cuda_memcpy_stream here, there is no ambiguity with the default stream (which is what the GET_STREAM macro was designed to do), isn't it?
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.
@edgargabriel I think you are right. Updated the PR.
Fix a couple minor typos to make compiler happy Signed-off-by: Wenduo Wang <[email protected]>
edgargabriel
left a comment
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.
LGTM
|
Merging the change for now. This is currently blocking CI on main branch. |
Fix a couple minor typos to make compiler happy