Uh oh!
There was an error while loading. Please reload this page.
cuda.core: return CUmodule via as_py in ObjectCode.get_module - #2481
Merged
Conversation
Contributor
Use the shared handle export path for legacy CUmodule interop instead of constructing driver.CUmodule directly. Signed-off-by: Jinfeng <jinfengl@nvidia.com>
Satisfy cython-lint after switching get_module() to as_py(). Signed-off-by: Jinfeng <jinfengl@nvidia.com>
lijinf2
commented
Aug 3, 2026
ContributorAuthor
/ok to test a4c1e46 |
This comment has been minimized.
This comment has been minimized.
isVoid
reviewed
Aug 3, 2026
| // Regular pointer (CUmodule), not a shared_ptr: lifetime is owned by the | ||
| // CUlibrary it was retrieved from. | ||
| inline PyObject* as_py(const CUmodule& h) noexcept { | ||
| return detail::make_py("cuda.bindings.driver", "CUmodule", reinterpret_cast<std::intptr_t>(h)); |
Contributor
There was a problem hiding this comment.
We should extend the overload for as_intptr(h) for CUmodule above?
Route as_py(CUmodule) through as_intptr for consistency with other handle exports.
lijinf2
commented
Aug 3, 2026
ContributorAuthor
/ok to test edb609d |
isVoid
reviewed
Aug 4, 2026
| // Regular pointer (CUmodule), not a shared_ptr: lifetime is owned by the | ||
| // CUlibrary it was retrieved from. | ||
| inline std::intptr_t as_intptr(const CUmodule& h) noexcept { | ||
| return reinterpret_cast<std::intptr_t>(h); |
Contributor
There was a problem hiding this comment.
Yeah.. I think the agent is not quite smart here. There's a pretty obvious pattern here: as_cu -> as_intptr -> as_py, but the agent isn't smart enough to pick it up. Also the comment is not really needed here.
ContributorAuthor
There was a problem hiding this comment.
Yes, agents chose not to revise as_cu and as_intptr and it needs clear instructions in the prompt.
Just revised as_cu to support CUModule.
lijinf2
commented
Aug 4, 2026
ContributorAuthor
/ok to test 869760b |
isVoid
approved these changes
Aug 4, 2026
lijinf2
enabled auto-merge (squash)
August 4, 2026 19:00
lijinf2
commented
Aug 5, 2026
ContributorAuthor
/ok to test ef8d8ab |
Uh oh!
There was an error while loading. Please reload this page.
|
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use the shared handle export path for legacy CUmodule interop instead of constructing driver.CUmodule directly.
Description
closes#2415
Add an
as_py(const CUmodule&)overload and use it inObjectCode.get_module()so legacyCUmoduleexport goes through the same handle→Python wrapper path as other CUDA resources.CUmodulefromcuLibraryGetModuleis not an owning*Handle(shared_ptr); its lifetime is owned by the parentCUlibrary, so this overload takes the regular pointer and only wraps it ascuda.bindings.driver.CUmodule.Checklist