fix(core): reject use_libdevice on the NVRTC and PTX backends - #2573
Open
LeSingh1 wants to merge 1 commit into
Open
fix(core): reject use_libdevice on the NVRTC and PTX backends#2573LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
`ProgramOptions.use_libdevice` is documented "Only supported for the NVVM
backend", but the guard enforcing that was written in `Program_init`'s final
`else` -- the branch reached only when `code_type` is not a backend at all:
else:
supported_code_types = tuple(x.value for x in SourceCodeType)
if options.use_libdevice:
raise ValueError("use_libdevice is only supported by the NVVM backend")
raise RuntimeError(f"Unsupported {code_type=} ({supported_code_types=})")
So it never fires for the two real non-NVVM backends, and it fires for the
wrong reason on a typo:
* `Program(src, "c++", ProgramOptions(use_libdevice=True))` and the "ptx"
equivalent are accepted silently. `self._use_libdevice` is initialised to
False and only set inside the `nvvm` branch, so libdevice is never loaded
and the caller learns about it from undefined-symbol errors at link time
instead of from the documented up-front ValueError.
* `Program(src, "bogus", ProgramOptions(use_libdevice=True))` raises
"use_libdevice is only supported by the NVVM backend", which says nothing
about the code_type that is the actual problem.
Move the check next to the per-branch `extra_sources` guards, which are the
sibling option with the same "NVVM only" contract and are already duplicated
into the "c++" and "ptx" branches, and drop it from the `else` so an
unrecognised code_type reports itself.
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.
Problem
ProgramOptions.use_libdeviceis documented as "Only supported for the NVVM backend" (_program.pyx:465-468). The guard that was supposed to enforce that sits inProgram_init's finalelse— the branch reached only whencode_typeis not a backend at all:Two wrong behaviours follow:
1. The two real non-NVVM backends accept the option silently.
Program(src, "c++", ProgramOptions(use_libdevice=True))and the"ptx"equivalent construct fine.self._use_libdeviceis initialised toFalse(_program.pyx:765) and only flipped inside thenvvmbranch, so libdevice is never loaded — the caller finds out via undefined-symbol errors at link time instead of via the documented up-frontValueError.2. A typo'd
code_typereports the wrong error.Program(src, "bogus", ProgramOptions(use_libdevice=True))raises"use_libdevice is only supported by the NVVM backend"and never mentions thatcode_typeis the actual problem.The sibling option shows the intended shape:
extra_sourceshas the same "NVVM only" contract, and its guard is duplicated into each real branch —_program.pyx:772-773(c++) and:790-791(ptx).Fix
Move the
use_libdevicecheck next to thoseextra_sourcesguards in thec++andptxbranches, and drop it from theelseso an unrecognisedcode_typereports itself. No behaviour changes forcode_type="nvvm".This is an error-path-only change: any program that compiled before still compiles, and any program that is newly rejected was already producing a build the user did not ask for.
Tests
Three cases in
cuda_core/tests/test_program.py, placed next to the existingextra_sourcestests they mirror:test_cpp_program_with_use_libdevice— NVRTC rejects it.test_ptx_program_use_libdevice_unsupported— the PTX/linker path rejects it.test_program_init_invalid_code_type_reports_the_code_type—use_libdevice=Trueno longer shadows the unrecognised-code_typeerror.What I ran
Environment: macOS, no CUDA driver and no CUDA toolkit, so
cuda.corecannot be built or imported here.cuda_core/tests/— they need a builtcuda.coreand a GPU. They are written against the existingextra_sourcestests immediately adjacent to them, which use the same fixtures.Program_init's dispatch with the CUDA calls removed and the guard placement kept verbatim, before and after the change:ruff check/ruff format --checkoncuda_core/tests/test_program.py— clean, no new findings against amainbaseline for that file.use_libdeviceappears in_program.pyxonly at the two sites above (thenvvmset and this guard), so nothing else depended on the old placement.test_program_init_invalid_code_typestill passes unchanged because itsProgramOptionsleavesuse_libdeviceat itsFalsedefault.