fix(core): reject use_libdevice on the NVRTC and PTX backends - #2573
fix(core): reject use_libdevice on the NVRTC and PTX backends#2573LeSingh1 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.
| mentioned that the code_type was the actual problem. | ||
| """ | ||
| with pytest.raises(RuntimeError, match=r"^Unsupported code_type='fortran'"): | ||
| Program("goto 100", "FORTRAN", ProgramOptions(arch="sm_80", use_libdevice=True)) |
There was a problem hiding this comment.
Regex can be :
match=r"^Unsupported code_type='fortran' \(supported_code_types=\('c\+\+', 'ptx', 'nvvm'\)\)$"
| if options.extra_sources is not None: | ||
| raise ValueError("extra_sources is not supported by the PTX backend.") | ||
| if options.use_libdevice: | ||
| raise ValueError("use_libdevice is not supported by the PTX backend.") |
There was a problem hiding this comment.
It seems deprecation is a much more stable option than directly removing .
if code_type == "c++":
assert_type(code, str)
if options.extra_sources is not None:
raise ValueError("extra_sources is not supported by the NVRTC backend (C++ code_type)")
if options.use_libdevice:
warnings.warn(
"use_libdevice is only supported by the NVVM backend; it is ignored "
"on the NVRTC and PTX backend and will raise ValueError in a future release.",
DeprecationWarning,
stacklevel=3,
)
There was a problem hiding this comment.
codex (in addition to the findings posted under this comment):
Good point. Although the documented NVVM-only contract makes an immediate error defensible as a bug fix, I agree that warning is the safer behavior for 1.x. The ignored behavior was explicitly codified before 1.0 by test_make_program_cache_key_use_libdevice_ignored_for_non_nvvm.
I’d mirror #2658: emit a visible UserWarning for both code_type="c++" and "ptx" and continue ignoring the option. I would not use DeprecationWarning, because ProgramOptions.use_libdevice itself is not deprecated—it remains supported for NVVM. We should still remove the misplaced check from the unknown-code_type branch so the invalid code type remains the primary error.
If we decide these combinations should eventually raise, our support policy puts that change at 2.0.0, and we should state that version explicitly. Also, agreed on tightening the regex to the full expected message.
|
I asked codex gpt-5.6-sol ultra for a full review, this is what it found: Current HEAD commit: 69df389 The dispatch fix is sensible, but immediate rejection conflicts with released behavior and misses another public API surface. Findings
Current |
3 participants
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.