Skip to content

Move driver and nvrtc cython and internal layers to new generator#1972

Merged
mdboom merged 28 commits into
NVIDIA:mainfrom
mdboom:driver-v2
Jun 12, 2026
Merged

Move driver and nvrtc cython and internal layers to new generator#1972
mdboom merged 28 commits into
NVIDIA:mainfrom
mdboom:driver-v2

Conversation

@mdboom

@mdboom mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

This is a continuation of the work in #1900. Now adds driver to the mix and both nvrtc and driver are generated from the "real" new generator.

@copy-pr-bot

copy-pr-bot Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot added the cuda.bindings Everything related to the cuda.bindings module label Apr 24, 2026
@mdboom

mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

1 similar comment
@mdboom

mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions github-actions Bot added CI/CD CI/CD infrastructure cuda.core Everything related to the cuda.core module labels Apr 24, 2026
@mdboom

mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions

This comment has been minimized.

@leofang leofang self-requested a review April 24, 2026 23:59
@leofang leofang added this to the cuda.bindings 13.3.0 & 12.9.7 milestone Apr 24, 2026
@mdboom

mdboom commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

1 similar comment
@mdboom

mdboom commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

LGTM overall, @mdboom it seems the build fails at cythonization again 😛

Yeah, my agent found something easy, but it just fixed Python 3.15 by breaking everyone else ;) Need to work deeper.

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 except ?CUDA_ERROR_NOT_FOUND. cybind generates except ?_CURESULT_INTERNAL_LOADING_ERROR. The frustrating thing is that these are equivalent values. (There is a line _CURESULT_INTERNAL_LOADING_ERROR = CUDA_ERROR_NOT_FOUND)... But Cython can't see through that. The solution I arrived at was to add another override so cybind will generate CUDA_ERROR_NOT_FOUND for driver only. (Since we have to make cuda-core work for cuda-bindings from both before and after this PR). Yet another special case for these "legacy" core libraries, but not too bad, I suppose.

@mdboom mdboom requested a review from leofang June 10, 2026 18:32
@mdboom

mdboom commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@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).

@leofang

leofang commented Jun 10, 2026

Copy link
Copy Markdown
Member

It turns out the exception signature needs to match exactly when declaring function pointer types (which cuda-core does in one spot).

I assume that this also helps us avoid any potential (Cython) ABI breakage?

@leofang

leofang commented Jun 10, 2026

Copy link
Copy Markdown
Member

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 leofang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 functions
  • cuStreamBeginRecaptureToGraph — 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

Comment thread cuda_bindings/cuda/bindings/_internal/driver_linux.pyx Outdated
Comment thread cuda_bindings/cuda/bindings/_internal/driver_linux.pyx Outdated
@mdboom

mdboom commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

It turns out the exception signature needs to match exactly when declaring function pointer types (which cuda-core does in one spot).

I assume that this also helps us avoid any potential (Cython) ABI breakage?

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.

@github-actions github-actions Bot added the CI/CD CI/CD infrastructure label Jun 11, 2026
@mdboom

mdboom commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

@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.

@mdboom mdboom requested a review from leofang June 11, 2026 20:40
@mdboom mdboom enabled auto-merge (squash) June 11, 2026 20:47
@leofang

leofang commented Jun 11, 2026

Copy link
Copy Markdown
Member

@mdboom this regression is not yet addressed?

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 functions
  • cuStreamBeginRecaptureToGraph — 1 function

@mdboom

mdboom commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@mdboom this regression is not yet addressed?

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 functions
  • cuStreamBeginRecaptureToGraph — 1 function

Sorry, I didn't see this comment yesterday. These have been added now.

@mdboom mdboom merged commit c128381 into NVIDIA:main Jun 12, 2026
200 of 202 checks passed
@github-actions

Copy link
Copy Markdown
Doc Preview CI
Preview removed because the pull request was closed or merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD CI/CD infrastructure cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P0 High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants