Only request hlsl_nvapi capability where the NVAPI Slang module is linked - #1088
Only request hlsl_nvapi capability where the NVAPI Slang module is linked#1088nv-slang-bot[bot] wants to merge 1 commit into
Conversation
…nked SlangSession::create_session requested the hlsl_nvapi Slang capability unconditionally. The sgl NVAPI Slang module that provides it is created and linked into programs only under SGL_HAS_NVAPI && DeviceType::d3d12, so the capability request was strictly broader than the module backing it. (The separate SGL_ENABLE_NVAPI compiler define, which shader code branches on, is already gated on that same predicate.) Slang 2026.12, the currently pinned version, tolerates the mismatch silently. shader-slang/slang#11225 rejects it: error[E36121]: requested capability 'hlsl_nvapi' is incompatible with compilation target 'spirv' Gate the request on the predicate already used at the module create and link sites so they cannot drift apart. The non-d3d12 device types this function maps to a target -- spirv, metal, wgsl, ptx and host-callable -- have no NVAPI module linked; of those, only spirv is covered by a job in the dispatch matrix that surfaced this, so that is the only target observed failing. Both halves of the predicate are load-bearing. SGL_HAS_NVAPI is OFF wherever slang-rhi does not build its NVAPI target -- which requires Windows, x86_64 and a D3D backend -- and there the module is never created or linked. Where it is ON, the capability is genuinely backed for d3d12 but not for other device types, so the device check is still required. Fixes #1087
998aeb2 to
1dc014b
Compare
Affected-Slang verification result — slangpy#1087 / PR #1088Result: the fix is verified against affected Slang. Without the guard, 28 × Head is now Toolchain provenance (the positive control, run first)Local Slang built from source at The change is present in the source: commit Positive control on the binary — this directly confirms the affected diagnostic is compiled into the | library |
Result — three arms, all at the compiler levelShader: a trivial
Arm 1 is the load-bearing one and it reproduces the exact CI error verbatim. Arm 2 shows suppressing What this does and does not establishEstablishes: the requested capability is the sole cause of E36121 in this minimized SPIR-V Arm 3 partially relaxes a caveat I previously published — and only the compiler-target half of it. It does not establish d3d12 device creation, runtime NVAPI linkage, that the Windows CI d3d12 subcases Not claimed: metal / wgsl / ptx / host-callable. Diagnostic suppression is expected by construction (the guard excludes them), but their runtime and layout behaviour is unverified — see the capability-set note in the PR body, which bounds one way removing the raw entry can affect those targets specifically. Integration arm — slangpy built against the affected SlangThe compiler arms above isolate causation; this exercises the actual Without the guard (the fix commit reverted, i.e. upstream 28 × The failure arithmetic closes: 33 failed = 28 (E36121) + 5 (the pre-existing Git LFS asset failures So the bug is reproduced locally through slangpy's own code path, not merely inferred from CI logs. With the guard (commit Zero Summary of the A/B
The 28 recovered cases equal the 28
The runtime-resolved library was checked too, not just the one on disk:
|
D3D12 CI has now closed the caveat aboveThe Verification section said the guard's Windows job ( Both clauses of The flags alone only make that arm possible; what shows it is reached is the test harness. #if SGL_WINDOWS
std::vector<DeviceType> device_types{DeviceType::d3d12, DeviceType::vulkan};
...
for (DeviceType device_type : device_types) {
SUBCASE(enum_to_string(device_type).c_str())So on Windows every non-skipped 200/200, zero Two honest notes on reading this:
Full CI on What remains open is unchanged and unaffected by this: the 🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify. |
SlangSession::create_sessionrequested thehlsl_nvapiSlang capability unconditionally, while the sgl NVAPI Slang module that provides it is created and linked only underSGL_HAS_NVAPI && DeviceType::d3d12. The capability request was therefore strictly broader than the module backing it.Slang 2026.12 (the currently pinned version) tolerates the mismatch silently. shader-slang/slang#11225 rejects it:
This is a latent SlangPy bug that #11225 surfaced — #11225 itself needs no change.
The fix
Gate the request on the predicate already used at the module-create (
shader.cpp:250) and module-link (:659) sites, so the guards cannot drift apart.SGL_ENABLE_NVAPI— the compiler define consumed bynvapi.slang— is already gated on that same predicate in this function.Both halves of the predicate are load-bearing. Note the evidence for each differs, and neither comes from the local A/B (which short-circuits — see Caveats):
SGL_HAS_NVAPIThe device clause is required because
SGL_HAS_NVAPIalone is insufficient: the Windows job has itONand still fails on vulkan. The availability clause is required for consistency with the module create (:250) and link (:659) sites — whereSGL_HAS_NVAPIisOFFthe module is never built, so the capability has nothing behind it on any device.Why one site is sufficient
Re-derived from the raw job logs of run 30779697035:
hlsl_nvapiis the only capability named in anyE36121— 56 occurrences total, 28 per job — andspirvthe only target. No other capability, no other target format.vulkan, on both platforms. Each failure prints aDEEPEST SUBCASE STACK REACHEDblock whose following lines name the subcase; enumerating those tokens across both logs (after stripping the Windows log's CRLF line endings) givesvulkan28× andd3d120× per job.{d3d12, vulkan}against linux's{vulkan}(tests/sgl/testing.cpp:73), which is consistent with d3d12 subcases executing but does not independently prove it — platform-specific assertions could also account for the difference.Had a second capability appeared, a broader audit of capability passing would have been required instead. It does not, so this is the single fix site — confirmed by the A/B below.
Verification
Against the pinned Slang 2026.12 — no NVAPI or shader-session regression observed on the change branch. Built
linux-gccwithSGL_HAS_NVAPI: OFF,SGL_HAS_D3D12: OFF,SGL_HAS_VULKAN: ON;unit-test-cpp195/200 with assertions 15529/15529, 0 failed, and zero nvapi errors.pytest slangpy/tests/device/test_shader_cursor.py test_shader.py: 18 passed. The harness instantiated realvulkanandcudadevices (adapterNVIDIA L40S), so the run was not GPU-skipped — this is not a claim that CUDA-target behaviour was verified.pre-commit run --all-filesexits 0 with no hook failures.The 5 failing cases (
dds_file/*,texture_loader_stream_*) are asset-loading failures: the 22.ddsfiles underdata/test_images/are Git LFS pointer files (filereports ASCII text; contents beginversion https://git-lfs.github.com/spec/v1) whose payloads were not materialized in this container. They are apparently unrelated to shader sessions, but the base commit was not built, so their baseline status was not verified.Because the pin predates #11225, this run shows only that the change introduces no observable regression on the tested paths; it does not reproduce
E36121.Against affected Slang (slang#11225) — verified. Built Slang from source at
pull/11225/head(v2026.14.1-24-gdb61cec) and rebuilt slangpy against it viaSGL_LOCAL_SLANG=ON, the mechanism CI uses.E36121mainbehaviour)The 28 recovered cases equal the 28
E36121occurrences exactly; all 28 are the identicalhlsl_nvapi/spirvpair, matching CI's 28-per-job.33 = 28 + 5, the 5 being the LFS asset failures above. Nothing else differed between arms.Three
slangcarms isolate the cause more sharply than the integration run can:slangc -target spirv -capability hlsl_nvapierror[E36121] ... 'hlsl_nvapi' ... target 'spirv'slangc -target spirvslangc -target dxil -profile sm_6_6 -capability hlsl_nvapiSo the capability request is the cause, suppressing it on non-DXIL targets fixes it, and it remains valid for the DXIL target the guard still requests it for.
A positive control ran before either arm:
strings … | grep -c 'is incompatible with compilation target'gives 2 for the source build and 0 for theslang-2026.14.1-linux-x86_64release artifact that the build downloads forslang-llvm. Testing against the latter would have returned clean on both arms — and the trap is self-concealing, since2026.14.1is newer than the pinned2026.12yet predates the unmerged #11225. Version-newer is not change-inclusive when the change is unmerged.Caveats, stated explicitly
Verified at the compile-target level; not at the device level.
SGL_HAS_D3D12: OFFhere, so the guard'struebranch never executed throughSlangSession::create_session. E36121 is raised byTargetRequest::checkCapabilities()keyed on the target, with no device term, which is why thedxil + hlsl_nvapiarm above validates the pairing thetruebranch depends on. Still unverified: d3d12 device creation, runtime NVAPI linkage, and Windows d3d12 subcase execution — D3D12 CI on this PR is what would establish those.What the A/B validates is the mechanism, not the choice of predicate.
src/sgl/CMakeLists.txt:392emitsSGL_HAS_NVAPIas a literal0/1, and it is0on this host — so the guard short-circuits at the first clause and thedevice_typecomparison is never evaluated. On Linux the patch is therefore behaviourally identical to deleting the capability request outright, or to guarding onSGL_HAS_NVAPIalone. The arm establishes that not requestinghlsl_nvapiis what fixes the SPIR-V failures — the causal claim — while the specific predicate is justified by matching the three existing NVAPI guard sites (:250,:508,:659) plus the Windows CI evidence. Per-clause attribution would need a host withSGL_HAS_NVAPI: ON.Only spirv is observed failing; there is no metal/CUDA/CPU job in the dispatch matrix that surfaced this. Diagnostic suppression on those targets follows by construction (the guard excludes them), but their runtime and layout behaviour is unverified — see the capability-set note below.
The logs also carry
error[E39999](import failed) andfatal error[E40003](compilation ceased). Both are cascade wrappers emitted immediately after eachE36121, not independent root causes.One non-diagnostic side effect of removing the raw capability entry, bounded. Slang's
slang-type-layout.cpp:3549iterates the rawCompilerOptionName::Capabilityarray to computespecificCapabilityRequested, and:3559-3563addsCapabilityName::descriptor_handleiff!specificProfileRequested && !specificCapabilityRequested. The promotion itself calls onlyaddUnexpandedCapabilites+setTargetCaps, i.e. it directly mutates the target capability set; downstream layout and lowering code then consults that set, so the effect is not confined to diagnostics. Dropping the entry could therefore flip that boolean and change the cooked capability set.That flip cannot occur on the targets involved here.
specificProfileRequestedis true whenever a real profile is set, and slangpy sets one only for d3d12 and vulkan (shader.cpp:452-454, whoseSGL_CHECKalso rejectsSLANG_PROFILE_UNKNOWN);TargetDesc.profileotherwise defaults toSLANG_PROFILE_UNKNOWN. So on vulkan — where all 28 failures reproduced —specificProfileRequestedis already true,descriptor_handlewas never being added, and removing the capability entry changes nothing in this auto-promotion path. On d3d12 the real profile likewise prevents auto-promotion; and where NVAPI is available the guard also retains the request.For metal / cuda / wgpu / cpu the profile stays
UNKNOWN, so those are the only sessions where dropping the entry can flipspecificCapabilityRequestedand re-enabledescriptor_handleauto-promotion. This matters under the pinned 2026.12, not only under #11225:maybePromoteDescriptorHandleCapabilityis present in the pinned release too (slang-type-layout.cpp:3519), and because this fix is deliberately pin-independent it reaches users well before #11225 merges. The correct risk baseline is therefore the pinned compiler, not the post-#11225 world.Tested against the pinned
slangc2026.12 with a shader storing through aDescriptorHandle<RWStructuredBuffer<float>>in aParameterBlock. What I measured was the compilation result and the diagnostics. Generated layout, emitted code and struct offsets were not compared, so any downstream layout or lowering effect of the capability-set change is untested.All four of
ptx,metal,cpp,wgslcompile successfully (rc=0) both with and without the stray entry. One diagnostic difference appears, and it goes the helpful way: onptxthe stray entry produces an extrawarning[E41012]: profile implicitly upgraded ... 'cuda_sm_2_0', which the guard removes. That warning also appears without anyDescriptorHandlein the shader, so it is caused by the stray capability itself rather than by promotion — and slangpy disables E41012 by default (shader.cpp:346), so a default session suppresses it — thoughenable_warningsis applied afterwards (:363-364), so a user who re-enables it could see the difference.This is evidence of no adverse change for this shader shape under the pinned compiler, not a proof of equivalence for all shaders, and it is untested on a real target device. Worth a follow-up issue rather than expanding this PR.
On guard consistency — all four runtime sites agree
Since the Linux A/B validates the mechanism rather than the predicate (see the caveat above), consistency with the existing NVAPI guards carries much of the justification for the predicate itself. That argument holds cleanly. Every runtime NVAPI condition in this file uses the identical two-clause form:
:250if (SGL_HAS_NVAPI && m_device->type() == DeviceType::d3d12)— module create:406if (SGL_HAS_NVAPI && device_type == DeviceType::d3d12)— this fix:508(SGL_HAS_NVAPI && m_device->type() == DeviceType::d3d12) ? "1" : "0"—SGL_ENABLE_NVAPIdefine:659if (SGL_HAS_NVAPI && m_device->type() == DeviceType::d3d12)— module linkThe one NVAPI condition without a device term,
#if SGL_HAS_NVAPIat:510-517(NV_SHADER_EXTN_SLOTand the NVAPI include path), is not a drifted guard — it answers a different question.#ifis a compile-time availability gate ("were NVAPI headers built into this binary at all"), whereas the four sites above are per-session runtime decisions. Its contents don't need the device term: the-Iargument is addressed to"dxc", which only runs for HLSL/DXIL, andNV_SHADER_EXTN_SLOThas no in-repository consumer (grepfinds only the define site — the NVAPI headers reached via that include path may consume it, but they too are only reachable on the DXIL path).A runtime
ifcould of course be nested inside that block, so this is a statement about what the site needs, not about what C++ permits. Adding one would change what is passed on Windows + vulkan sessions — a configuration this container cannot exercise (SGL_HAS_NVAPI: OFF, no D3D12) — so it is left alone rather than folded into a PR whose claims are currently verified.Sequencing
.github/workflows/ci-latest-slang.yml'sbuild-prjob checks out slangpy with noref:(:94-97), so slangpy is always built from its default branch;client_payloadcontrols only the slang ref.SlangPy Testson slang#11225 therefore stays red until this guard is onmain— an open PR will not flip it.SlangPy Testsis also not bot-rerunnable; a human must re-dispatch after merge.This does not make anything urgent: #11225 is currently unmerged and unapproved, so this guard is necessary-but-not-sufficient for it, and nothing here is racing a ready-to-merge change.
No
SGL_SLANG_VERSIONbump is bundled. slangpy pulls a release tarball rather than a SHA, and #11225 is in no release, so that change is two gates out — merge #11225, wait for a tag containing it, then bump the pin. Separate PR.Summary
E36121without the guard, 0 with it); no observed regression against the pinned Slang.Fixes #1087hlsl_nvapiwas requested for targets that cannot satisfy it; gating on the existing NVAPI predicate resolves it at the single site responsible, demonstrated by before/after against affected Slang.truebranch, and broader shader/device coverage of thedescriptor_handlecapability-set question.Fixes #1087
🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.