Add descriptor-heap ConstantBuffer array-of-matrix regression test (#11483)#11484
Open
nv-slang-bot[bot] wants to merge 3 commits into
Open
Add descriptor-heap ConstantBuffer array-of-matrix regression test (#11483)#11484nv-slang-bot[bot] wants to merge 3 commits into
nv-slang-bot[bot] wants to merge 3 commits into
Conversation
…11483) #11483 reported that a ConstantBuffer<T> fetched via the spvDescriptorHeapEXT untyped-pointer path read wrong data for a nested float4x4[N] member; on a slightly newer build the same pattern crashed during SPIR-V emission. Both the crash and the wrong-data are no longer reproducible on current main: the crash was fixed by #11211 (which post-dates the reporting/triage builds), and the heap path now emits byte-identical std140/scalar layout to a bound [[vk::binding]] ConstantBuffer (verified by diffing decorations and by spirv-val). #11211 added descriptor-heap ConstantBuffer coverage but only for scalar struct elements; this adds the missing nested array-of-matrix case (the member the reporter saw fail), asserting the std140 offsets/ArrayStride/MatrixStride and that the descriptor-heap load yields a Uniform-class buffer pointer. Co-authored-by: Harsh Aggarwal <haaggarwal@nvidia.com>
Mirror the issue's struct (six leading float4x4, float4x4[4] at std140 offset 384, trailing float4 at 640) so the test pins the exact nested array-of-matrix offsets/strides that motivated closing #11483, and tie the stride CHECKs to the named array/matrix-wrapper types. Co-authored-by: Harsh Aggarwal <haaggarwal@nvidia.com>
Address APPROVE_WITH_NITS feedback on the regression test: - Add a -fvk-use-scalar-layout run line (CHECKSCALAR) pinning the `_natural` block the reporter's repro actually exercised, alongside the default std140 path. - Reference the captured %[[CB]]/%[[CBN]] block name in the negative CHECK-NOT instead of a literal id, matching the sibling tests. - Pin the matrices' RowMajor orientation beside MatrixStride. - Rewrite the header to state the test is compile + emitted-layout only (it does not reproduce the runtime readback symptom) and that bound-buffer parity was established by the triage diff, not in-file. Co-authored-by: Harsh Aggarwal <haaggarwal@nvidia.com>
Contributor
Author
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
1 participant
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.
Summary
Adds a GPU-free regression test for the descriptor-heap
ConstantBuffer<T>path(
spvDescriptorHeapEXT) covering a nested array-of-matrix member — the shapereported in #11483. No compiler change: investigation found the underlying defect
is already resolved on
mainby #11211; this fills the test-coverage gap that#11211 left.
Diagnosis
#11483 reports two symptoms for
getDescriptorFromHandle(ConstantBuffer<T>.Handle(uint2(i,0))):a hard SIGSEGV during SPIR-V emission, and (on builds that don't crash) wrong data
read back for a nested
float4x4 shadow_transforms[4]member.aaa5f89dd, "Fix crashwhen
ConstantBuffer<T>.Handleis used withspvDescriptorHeapEXT") is not anancestor of the triaged commit (
git merge-base --is-ancestor→ false). The crashis the pre-Fix crash when ConstantBuffer<T>.Handle is used with spvDescriptorHeapEXT (#11037) #11211 defect, which Fix crash when ConstantBuffer<T>.Handle is used with spvDescriptorHeapEXT (#11037) #11211 already fixes — not an incomplete fix.
main, the descriptor-heap path emits byte-identical internallayout to a bound
[[vk::binding]] ConstantBufferof the same struct — diffedOpDecorate/OpMemberDecorate(Offset/ArrayStride/MatrixStride/Block) in bothstd140 and
-fvk-use-scalar-layout. The only delta is the expected outerdescriptor-heap runtime-array stride. The module passes
spirv-val.Layout parity +
spirv-valfound no compiler emission/layout defect; with noGPU available they cannot fully exclude a runtime-only defect (e.g. descriptor
heap index/stride interaction at execution time), so final closure should ride on a
reporter retest on hardware.
Approach
#11211 added descriptor-heap
ConstantBuffertests, but only for scalar-structelements. The reporter's failing member is a nested
float4x4[N], which is laid outthrough std140 array/matrix wrapper types — a path none of the existing tests
exercise. This PR adds that coverage so a future regression in the nested-member
layout (or a return of the emission crash) is caught.
Files changed
tests/spirv/descriptor-heap-constant-buffer-array-of-matrix.slang(new test)Tests
New test compiles a compute entry point fetching
ConstantBuffer<CameraData>through the descriptor heap, where
CameraDatamirrors the reporter's struct (sixleading
float4x4,float4x4 shadowTransforms[4], trailingfloat4). It runsunder two layout configurations — the default std140 path and
-fvk-use-scalar-layout(the_naturalpath the reporter's repro actually used).FileCheck pins, for each:
Blockon the block type; member offsets 0/384/640; thematrix array's
ArrayStride 64; per-matrixMatrixStride 16+RowMajor; and theheap load producing a
Uniform-class buffer pointer consumed byOpBufferPointerEXT(never a
Function-class pointer — guarding against a copy lowering).Scope is compile + emitted layout only; it does not reproduce the runtime
wrong-data readback (no GPU in CI), and the bound-buffer parity noted in the
diagnosis is not re-diffed in-file — the test pins the descriptor-heap path's
absolute offsets/strides.
slang-test tests/spirv/descriptor-heap-constant-buffer-array-of-matrix.slang→ PASS (both std140 + scalar run lines)Risk
Test-only; no compiler source touched. Lowest-risk change class. The single
unverifiable gap is runtime behavior (no GPU), disclosed above.
Relates to #11483.