You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Building off of #8281, this PR adds a native lowering via SPV_EXT_descriptor_heap and SPV_KHR_untyped_pointers and is part 1/3 in a series.
ResourceDescriptorHeap and SamplerDescriptorHeap are lowered to untyped variables decorated with ResourceHeapEXT and SamplerHeapEXT. Each heap access emits OpUntypedAccessChainKHR into a runtime array of the appropriate descriptor type. Buffer-like resources (StructuredBuffer, ByteAddressBuffer, ConstantBuffer, TextureBuffer) use OpTypeBufferEXT and OpBufferPointerEXT; image and sampler resources use OpLoad. Interlocked operations on RWTexture use OpUntypedImageTexelPointerEXT.
Jonathan Zakharov (@jzakharovnv) I think the OpConstantSizeOfEXT usage in this MR isn't quite right - it looks like you're just taking the stride of the descriptor type being loaded and using that? This works for the sampler heap where there's only one type (samplers), but not for the resource heap.
I might be misunderstanding the code, but I don't see anything for finding the largest of the two sizes? They're both POT, so it is a simple "which is max" calculation, but I don't see it being done in this PR.
Diego Novillo (@dnovillo) In response to your last comment on this PR, yes I think it's more correct to have the TODO commit a part of this initial branch rather than one down the line. Cherrypicked and rebased to reflect this.
This looks like a user-visible bug fix/feature change. Please add (or point to) the corresponding entry in docs/ReleaseNotes.md.
If release-note coverage is planned in a related PR (including one that hasn’t been submitted yet), please mention that plan/link so we can avoid duplicate notes.
Fixes#8603: MergeBinaryOpSelect produced an OpSelect with a vector
result type and a scalar condition, which is only legal in SPIR-V 1.4+
(KhronosGroup/SPIRV-Tools#6827).
This exposes #8740 where the resource-heap-ext-texture.hlsl test fails
validation - for now this is worked around by disabling the test, since
#8517 looks like it is reworking it anyway.
Assisted-by: copilot
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b04b21af-2d8b-4af5-b9ad-1b84073588f5
Normalize chained assignment RHS values for heap aliasing
tools/clang/lib/SPIRV/SpirvEmitter.cpp:8540
This predicate misses valid value-producing expressions such as chained assignments. For RWTexture2D<uint> a, b; a = b = ResourceDescriptorHeap[0];, the inner assignment returns the heap handle, but the outer RHS is a BinaryOperator, so a is marked bound and never receives heap-index metadata; an atomic through a then falls back to invalid OpImageTexelPointer. Buffer aliases similarly fall through the normal whole-resource path. Recursively normalize assignment RHS values (or reject these forms explicitly) before classification and alias propagation.
Correct references to image alias regression tests
These references point to the wrong filenames: alias-return.hlsl covers buffer returns and explicitly excludes image aliases, while the image atomic regressions are in the two .error.hlsl files. Point readers to the actual image return and parameter tests.
Track heap-sourced images returned from function calls
tools/clang/lib/SPIRV/SpirvEmitter.cpp:5307
isHeapSourcedValue does not recognize a call that returns a heap-sourced image. Consequently, RWTexture2D x = makeHeapImage(); records x as Bound, and a later x = ResourceDescriptorHeap[i] is rejected as bound/heap mixing even though both sources are heap-backed. Function-returned image handles are explicitly supported for ordinary reads/writes, so this rejects an otherwise supported reassignment; classify heap-returning calls (and their forwarded locals) separately from genuinely bound resources here.
Preserve heap provenance through conditional image selection
tools/clang/lib/SPIRV/SpirvEmitter.cpp:8598
Valid HLSL heap provenance is not limited to a direct subscript or DeclRefExpr; conditional selection is commonly used (for example, RWTexture2D t = cond ? ResourceDescriptorHeap[a] : ResourceDescriptorHeap[b]). This returns false for that expression, so buffer resources fall back to storing OpBufferPointerEXT values through an ordinary function variable, while an image atomic later has no descriptor index and falls back to invalid OpImageTexelPointer. Track/merge both heap indices through conditionals, or diagnose this form before lowering it.
Scan nested declaration contexts for heap image callers
tools/clang/lib/SPIRV/SpirvEmitter.cpp:8715
This caller search skips function bodies nested in record/namespace declaration contexts. HLSL supports member functions, so a member can load an RWTexture from ResourceDescriptorHeap and pass it to a free function whose parameter is used by an interlocked operation; that call site is never found here, and the callee falls back to the invalid OpImageTexelPointer path this check is intended to prevent. Traverse nested declaration contexts (or a TU-wide call graph) so every user-function body is scanned.
// No call graph is available, so finding param's callers means walking
// every function body in the TU for a CallExpr targeting owner.
// Shallow scan of TU decls, dont visit methods on user-defined types.
for (const Decl *d : astContext.getTranslationUnitDecl()->decls()) {
const auto *caller = dyn_cast<FunctionDecl>(d);
if (!caller || !caller->getBody())
This branch has not been deployed
No deployments
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
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.
Diego Novillo (dnovillo)
left a comment
Building off of #8281, this PR adds a native lowering via SPV_EXT_descriptor_heap and SPV_KHR_untyped_pointers and is part 1/3 in a series.
ResourceDescriptorHeap and SamplerDescriptorHeap are lowered to untyped variables decorated with ResourceHeapEXT and SamplerHeapEXT. Each heap access emits OpUntypedAccessChainKHR into a runtime array of the appropriate descriptor type. Buffer-like resources (StructuredBuffer, ByteAddressBuffer, ConstantBuffer, TextureBuffer) use OpTypeBufferEXT and OpBufferPointerEXT; image and sampler resources use OpLoad. Interlocked operations on RWTexture use OpUntypedImageTexelPointerEXT.
Requires -fspv-target-env=vulkan1.3.
Assisted by an AI agent.
Diego Novillo (@dnovillo)