Skip to content

Commit 6cb6465

Browse files
Guang-035Guang-amd
andauthored
spirv-val: Allow ArrayStrideIdEXT on structs with descriptors (KhronosGroup#6705)
Per `SPV_EXT_descriptor_heap`, `ArrayStrideIdEXT` must be allowed on arrays containing descriptor types. This includes composites that contain descriptors, not only arrays whose element type is directly a descriptor. **Root Cause**: The validator only checked whether the array element type itself was a descriptor type. For [RuntimeArray<Material>](https://github.com/KhronosGroup/glslang/pull/4272/changes#diff-3142250076641bab8379f6637d480636190a14c9f31e0f7915cb4e76fc1d840e), the direct element type is `OpTypeStruct`, so validation rejected it even though `Material` contains `OpTypeBufferEXT`. **Solution**: Recursively inspect the array element type when validating `ArrayStrideIdEXT`, so structs and other composites containing descriptor types are accepted. **Test**: Added a validator regression test covering a runtime array of a struct containing `OpTypeBufferEXT`. Co-authored-by: guangxu2 <guangxu2@amd.com>
1 parent 236bf64 commit 6cb6465

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

source/val/validate_annotation.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,10 +403,15 @@ spv_result_t ValidateDecorateId(ValidationState_t& _, const Instruction* inst) {
403403
}
404404
}
405405

406-
// Strip array and should be the descriptor type
406+
const auto is_descriptor_type = [&_](const Instruction* type_inst) {
407+
return _.IsDescriptorType(type_inst->opcode());
408+
};
409+
410+
// Strip the array. The element may be a descriptor type directly, or a
411+
// composite containing a descriptor type.
407412
const uint32_t element_type =
408413
_.FindDef(target_id)->GetOperandAs<uint32_t>(1);
409-
if (!_.IsDescriptorType(element_type)) {
414+
if (!_.ContainsType(element_type, is_descriptor_type, true)) {
410415
return _.diag(SPV_ERROR_INVALID_ID, inst)
411416
<< "ArrayStrideIdEXT decoration must only be applied to"
412417
<< " array type containing a Descriptor type.";

test/val/val_extension_spv_ext_descriptor_heap.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,6 +1560,33 @@ TEST_F(ValidateSpvEXTDescriptorHeap, ArrayStrideNonDescriptor) {
15601560
"array type containing a Descriptor type."));
15611561
}
15621562

1563+
TEST_F(ValidateSpvEXTDescriptorHeap, ArrayStrideStructContainingDescriptor) {
1564+
const std::string str = R"(
1565+
OpCapability Shader
1566+
OpCapability DescriptorHeapEXT
1567+
OpExtension "SPV_EXT_descriptor_heap"
1568+
OpMemoryModel Logical GLSL450
1569+
OpEntryPoint GLCompute %main "main"
1570+
OpExecutionMode %main LocalSize 1 1 1
1571+
OpDecorateId %_runtimearr_Material ArrayStrideIdEXT %uint_16
1572+
%void = OpTypeVoid
1573+
%3 = OpTypeFunction %void
1574+
%uint = OpTypeInt 32 0
1575+
%uint_16 = OpConstant %uint 16
1576+
%float = OpTypeFloat 32
1577+
%v4float = OpTypeVector %float 4
1578+
%buffer = OpTypeBufferEXT StorageBuffer
1579+
%Material = OpTypeStruct %v4float %buffer
1580+
%_runtimearr_Material = OpTypeRuntimeArray %Material
1581+
%main = OpFunction %void None %3
1582+
%5 = OpLabel
1583+
OpReturn
1584+
OpFunctionEnd
1585+
)";
1586+
CompileSuccessfully(str.c_str(), SPV_ENV_VULKAN_1_3);
1587+
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1588+
}
1589+
15631590
TEST_F(ValidateSpvEXTDescriptorHeap, MemberDecorateIdArrayStrideIdEXT) {
15641591
const std::string str = R"(
15651592
OpCapability Shader

0 commit comments

Comments
 (0)