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: make cuGetProcAddress to work with cuda 12 #8763

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

shamisp
Copy link
Contributor

@shamisp shamisp commented Dec 12, 2022

What

CUDA: Adding support for CUDA-12

@shamisp
Copy link
Contributor Author

shamisp commented Dec 12, 2022

@Akshay-Venkatesh can you please take a look ? Thanks

Copy link
Contributor

@Akshay-Venkatesh Akshay-Venkatesh left a comment

Choose a reason for hiding this comment

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

@shamisp changes look fine but we probably ought to consolidate code which gets split based on version. There are other instances similar to these where fxn signature changes but we should probably have a common function so that caller doesn't see all the ugly bits.

@shamisp
Copy link
Contributor Author

shamisp commented Dec 12, 2022

@Akshay-Venkatesh In a longer term this makes sense but it is much more substantial overhaul of CUDA UCT. Majority of the ifdef flows were introduced in 11070 and I didn't want to change this.

@Akshay-Venkatesh
Copy link
Contributor

@Akshay-Venkatesh In a longer term this makes sense but it is much more substantial overhaul of CUDA UCT. Majority of the ifdef flows were introduced in 11070 and I didn't want to change this.

Agreed that we should not make that overhaul in this PR.

Minor nitpick: the title of this PR is deceiving. Shouldn't it just be "make cuGetProcAddress work with cuda 12"?

@shamisp shamisp changed the title CUDA: Adding support for CUDA-12 CUDA: make cuGetProcAddress work with cuda 12 Dec 12, 2022
@shamisp
Copy link
Contributor Author

shamisp commented Dec 12, 2022

@Akshay-Venkatesh In a longer term this makes sense but it is much more substantial overhaul of CUDA UCT. Majority of the ifdef flows were introduced in 11070 and I didn't want to change this.

Agreed that we should not make that overhaul in this PR.

Minor nitpick: the title of this PR is deceiving. Shouldn't it just be "make cuGetProcAddress work with cuda 12"?

agreed

@shamisp shamisp changed the title CUDA: make cuGetProcAddress work with cuda 12 CUDA: make cuGetProcAddress to work with cuda 12 Dec 13, 2022
@shamisp
Copy link
Contributor Author

shamisp commented Dec 13, 2022

@Akshay-Venkatesh can you please approve so I can merge it once it is ready ? thanks !

brminich
brminich previously approved these changes Dec 14, 2022
@yosefe
Copy link
Contributor

yosefe commented Dec 14, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

yosefe
yosefe previously approved these changes Dec 14, 2022
@yosefe
Copy link
Contributor

yosefe commented Dec 14, 2022

@shamisp can you pls squash?

@shamisp
Copy link
Contributor Author

shamisp commented Dec 15, 2022

@yosefe DONE. The failures are unrelated: fatal error: error writing to /tmp/ccn6FDdP.s: No space left on device

@yosefe yosefe merged commit a5039ee into openucx:master Dec 16, 2022
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