Fix D3D12 device removal when zeroing read-only (shader_resource) tensors (#1079)

[nv-slang-bot]

Summary

• Status: Fix implemented; builds and all targeted tests pass on Vulkan + CUDA. D3D12 root cause source-confirmed; on-device verification pending HW (no D3D12/Metal hardware in this CI/dev environment — the device-removal itself only reproduces on D3D12).
• Link: Fixes Arrays of tensor parameters: incorrect results on Metal, device removal on D3D12 #1079 (Defect 2, D3D12 device removal). Stacked on Add tests for arrays of Tensor, RWTensor, DiffTensor, RWDiffTensor #1078 (which adds the repro tests).
• Verdict: Root cause is Tensor::clear() issuing a UAV zeroing op that D3D12 rejects (removing the device) on a buffer created without the unordered_access flag. Per maintainer review, fixed by rejecting the misuse: clear() now raises a clean exception on read-only storage (clearing is a write and requires a writable buffer), so no invalid UAV op is issued.
• Next-action: Maintainer review; on-device D3D12 confirmation (device should no longer be removed; the D3D12-skipped read tests can then be re-enabled).
• Blocker: None for the fix. Metal wrong-results (Defect 1 of Arrays of tensor parameters: incorrect results on Metal, device removal on D3D12 #1079) is intentionally out of scope here and tracked separately.

The bug

Tensor::clear() (src/sgl/func/tensor.cpp) called clear_buffer() unconditionally. On the D3D12 RHI backend clear_buffer is implemented via a UAV (ClearUnorderedAccessViewUint), which requires the buffer to have been created with the unordered_access usage flag. A Tensor created with usage=BufferUsage.shader_resource only (no UAV) — which is the correct, required usage for read-only tensor arrays — therefore failed CreateUnorderedAccessView, triggering RemoveDevice (DXGI_ERROR_INVALID_CALL) and poisoning the entire shared D3D12 device. Vulkan and CUDA implement the clear via transfer ops (no UAV requirement), which is why the failure was D3D12-specific.

Tensor::clear() is reached by Tensor.zeros, Tensor.zeros_like, and with_grads gradient-buffer zeroing, so a guard at that single method covers all three paths.

Note: adding the unordered_access bit to make the clear succeed is not a valid fix — it flips the tensor's derived writability, renaming its element type Tensor<...> → RWTensor<...>, which breaks array type-resolution (element types are compared by full-name string).

The fix

Tensor::clear() now raises a clean exception when its storage lacks unordered_access:

Cannot clear a read-only tensor: clearing is a write operation and requires storage created with BufferUsage::unordered_access.

Clearing is inherently a write, so requiring a writable buffer is the natural contract, and rejecting the misuse before any GPU work means the invalid UAV op is never issued — the D3D12 device is no longer removed. This also directly satisfies the issue's second ask (a read-only-tensor misuse should surface as a clean exception, not a device loss). The approach follows maintainer review on the PR (a copy-path zeroing alternative was rejected as too expensive — a full-size host allocation + upload-heap staging + GPU copy per clear).

Consequence: Tensor.zeros(usage=shader_resource) / zeros_like on read-only storage now raise too (consistent with "zeroing is a write"). A read-only tensor that needs initial contents is built with Tensor.empty(...) + copy_from_numpy(...) (a copy, not a UAV op — valid on read-only storage).

Tests

slangpy/tests/slangpy_tests/test_array.py:

• test_clear_read_only_tensor_raises_not_removes_device — Tensor.zeros(shader_resource) raises cleanly (not a device loss), and the device stays usable afterward.
• test_clear_writable_tensor_via_command_encoder — clear(cmd) on a writable tensor zeroes it through a caller-supplied encoder (the ordered path).
• test_read_only_tensor_to_writable_param_raises_cleanly / test_array_of_read_only_tensors_to_rwtensor_array_raises_cleanly — read-only→writable binding is rejected at resolution with a clean ResolveException (scalar and array); device stays alive.
• The two read repro tests (test_array_of_tensors_read, test_array_of_difftensors_read) construct their read-only inputs via Tensor.empty(shader_resource) + copy_from_numpy (the previous zeros zero-init was overwritten anyway).

All pass on Vulkan + CUDA. pre-commit run clean.

Review history

An earlier revision of this PR zeroed read-only storage via a copy-path upload; maintainer review (@ccummingsNV) flagged that as too expensive for large tensors and preferred raising an exception, which this revision implements. Also peer-reviewed by a coworker + independent codex passes.

Scope

Defect 1 of #1079 (Metal wrong results for arrays-of-tensors and tensor-in-struct) is a separate root cause (likely upstream Slang Metal codegen for the plain-array subset; possibly slangpy marshalling / PR #1045 for the tensor-in-struct subset) and is intentionally not addressed here. #1079 should remain open until both backends are green and the per-backend skips in test_array.py are re-enabled.

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}

[ccummingsNV]

I don't think we can do this - we're effectively saying that a fairly innocent 'clear' function asked to clear a 1GB tensor has to:

• allocate 1GB of heap memory
• set it 0
• allocate another 1GB in the upload heap internally
• copy memory into it
• free the first 1GB
• perform buffer copy on gpu
• free the upload heap memory

All to set it 0 :)

