Skip to content

Commit

Permalink
Fix layout checks with untyped pointers (KhronosGroup#5970)
Browse files Browse the repository at this point in the history
* Fix layout checks with untyped pointers

* Handle the case where an untyped pointer represents a descriptor array
  * the array should not be required to have an array stride, but the
    element layout should be checked

* formatting
  • Loading branch information
alan-baker authored Jan 29, 2025
1 parent d99e54e commit 2e81137
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 1 deletion.
20 changes: 19 additions & 1 deletion source/val/validate_decorations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1464,11 +1464,29 @@ spv_result_t CheckDecorationsOfBuffers(ValidationState_t& vstate) {
: (sc == spv::StorageClass::Workgroup ? "Workgroup"
: "StorageBuffer"));

const auto data_type = vstate.FindDef(data_type_id);
auto data_type = vstate.FindDef(data_type_id);
scalar_block_layout =
sc == spv::StorageClass::Workgroup
? vstate.options()->workgroup_scalar_block_layout
: vstate.options()->scalar_block_layout;

// If the data type is an array that contains a Block- or
// BufferBlock-decorated struct, then use the struct for layout checks
// instead of the array. In this case, the array represents a descriptor
// array which should not have an explicit layout.
if (data_type->opcode() == spv::Op::OpTypeArray ||
data_type->opcode() == spv::Op::OpTypeRuntimeArray) {
const auto ele_type =
vstate.FindDef(data_type->GetOperandAs<uint32_t>(1u));
if (ele_type->opcode() == spv::Op::OpTypeStruct &&
(vstate.HasDecoration(ele_type->id(), spv::Decoration::Block) ||
vstate.HasDecoration(ele_type->id(),
spv::Decoration::BufferBlock))) {
data_type = ele_type;
data_type_id = ele_type->id();
}
}

// Assume uniform storage class uses block rules unless we see a
// BufferBlock decorated struct in the data type.
bool bufferRules = sc == spv::StorageClass::Uniform ? false : true;
Expand Down
61 changes: 61 additions & 0 deletions test/val/val_decoration_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10696,6 +10696,67 @@ OpDecorate %array ArrayStride 4
"decorated with ArrayStride"));
}

TEST_F(ValidateDecorations, BlockArrayWithoutStride) {
const std::string spirv = R"(
OpCapability Shader
OpExtension "SPV_KHR_storage_buffer_storage_class"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpDecorate %struct Block
OpMemberDecorate %struct 0 Offset 0
OpDecorate %var DescriptorSet 0
OpDecorate %var Binding 0
%int = OpTypeInt 32 0
%int_4 = OpConstant %int 4
%struct = OpTypeStruct %int
%array = OpTypeArray %struct %int_4
%ptr = OpTypePointer StorageBuffer %array
%var = OpVariable %ptr StorageBuffer
%void = OpTypeVoid
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_0));
}

TEST_F(ValidateDecorations, BlockArrayWithoutStrideUntypedAccessChain) {
const std::string spirv = R"(
OpCapability Shader
OpCapability UntypedPointersKHR
OpExtension "SPV_KHR_storage_buffer_storage_class"
OpExtension "SPV_KHR_untyped_pointers"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpDecorate %struct Block
OpMemberDecorate %struct 0 Offset 0
OpDecorate %var DescriptorSet 0
OpDecorate %var Binding 0
%int = OpTypeInt 32 0
%int_4 = OpConstant %int 4
%struct = OpTypeStruct %int
%array = OpTypeArray %struct %int_4
%void = OpTypeVoid
%ptr = OpTypeUntypedPointerKHR StorageBuffer
%var = OpUntypedVariableKHR %ptr StorageBuffer %array
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
%gep = OpUntypedAccessChainKHR %ptr %array %var
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_0));
}

} // namespace
} // namespace val
} // namespace spvtools

0 comments on commit 2e81137

Please sign in to comment.