From cd7da79f50f2b8fb9bca797131585ff1b85698f6 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Oct 2025 14:14:05 -0700 Subject: Fix incorrect binding index assignment for StructuredBuffer and ByteAddressBuffer with DescriptorHandle (#8252) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - [x] Fix segmentation fault in wrapConstantBufferElement for DescriptorHandle types - [x] Split DescriptorKind.Buffer into ConstantBuffer and StorageBuffer - [x] Update binding enums with descriptive names (ConstantBuffer_Read, StorageBuffer_Read, etc.) - [x] Update resource type mappings for correct binding assignments - [x] Update template logic to handle ConstantBuffer and StorageBuffer kinds separately - [x] Update tests to reflect correct binding assignments - [x] Split DescriptorKind.TexelBuffer into UniformTexelBuffer and StorageTexelBuffer - [x] Update TextureBuffer to use UniformTexelBuffer kind - [x] Update _Texture extension to determine texel buffer kind based on access mode - [x] Update test desc-handle-1.slang to handle new DescriptorKind enum cases --- ✨ Let Copilot coding agent [set things up for you](https://github.com/shader-slang/slang/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: Yong He --- source/slang/hlsl.meta.slang | 82 ++++++++++++---------- source/slang/slang-check-decl.cpp | 19 +++-- .../descriptor-handle/desc-handle-1.slang | 2 +- .../descriptor-handle/desc-handle-default.slang | 4 +- .../enums/enum-use-prev-value.slang | 18 +++++ 5 files changed, 76 insertions(+), 49 deletions(-) create mode 100644 tests/language-feature/enums/enum-use-prev-value.slang diff --git a/source/slang/hlsl.meta.slang b/source/slang/hlsl.meta.slang index bf5039905..489bcbab6 100644 --- a/source/slang/hlsl.meta.slang +++ b/source/slang/hlsl.meta.slang @@ -22770,10 +22770,13 @@ enum DescriptorKind Unknown, /// Unknown descriptor kind. Texture, /// A texture descriptor. CombinedTextureSampler, /// A combined texture and sampler state descriptor. - Buffer, /// A buffer descriptor. + ConstantBuffer, /// A constant buffer descriptor. + StorageBuffer, /// A storage buffer descriptor. + Buffer = StorageBuffer, /// Deprecated. Use `StorageBuffer` instead. Sampler, /// A sampler state descriptor. AccelerationStructure, /// A ray tracing acceleration structure descriptor. - TexelBuffer /// A texel buffer descriptor. + UniformTexelBuffer, /// A uniform texel buffer descriptor (read-only). + StorageTexelBuffer /// A storage texel buffer descriptor (read-write). } enum DescriptorAccess @@ -22827,7 +22830,7 @@ extension _Texture; static const DescriptorKind kind = (Shape.flavor == __ShapeBuffer.flavor ? - DescriptorKind.TexelBuffer : (isCombined != 0 ? DescriptorKind.CombinedTextureSampler : DescriptorKind.Texture) + (access == $(kCoreModule_ResourceAccessReadOnly) ? DescriptorKind.UniformTexelBuffer : DescriptorKind.StorageTexelBuffer) : (isCombined != 0 ? DescriptorKind.CombinedTextureSampler : DescriptorKind.Texture) ); static const DescriptorAccess descriptorAccess = (DescriptorAccess)access; @@ -22849,19 +22852,19 @@ struct DynamicResourceTypeInfo }; const DynamicResourceTypeInfo kDynamicResourceCastableTypes[] = { - { "StructuredBuffer", "Buffer", "General", "Read" }, - { "RWStructuredBuffer", "Buffer", "General", "ReadWrite" }, - { "AppendStructuredBuffer", "Buffer", "General", "ReadWrite" }, - { "ConsumeStructuredBuffer", "Buffer", "General", "ReadWrite" }, - { "RasterizerOrderedStructuredBuffer", "Buffer", "General", "ReadWrite" }, - { "ByteAddressBuffer", "Buffer", "General", "Read" }, - { "RWByteAddressBuffer", "Buffer", "General", "ReadWrite" }, - { "RasterizerOrderedByteAddressBuffer", "Buffer", "General", "ReadWrite" }, + { "StructuredBuffer", "StorageBuffer", "General", "Read" }, + { "RWStructuredBuffer", "StorageBuffer", "General", "ReadWrite" }, + { "AppendStructuredBuffer", "StorageBuffer", "General", "ReadWrite" }, + { "ConsumeStructuredBuffer", "StorageBuffer", "General", "ReadWrite" }, + { "RasterizerOrderedStructuredBuffer", "StorageBuffer", "General", "ReadWrite" }, + { "ByteAddressBuffer", "StorageBuffer", "General", "Read" }, + { "RWByteAddressBuffer", "StorageBuffer", "General", "ReadWrite" }, + { "RasterizerOrderedByteAddressBuffer", "StorageBuffer", "General", "ReadWrite" }, { "SamplerState", "Sampler", "Sampler", "Unknown" }, { "SamplerComparisonState", "Sampler", "Sampler", "Unknown" }, - { "ConstantBuffer", "Buffer", "General", "Read"}, - { "TextureBuffer", "Buffer", "General", "Read"}, - { "RaytracingAccelerationStructure", "AccelerationStructure", "General", "Read"}, + { "ConstantBuffer", "ConstantBuffer", "General", "Read" }, + { "TextureBuffer", "UniformTexelBuffer", "General", "Read" }, + { "RaytracingAccelerationStructure", "AccelerationStructure", "General", "Read" }, }; for (auto type : kDynamicResourceCastableTypes) { @@ -22996,12 +22999,13 @@ 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 + SampledImage = 2, /// SAMPLED_IMAGE + StorageImage = 3, /// STORAGE_IMAGE + UniformTexelBuffer = 4, /// UNIFORM_TEXEL_BUFFER + StorageTexelBuffer = 5, /// STORAGE_TEXEL_BUFFER + ConstantBuffer_Read = 6, /// UNIFORM_BUFFER + StorageBuffer_Read = 7, /// STORAGE_BUFFER + StorageBuffer_ReadWrite = 7, /// STORAGE_BUFFER Unknown = 8, /// Other } @@ -23010,12 +23014,13 @@ 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, + SampledImage = 2, /// SAMPLED_IMAGE + StorageImage = 2, /// STORAGE_IMAGE + UniformTexelBuffer = 2, /// UNIFORM_TEXEL_BUFFER + StorageTexelBuffer = 2, /// STORAGE_TEXEL_BUFFER + ConstantBuffer_Read = 2, /// UNIFORM_BUFFER + StorageBuffer_Read = 2, /// STORAGE_BUFFER + StorageBuffer_ReadWrite = 2, /// STORAGE_BUFFER, Unknown = 3, /// Other } @@ -23096,23 +23101,28 @@ ${{{{ case DescriptorKind.Texture: { if(DescriptorAccess.Read == T.descriptorAccess) - return __getDynamicResourceHeap($(bindlessOption.enumType).Texture_Read)[((uint2)handleValue).x]; + return __getDynamicResourceHeap($(bindlessOption.enumType).SampledImage)[((uint2)handleValue).x]; else - return __getDynamicResourceHeap($(bindlessOption.enumType).Texture_ReadWrite)[((uint2)handleValue).x]; + return __getDynamicResourceHeap($(bindlessOption.enumType).StorageImage)[((uint2)handleValue).x]; } - case DescriptorKind.TexelBuffer: + case DescriptorKind.UniformTexelBuffer: { - if(DescriptorAccess.Read == T.descriptorAccess) - return __getDynamicResourceHeap($(bindlessOption.enumType).TexelBuffer_Read)[((uint2)handleValue).x]; - else - return __getDynamicResourceHeap($(bindlessOption.enumType).TexelBuffer_ReadWrite)[((uint2)handleValue).x]; + return __getDynamicResourceHeap($(bindlessOption.enumType).UniformTexelBuffer)[((uint2)handleValue).x]; + } + case DescriptorKind.StorageTexelBuffer: + { + return __getDynamicResourceHeap($(bindlessOption.enumType).StorageTexelBuffer)[((uint2)handleValue).x]; + } + case DescriptorKind.ConstantBuffer: + { + return __getDynamicResourceHeap($(bindlessOption.enumType).ConstantBuffer_Read)[((uint2)handleValue).x]; } - case DescriptorKind.Buffer: + case DescriptorKind.StorageBuffer: { if(DescriptorAccess.Read == T.descriptorAccess) - return __getDynamicResourceHeap($(bindlessOption.enumType).Buffer_Read)[((uint2)handleValue).x]; + return __getDynamicResourceHeap($(bindlessOption.enumType).StorageBuffer_Read)[((uint2)handleValue).x]; else - return __getDynamicResourceHeap($(bindlessOption.enumType).Buffer_ReadWrite)[((uint2)handleValue).x]; + return __getDynamicResourceHeap($(bindlessOption.enumType).StorageBuffer_ReadWrite)[((uint2)handleValue).x]; } case DescriptorKind.AccelerationStructure: return __slang_noop_cast(RaytracingAccelerationStructure(__asuint64((uint2)handleValue))); diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index 77a799f1c..6f2b01aa1 100644 --- a/source/slang/slang-check-decl.cpp +++ b/source/slang/slang-check-decl.cpp @@ -8992,6 +8992,7 @@ void SemanticsDeclBodyVisitor::visitEnumDecl(EnumDecl* decl) tryConstantFoldExpr(explicitTagValExpr, ConstantFoldingKind::CompileTime, nullptr); if (explicitTagVal) { + caseDecl->tagVal = explicitTagVal; if (auto constIntVal = as(explicitTagVal)) { defaultTag = constIntVal->getValue(); @@ -9005,8 +9006,10 @@ void SemanticsDeclBodyVisitor::visitEnumDecl(EnumDecl* decl) else { // If this happens, then the explicit tag value expression - // doesn't seem to be a constant after all. In this case - // we expect the checking logic to have applied already. + // doesn't seem to be a constant after all. + getSink()->diagnose( + explicitTagValExpr, + Diagnostics::expectedIntegerConstantNotConstant); } } else @@ -9017,8 +9020,8 @@ void SemanticsDeclBodyVisitor::visitEnumDecl(EnumDecl* decl) tagValExpr->loc = caseDecl->loc; tagValExpr->type = QualType(tagType); tagValExpr->value = defaultTag; - caseDecl->tagExpr = tagValExpr; + caseDecl->tagVal = m_astBuilder->getIntVal(enumType, defaultTag); } // Default tag for the next case will be one more than @@ -9058,15 +9061,9 @@ void SemanticsDeclBodyVisitor::visitEnumCaseDecl(EnumCaseDecl* decl) if (auto initExpr = decl->tagExpr) { initExpr = CheckTerm(initExpr); - initExpr = coerce(CoercionSite::General, tagType, initExpr, getSink()); - // We want to enforce that this is an integer constant - // expression. - decl->tagVal = CheckIntegerConstantExpression( - initExpr, - IntegerConstantExpressionCoercionType::AnyInteger, - nullptr, - ConstantFoldingKind::CompileTime); + if (initExpr->type != decl->type.type) + initExpr = coerce(CoercionSite::General, tagType, initExpr, getSink()); decl->tagExpr = initExpr; } diff --git a/tests/language-feature/descriptor-handle/desc-handle-1.slang b/tests/language-feature/descriptor-handle/desc-handle-1.slang index 0621f4a84..407aee684 100644 --- a/tests/language-feature/descriptor-handle/desc-handle-1.slang +++ b/tests/language-feature/descriptor-handle/desc-handle-1.slang @@ -16,7 +16,7 @@ export T getDescriptorFromHandle(DescriptorHandle handle __target_switch { case spirv: - if (T.kind != DescriptorKind.Buffer) + if (T.kind != DescriptorKind.ConstantBuffer && T.kind != DescriptorKind.StorageBuffer) return resourceHeap[((uint2)handleValue).x].asOpaqueDescriptor(); else return defaultGetDescriptorFromHandle(handleValue); diff --git a/tests/language-feature/descriptor-handle/desc-handle-default.slang b/tests/language-feature/descriptor-handle/desc-handle-default.slang index 9a4d444d7..674662f4b 100644 --- a/tests/language-feature/descriptor-handle/desc-handle-default.slang +++ b/tests/language-feature/descriptor-handle/desc-handle-default.slang @@ -32,12 +32,12 @@ Texture1D t4; //MIX-DAG: OpDecorate %__slang_resource_heap{{.*}} Binding 0 //MIX-DAG: OpDecorate %__slang_resource_heap{{.*}} Binding 5 //MIX-DAG: OpDecorate %__slang_resource_heap{{.*}} Binding 6 +//MIX-DAG: OpDecorate %__slang_resource_heap{{.*}} Binding 7 //MIX-NOT: OpDecorate %__slang_resource_heap{{.*}} Binding 1 //MIX-NOT: OpDecorate %__slang_resource_heap{{.*}} Binding 2 //MIX-NOT: OpDecorate %__slang_resource_heap{{.*}} Binding 3 //MIX-NOT: OpDecorate %__slang_resource_heap{{.*}} Binding 4 -//MIX-NOT: OpDecorate %__slang_resource_heap{{.*}} Binding 7 //MIX-NOT: OpDecorate %__slang_resource_heap{{.*}} Binding 8 #ifdef SAMPLER @@ -89,6 +89,8 @@ uniform RWBuffer.Handle rwTexelBuffer; #ifdef UNIFORM_BUFFER //UNIFORM_BUFFER: OpDecorate %__slang_resource_heap{{.*}} Binding 6 //UNIFORM_BUFFER-NEXT: OpDecorate %__slang_resource_heap{{.*}} DescriptorSet 3 +//UNIFORM_BUFFER: OpDecorate %__slang_resource_heap{{.*}} Binding 7 +//UNIFORM_BUFFER-NEXT: OpDecorate %__slang_resource_heap{{.*}} DescriptorSet 3 struct Data { float v; diff --git a/tests/language-feature/enums/enum-use-prev-value.slang b/tests/language-feature/enums/enum-use-prev-value.slang new file mode 100644 index 000000000..b3a9ed2eb --- /dev/null +++ b/tests/language-feature/enums/enum-use-prev-value.slang @@ -0,0 +1,18 @@ +//TEST:INTERPRET(filecheck=CHECK): + +enum MyEnum +{ + A, + B = A, + C +} + +void main() +{ + // CHECK: A = 0 + printf("A = %d\n", MyEnum.A); + // CHECK: B = 0 + printf("B = %d\n", MyEnum.B); + // CHECK: C = 1 + printf("C = %d\n", MyEnum.C); +} \ No newline at end of file -- cgit v1.2.3