Move driver and nvrtc cython and internal layers to new generator#1972
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
|
/ok to test |
|
/ok to test |
|
/ok to test |
1 similar comment
|
/ok to test |
|
/ok to test |
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
|
/ok to test |
|
/ok to test |
1 similar comment
|
/ok to test |
|
/ok to test |
It turns out the exception signature needs to match exactly when declaring function pointer types (which cuda-core does in one spot). cython-gen was generating |
|
@leofang: This is passing all the tests now. I think this is good to go (and let's merge the generator-side thing at roughly the same time, if we can). |
I assume that this also helps us avoid any potential (Cython) ABI breakage? |
|
I wish we have the ABI test running in the CI 😛 Eyeball'd as much as I can. Thanks, Mike! The CI flakiness should be fixed in #2195. I retried a few times here to make it green. |
leofang
left a comment
There was a problem hiding this comment.
Re-reviewing the symbol-loading rework end-to-end against main. Two file-anchored concerns below, plus one branch-state issue:
Missing CUDA 13.3 symbols. The following are declared and loaded in _bindings/cydriver.pyx.in on main (all at cudaVersion=13030) but absent from every new file in this PR (driver_linux.pyx, driver_windows.pyx, _internal/driver.pxd, cydriver.pyx):
cuLogicalEndpoint{Create,Destroy,AddDevice,BindAddr,BindMem,Unbind,Export,Import,Query,GetLimits,IdRelease,IdReserve}— 12 functionscuStreamBeginRecaptureToGraph— 1 function
Looks like the branch was regenerated before these landed on main. Calling any of them post-merge would be a hard breakage, so this needs a rebase + regen before going in. Not a design fix.
Cancelling my prior approval until that and the inline comments are addressed.
-- Leo's bot
It does. It's overly strict (it wouldn't have been a breakage in this specific case), but at least if we /really/ broke it we know Cython would tell us. |
|
@leofang: I have updated this to include the ptds version numbers where appropriate and also added ptds testing to a couple of the configurations in CI. |
Co-authored-by: Leo Fang <leof@nvidia.com>
|
@mdboom this regression is not yet addressed?
|
Sorry, I didn't see this comment yesterday. These have been added now. |
|
This is a continuation of the work in #1900. Now adds
driverto the mix and bothnvrtcanddriverare generated from the "real" new generator.