From 0244c96d637f47fa264d441a82d3dca78889373b Mon Sep 17 00:00:00 2001 From: ArielG-NV <159081215+ArielG-NV@users.noreply.github.com> Date: Fri, 16 May 2025 11:57:17 -0700 Subject: Fix correct bindings for bindless resource model [SPIRV and GLSL] (#7131) Fix correct bindings for bindless resource model [spirv and glsl] fixes: #6952 Problem: * Currently all bindless objects are placed in the same set (fine) and same binding (incorrect behavior for vulkan). This is incorrect since as per [spec](https://registry.khronos.org/vulkan/specs/latest/man/html/VkDescriptorType.html), only 1 resource type may be written to each index inside a set (these rules are loosened with VK_EXT_mutable_descriptor_type) * This means currently generated bindings do not work in practice if we (for example) use `Sampler2D.Handle` and `Texture1D.Handle` in a shader since we would place 2 incompatible objects in the same binding-index and set. Solution: * `__getDynamicResourceHeap` was modified to allow bindings to chosen dynamically for a descriptor * use `IOpaqueDescriptor` to check compile-time information of resource types so that we can identify different resources * Using this information of `IOpaqueDescriptor`, we modify `defaultGetDescriptorFromHandle` to provide a binding model (1 resource per binding-index) which produces legal spirv/glsl. * To support `VK_EXT_mutable_descriptor_type` the function `defaultGetDescriptorFromHandle` has a set of options (`BindlessDescriptorOptions`) for a user to pick-from to support their binding model. Capabilities are not used here for flexibility purposes (specifically old shaders mixed with modern vulkan extensions). Other changes: * Added `TexelBuffer` DescriptorKind to aid in generating correct bindings * format code * Add to docs bindless changes, make AccelerationStructure use its handle directly, adjust tests accordingly --------- Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com> --- source/slang/hlsl.meta.slang | 110 ++++++++++++++++++++- .../slang/slang-ir-lower-dynamic-resource-heap.cpp | 16 ++- 2 files changed, 119 insertions(+), 7 deletions(-) (limited to 'source') diff --git a/source/slang/hlsl.meta.slang b/source/slang/hlsl.meta.slang index 99f83776e..57cb188bb 100644 --- a/source/slang/hlsl.meta.slang +++ b/source/slang/hlsl.meta.slang @@ -22251,6 +22251,7 @@ enum DescriptorKind Buffer, /// A buffer descriptor. Sampler, /// A sampler state descriptor. AccelerationStructure, /// A ray tracing acceleration structure descriptor. + TexelBuffer /// A texel buffer descriptor. } enum DescriptorAccess @@ -22303,7 +22304,9 @@ extension _Texture; - static const DescriptorKind kind = isCombined != 0 ? DescriptorKind.CombinedTextureSampler : DescriptorKind.Texture; + static const DescriptorKind kind = (Shape.flavor == __ShapeBuffer.flavor ? + DescriptorKind.TexelBuffer : (isCombined != 0 ? DescriptorKind.CombinedTextureSampler : DescriptorKind.Texture) +); static const DescriptorAccess descriptorAccess = (DescriptorAccess)access; __implicit_conversion($(kConversionCost_ImplicitDereference)) @@ -22422,8 +22425,36 @@ __prefix T operator*(DescriptorHandle value) return getDescriptorFromHandle(value); } +// https://registry.khronos.org/vulkan/specs/latest/man/html/VkDescriptorType.html +enum DefaultVkBindlessBindings : uint +{ + Sampler = 0, /// SAMPLER + CombinedTextureSampler = 1, /// COMBINED_IMAGE_SAMPLER + Texture_Read = 2, /// SAMPLED_IMAGE + Texture_ReadWrite = 3, /// STORAGE_IMAGE + TexelBuffer_Read = 4, /// UNIFORM_TEXEL_BUFFER + TexelBuffer_ReadWrite = 5, /// STORAGE_TEXEL_BUFFER + Buffer_Read = 6, /// UNIFORM_BUFFER + Buffer_ReadWrite = 7, /// STORAGE_BUFFER + Unknown = 8, /// Other +} + +// https://github.com/KhronosGroup/Vulkan-Docs/blob/main/proposals/VK_EXT_mutable_descriptor_type.adoc +enum VkMutableBindlessBindings : uint +{ + Sampler = 0, /// SAMPLER + CombinedTextureSampler = 1, /// COMBINED_IMAGE_SAMPLER + Texture_Read = 2, /// SAMPLED_IMAGE + Texture_ReadWrite = 2, /// STORAGE_IMAGE + TexelBuffer_Read = 2, /// UNIFORM_TEXEL_BUFFER + TexelBuffer_ReadWrite = 2, /// STORAGE_TEXEL_BUFFER + Buffer_Read = 2, /// UNIFORM_BUFFER + Buffer_ReadWrite = 2, /// STORAGE_BUFFER, + Unknown = 3, /// Other +} + __intrinsic_op($(kIROp_GetDynamicResourceHeap)) -T[] __getDynamicResourceHeap(); +T[] __getDynamicResourceHeap(constexpr uint bindingIndex = 0); // Used by the HLSL backend only, load a sampler state handle from SamplerDescriptorHeap. @@ -22442,10 +22473,16 @@ T __makeCombinedTextureSamplerFromHandle(U handle); __intrinsic_op($(kIROp_CastDescriptorHandleToResource)) T __castDescriptorHandleToResource(DescriptorHandle ptr); +public enum BindlessDescriptorOptions +{ + None = 0, /// Bind assuming regular binding model rules. + VkMutable = 1, /// Bind assuming `VK_EXT_mutable_descriptor_type` without mutable `AccelerationStructure` binding support. +} + /// The default implementation of `getDescriptorFromHandle`, which converts from a descriptor handle /// to a descriptor object. [ForceInline] -T defaultGetDescriptorFromHandle(DescriptorHandle handleValue) +T defaultGetDescriptorFromHandle(DescriptorHandle handleValue, constexpr BindlessDescriptorOptions bindlessOptions = BindlessDescriptorOptions.None) { __target_switch { @@ -22458,6 +22495,73 @@ T defaultGetDescriptorFromHandle(DescriptorHandle handle return __loadResourceDescriptorFromHeap(((uint2)handleValue).x); case spirv: case glsl: + + switch(bindlessOptions) + { +${{{{ +{ + static const struct + { + char const* option; + char const* enumType; + }kBindlessOptions[] = + { + {"None", "DefaultVkBindlessBindings"}, + {"VkMutable", "VkMutableBindlessBindings"}, + }; + + for(auto bindlessOption : kBindlessOptions) + { + StringBuilder bindlessOptionIfElsePre; + StringBuilder bindlessOptionIfElsePost; + + bindlessOptionIfElsePre << "case BindlessDescriptorOptions." << bindlessOption.option <<":\n{\n"; + bindlessOptionIfElsePost << "}\n"; +}}}} + $(bindlessOptionIfElsePre.toString()) + switch(T.kind) + { + case DescriptorKind.Sampler: + return __getDynamicResourceHeap($(bindlessOption.enumType).Sampler)[((uint2)handleValue).x]; + case DescriptorKind.CombinedTextureSampler: + return __getDynamicResourceHeap($(bindlessOption.enumType).CombinedTextureSampler)[((uint2)handleValue).x]; + case DescriptorKind.Texture: + { + if(DescriptorAccess.Read == T.descriptorAccess) + return __getDynamicResourceHeap($(bindlessOption.enumType).Texture_Read)[((uint2)handleValue).x]; + else + return __getDynamicResourceHeap($(bindlessOption.enumType).Texture_ReadWrite)[((uint2)handleValue).x]; + } + case DescriptorKind.TexelBuffer: + { + if(DescriptorAccess.Read == T.descriptorAccess) + return __getDynamicResourceHeap($(bindlessOption.enumType).TexelBuffer_Read)[((uint2)handleValue).x]; + else + return __getDynamicResourceHeap($(bindlessOption.enumType).TexelBuffer_ReadWrite)[((uint2)handleValue).x]; + } + case DescriptorKind.Buffer: + { + if(DescriptorAccess.Read == T.descriptorAccess) + return __getDynamicResourceHeap($(bindlessOption.enumType).Buffer_Read)[((uint2)handleValue).x]; + else + return __getDynamicResourceHeap($(bindlessOption.enumType).Buffer_ReadWrite)[((uint2)handleValue).x]; + } + case DescriptorKind.AccelerationStructure: + return __slang_noop_cast(RaytracingAccelerationStructure(__asuint64((uint2)handleValue))); + default: + return __getDynamicResourceHeap($(bindlessOption.enumType).Unknown)[((uint2)handleValue).x]; + } + $(bindlessOptionIfElsePost.toString()) +${{{{ + } +} +}}}} + default: + { + static_assert(false, "Impossible to end up here unless something went very wrong"); + return __getDynamicResourceHeap()[((uint2)handleValue).x]; + } + } case wgsl: return __getDynamicResourceHeap()[((uint2)handleValue).x]; default: diff --git a/source/slang/slang-ir-lower-dynamic-resource-heap.cpp b/source/slang/slang-ir-lower-dynamic-resource-heap.cpp index 60ec688a0..b2199a174 100644 --- a/source/slang/slang-ir-lower-dynamic-resource-heap.cpp +++ b/source/slang/slang-ir-lower-dynamic-resource-heap.cpp @@ -58,10 +58,11 @@ UInt findUnusedSpaceIndex(TargetProgram* targetProgram, IRModule* module) return index; } -IRVarLayout* createResourceHeapVarLayoutWithSpace( +IRVarLayout* createResourceHeapVarLayoutWithSpaceAndBinding( IRBuilder& builder, IRInst* param, - UInt spaceIndex) + UInt spaceIndex, + UInt bindingIndex) { SLANG_UNUSED(param); IRTypeLayout::Builder typeLayoutBuilder(&builder); @@ -71,7 +72,8 @@ IRVarLayout* createResourceHeapVarLayoutWithSpace( auto typeLayout = typeLayoutBuilder.build(); IRVarLayout::Builder varLayoutBuilder(&builder, typeLayout); varLayoutBuilder.findOrAddResourceInfo(LayoutResourceKind::RegisterSpace)->offset = spaceIndex; - varLayoutBuilder.findOrAddResourceInfo(LayoutResourceKind::DescriptorTableSlot)->offset = 0; + varLayoutBuilder.findOrAddResourceInfo(LayoutResourceKind::DescriptorTableSlot)->offset = + bindingIndex; return varLayoutBuilder.build(); } @@ -93,8 +95,14 @@ void lowerDynamicResourceHeap(TargetProgram* targetProgram, IRModule* module, Di IRBuilder builder(inst); builder.setInsertBefore(inst); + auto bindingIndex = (UInt)as(inst->getOperand(0))->getValue(); + auto param = builder.createGlobalParam(arrayType); - auto varLayout = createResourceHeapVarLayoutWithSpace(builder, param, unusedSpaceIndex); + auto varLayout = createResourceHeapVarLayoutWithSpaceAndBinding( + builder, + param, + unusedSpaceIndex, + bindingIndex); builder.addLayoutDecoration(param, varLayout); builder.addNameHintDecoration(param, toSlice("__slang_resource_heap")); inst->replaceUsesWith(param); -- cgit v1.2.3