Consider the NVRTC major version when deciding whether cubin is available - #2564
Open
LeSingh1 wants to merge 1 commit into
Open
Consider the NVRTC major version when deciding whether cubin is available#2564LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
…able
_, nvrtc_minor = nvrtc.version()
use_cubin = nvrtc_minor >= 1
nvrtc.version() returns (major, minor) and the major is discarded, so the
test is really "is the minor at least 1". That is wrong for every x.0
release:
(11, 0) -> False correct
(11, 1) -> True correct
(12, 0) -> False WRONG
(12, 9) -> True correct
(13, 0) -> False WRONG
The intent is "NVRTC is new enough to emit cubin" (nvrtcGetCUBIN arrived in
CUDA 11.1). On a 12.0 or 13.0 toolkit the check silently selects the
--gpu-architecture=compute_XX + PTX path instead of sm_XX + cubin, so the
examples JIT-compile PTX at load time rather than using the cubin they asked
for. Every example in this repo carries a cuda-bindings>13.2.1 PEP-723
header, so a 13.0 toolkit is a realistic configuration.
Compare (major, minor) as a tuple. In common.py this becomes a named
nvrtc_supports_cubin() helper so the decision table can be unit tested
without an NVRTC install; jit_program.py, which does not import the example
helpers, gets the equivalent inline comparison.
Adds tests/test_kernel_helper.py covering the table above, including a case
that pins the exact disagreement between the old and new expressions.
Contributor
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
nvrtc.version()returns(major, minor)and the major is discarded, so the test isreally "is the minor at least 1". That is wrong for every
x.0release:nvrtc.version()nvrtc_minor >= 1(11, 0)False(11, 1)True(12, 0)False(12, 9)True(13, 0)FalseThe intent is "NVRTC is new enough to emit cubin" —
nvrtcGetCUBINarrived in CUDA 11.1.On a 12.0 or 13.0 toolkit the check silently selects
--gpu-architecture=compute_XX+nvrtc.get_ptx()instead ofsm_XX+nvrtc.get_cubin(), so the examples JIT-compile PTX at module-load time rather than usingthe cubin they asked for. It is a silent fallback, not a crash — which is why it has gone
unnoticed. Every example here carries a
cuda-bindings>13.2.1PEP-723 header, so a13.0toolkit is a realistic configuration.
Both copies of the expression are affected:
cuda/bindings/_example_helpers/common.py:58-59(used byKernelHelper, i.e. mostexamples) and
examples/extra/jit_program.py:71-72.Fix
Compare
(major, minor)as a tuple against(11, 1).In
common.pythis becomes a namednvrtc_supports_cubin()helper, so the decision tableis unit-testable without an NVRTC install.
jit_program.pydoes not import the examplehelpers, so it gets the equivalent inline comparison plus a comment rather than a new
dependency.
I kept the 11.1 threshold exactly as-is — this PR is only about the major being ignored,
not about re-deciding where cubin support starts.
Test
New
cuda_bindings/tests/test_kernel_helper.pycovers the full table above, plus a casethat pins the exact disagreement between the old and new expressions for
(12, 0)and(13, 0). Pure Python — no NVRTC, no toolkit, no GPU.ruff checkandruff format --checkare clean on all three files.