You probably could achieve this in D3D12 by creating an aliased UAV resource and clearing that.

My preference to be honest, unless there's a real demand for this, is to raise an exception saying 'To clear a tensor it must be writable'. I don't see that as unreasonable - clearing is an inherently write-operation. If the user has allocated a read-only tensor and then asked to write to it, it's valid to call that an error.

[nv-slang-bot]

You're right — thanks, that's a fair objection. The copy path turns a "clear" into a full-size host allocation + an upload-heap staging copy + a GPU copy, which is a bad trade for large tensors, and it's the wrong shape for what clear should cost.

I'll pivot to your preferred approach: Tensor::clear() raises when the storage isn't writable (buffer lacks unordered_access) — message along the lines of "cannot clear a read-only tensor; clearing is a write operation and requires unordered_access usage." That also happens to satisfy the original issue's second ask directly (a read-only-tensor misuse should surface as a clean SlangPyError/exception rather than removing the D3D12 device), so the device-removal is fixed by not issuing the UAV clear at all, before any GPU work.

One consequence worth flagging: Tensor::clear() is reached by Tensor.zeros/zeros_like (and with_grads(zero=True)), so Tensor.zeros(usage=shader_resource) will now raise too — which is consistent with your framing (zeroing is a write). The read-only repro tests in this stack construct their inputs with Tensor.zeros(shader_resource) then immediately copy_from_numpy(real_data), so I'll switch those to Tensor.empty(shader_resource) + copy_from_numpy (the zero-init was being overwritten anyway; copy_from_numpy is a copy, not a UAV op, so it works on read-only storage). A read-only tensor that genuinely needs to be all-zeros can still be built with empty + copy_from_numpy(np.zeros(...)).

I'll leave the aliased-UAV-resource D3D12 trick aside unless you'd prefer it — the exception is simpler and, unless there's real demand for clearing read-only tensors, it's the right default.

Pushing the revised diff shortly (revert the copy path → writable-check exception; update the repro + regression tests; rebuild + re-run VK/CUDA).

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}

[nv-slang-bot]

Pushed the revised approach (1424924): Tensor::clear() now raises when the storage lacks unordered_access instead of zeroing via a copy path — "Cannot clear a read-only tensor: clearing is a write operation and requires storage created with BufferUsage::unordered_access." The invalid UAV clear is never issued, so the D3D12 device removal is gone, and this also gives the issue's ask-#2 the clean exception (rather than device loss).

As flagged, Tensor.zeros(usage=shader_resource) / zeros_like now raise too (they clear at creation). The two read repro tests build their read-only inputs via Tensor.empty(shader_resource) + copy_from_numpy instead; regression tests assert the clean RuntimeError on both zeros(shader_resource) and an explicit clear() of a read-only tensor, plus device-survival.

test_array.py 44 passed / 2 skipped (pre-existing #255) / 0 failed on Vulkan+CUDA; test_tensor.py 78 passed / 4 skipped; pre-commit clean. Left the aliased-UAV D3D12 trick out per your steer. The D3D12 device-removal itself is still unverifiable here (no D3D12 HW on Linux) — a D3D12 CI/Windows run remains the empirical confirmation.

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}

[ccummingsNV]

Better, but appears to be a lot of tests that have been disabled or set to use uninitialized data, rather than just to use a writable tensor. Unless the test is explicitly designed to validate read-only tensor behaviour, it is better that we use the correct tensor type than to remove or make less robust the test

[ccummingsNV]

This comment can be a lot shorter - 1 line is enough!

[ccummingsNV]

This 'device must still be alive check is overkill' - if every test that could kill the device did that we'd have 1000s of dummy tensors made. We should remove it from these tests.

[ccummingsNV]

Any reason we don't just use a writable tensor?

[jhelferty-nv]

@nv-slang-bot Do you have an answer to Chris' question? I'm not necessarily asking for a change, but let's start by understanding the decision-making here.

[nv-slang-bot]

Short answer: these read tests deliberately exercise the read-only Tensor<float,1>[4] parameter shape, and a writable tensor can't bind to that parameter. Switching to a writable tensor would either fail to resolve (against the read-only Tensor[] param) or — if we also flipped the parameter to RWTensor[] — just duplicate the existing test_array_of_rwtensors_write. The empty() + copy_from_numpy construction is what lets us build a read-only input without tripping the very check this PR adds.

The detail (resolution paths under slangpy/builtin/):

A tensor marshall encodes its access in the Slang type name — writable → RWTensor<float,1>, read-only → Tensor<float,1> (tensor.py builds slang_type with access = read_write if writable else read). Resolution then treats the scalar and array cases differently:

• Scalar Tensor param (tensorcommon.py, the ITensorType branch): only the read-only → writable direction is rejected (if bound_type.writable and not self.writable: raise "Can't pass a read-only tensor to a writable tensor"). The opposite is adapted — the result type is rebuilt with access = bound_type.access — so a writable tensor can bind to a read-only scalar Tensor param.
• Array Tensor[] param (array.py, ArrayMarshall.resolve_types): elements are matched by full_name string (st.element_type.full_name == bound_type.full_name), and array_to_array_scalarconvertable only adapts scalar element types / unknown-generic / unknown-resource elements — there is no access adaptation on the array element path. So a writable input array (RWTensor<float,1>[4]) doesn't match a read-only Tensor<float,1>[4] parameter by name, and resolution fails with a ResolveException.

The reverse of that mismatch has an explicit regression test in this PR — test_array_of_read_only_tensors_to_rwtensor_array_raises_cleanly asserts ... does not match slang type RWTensor<float, 1>[4] — the same name-based array matching, just in the other direction.

So for test_array_of_tensors_read / test_array_of_difftensors_read the input must be read-only to match the parameter. Before this PR the natural way to make one was Tensor.zeros(shader_resource), but that is exactly what this PR now makes raise (zeroing goes through clear(), and a clear on read-only storage is the D3D12 device-removal we're fixing). Tensor.empty(shader_resource) allocates without clearing, and copy_from_numpy is a copy — valid on read-only storage, no UAV op — so it fills the input without a clear.

If you'd prefer these two to test the writable path instead, the change is: parameter → RWTensor<float,1>[4], inputs → writable Tensor.zeros(...). That's a valid test, but it is already what test_array_of_rwtensors_write covers, and we'd lose coverage of the read-only array-parameter shape (a distinct resolution path). Happy to consolidate that way if you'd rather — otherwise the intent here was to keep the read-only array-param path under test.

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}

[ccummingsNV]

Can't we make this test work by making the tesnors writable - it seems like a valuable test to have.

[ccummingsNV]

Why not just make tensor writable below + let this test work?

[nv-slang-bot]

Addressed the round-2 comments (pushed f9b1b14):

• Shorter clear() comment — reduced to one line.
• Removed the "device must still be alive" checks — dropped the survivor-tensor assertions from the clear/resolution regression tests (agreed, overkill).
• test_array_of_tensors_read / test_array_of_difftensors_read — removed the D3D12 skip (the read-only inputs now use Tensor.empty(), which does not clear, so the device-removal trigger is gone). These two keep read-only inputs deliberately: their whole point is to validate the read-only Tensor<float,1>[4] / DiffTensor<float,1>[4] parameter shape. A writable input can't be used here — a writable tensor resolves to RWTensor<...>, and the array path matches element types by name, so RWTensor does not resolve against a read-only Tensor[] parameter (verified locally: it raises a ResolveException). Making them writable would require changing the parameter to RWTensor[], which just duplicates the existing RW write tests and stops testing the read-only array shape. The RW write tests already use writable tensors.

Note this is exactly the "explicitly designed to validate read-only tensor behaviour" case your review body carves out, so empty(shader_resource) is the correct type rather than a downgrade — I've added a one-line comment on each saying so.

test_array.py 44 passed / 2 skipped (pre-existing #255) / 0 failed on Vulkan+CUDA; pre-commit clean. Let me know if you'd still prefer these two converted to RWTensor[] write-style tests instead — happy to, it just changes what they cover.

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}

The base branch was changed.

[nv-slang-bot]

CI triage for the build (windows, x86_64, msvc, Release, 3.10) failure at c61d279 (the merge of main into this branch):

This is a flaky unit test, not a build failure and unrelated to this PR's change. The build itself succeeded; the job failed on a single C++ doctest assertion (1 of 20,980):

tests/sgl/device/test_profiler.cpp(534): CHECK( trace->query_zones("exhausted gpu", gpu)->count() == 1 )
values: CHECK( 0 == 1 )
TEST CASE: "GPU query exhaustion preserves CPU zones" (vulkan)
[doctest] 263 cases | 262 passed | 1 failed | 7 skipped

Signals that point to a flake rather than a regression:

• This PR's net diff is 2 files — slangpy/tests/slangpy_tests/test_array.py and src/sgl/func/tensor.cpp. Neither touches the profiler; the change can't affect a GPU-query-pool timing test.
• Release-only, 11/12 builds green — Windows MSVC Debug and every other platform passed; only Windows MSVC Release hit it.
• The same job is green on latest main (run 32155856004), so it isn't a deterministic break in the merged content either.
• The profiler suite has a documented history of intermittent async-collector/timing failures (Profiler can finalize a global frame before consuming attached cross-thread zones #1072, Fix profiler collector finalizing frames before consuming their zones (#1072) #1073, Temporarily disable intermittent profiler tests #1076, Re-enable profiler tests disabled for collector race #1077).

I've re-run the failed job. If it recurs, the flake likely warrants its own tracking issue against the profiler tests (same family as #1077) rather than a change here.

_{🤖 Generated by an automated SlangPy coworker — may be inaccurate. A human maintainer should verify.}