Fix layout checks for nested struct in relaxed layout; and descriptor arrays (#2312)

* Fixed layout checks for nested structures

Fixes #2303

* Incoming offsets accumulate through nested structures

* Check layouts through arrays

* Perform layout checks in the presence of descriptor arrays (and
runtime arrays)

* Fix formatting
This commit is contained in:
David Neto 2019-01-22 15:15:24 -08:00 committed by GitHub
parent 3a3ad2ec50
commit 4a405eda53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 130 additions and 10 deletions

View File

@ -379,6 +379,7 @@ bool IsAlignedTo(uint32_t offset, uint32_t alignment) {
// or row major-ness.
spv_result_t checkLayout(uint32_t struct_id, const char* storage_class_str,
const char* decoration_str, bool blockRules,
uint32_t incoming_offset,
MemberConstraints& constraints,
ValidationState_t& vstate) {
if (vstate.options()->skip_block_layout) return SPV_SUCCESS;
@ -429,7 +430,8 @@ spv_result_t checkLayout(uint32_t struct_id, const char* storage_class_str,
}
}
}
member_offsets.push_back(MemberOffsetPair{memberIdx, offset});
member_offsets.push_back(
MemberOffsetPair{memberIdx, incoming_offset + offset});
}
std::stable_sort(
member_offsets.begin(), member_offsets.end(),
@ -493,9 +495,9 @@ spv_result_t checkLayout(uint32_t struct_id, const char* storage_class_str,
// Check struct members recursively.
spv_result_t recursive_status = SPV_SUCCESS;
if (SpvOpTypeStruct == opcode &&
SPV_SUCCESS != (recursive_status =
checkLayout(id, storage_class_str, decoration_str,
blockRules, constraints, vstate)))
SPV_SUCCESS != (recursive_status = checkLayout(
id, storage_class_str, decoration_str, blockRules,
offset, constraints, vstate)))
return recursive_status;
// Check matrix stride.
if (SpvOpTypeMatrix == opcode) {
@ -514,7 +516,7 @@ spv_result_t checkLayout(uint32_t struct_id, const char* storage_class_str,
if (SpvOpTypeStruct == arrayInst->opcode() &&
SPV_SUCCESS != (recursive_status = checkLayout(
typeId, storage_class_str, decoration_str,
blockRules, constraints, vstate)))
blockRules, offset, constraints, vstate)))
return recursive_status;
// Check array stride.
for (auto& decoration : vstate.id_decorations(id)) {
@ -859,8 +861,15 @@ spv_result_t CheckDecorationsOfBuffers(ValidationState_t& vstate) {
if (uniform || push_constant || storage_buffer || phys_storage_buffer) {
const auto ptrInst = vstate.FindDef(words[1]);
assert(SpvOpTypePointer == ptrInst->opcode());
const auto id = ptrInst->words()[3];
if (SpvOpTypeStruct != vstate.FindDef(id)->opcode()) continue;
auto id = ptrInst->words()[3];
auto id_inst = vstate.FindDef(id);
// Jump through one level of arraying.
if (id_inst->opcode() == SpvOpTypeArray ||
id_inst->opcode() == SpvOpTypeRuntimeArray) {
id = id_inst->GetOperandAs<uint32_t>(1u);
id_inst = vstate.FindDef(id);
}
if (SpvOpTypeStruct != id_inst->opcode()) continue;
MemberConstraints constraints;
ComputeMemberConstraintsForStruct(&constraints, id, LayoutConstraints(),
vstate);
@ -942,12 +951,12 @@ spv_result_t CheckDecorationsOfBuffers(ValidationState_t& vstate) {
"decorations.";
} else if (blockRules &&
(SPV_SUCCESS != (recursive_status = checkLayout(
id, sc_str, deco_str, true,
id, sc_str, deco_str, true, 0,
constraints, vstate)))) {
return recursive_status;
} else if (bufferRules &&
(SPV_SUCCESS != (recursive_status = checkLayout(
id, sc_str, deco_str, false,
id, sc_str, deco_str, false, 0,
constraints, vstate)))) {
return recursive_status;
}

View File

@ -3285,7 +3285,7 @@ TEST_F(ValidateDecorations,
getDiagnosticString(),
HasSubstr("Structure id 6 decorated as Block for variable in Uniform "
"storage class must follow standard uniform buffer layout "
"rules: member 2 at offset 24 is not aligned to 16"));
"rules: member 2 at offset 152 is not aligned to 16"));
}
TEST_F(ValidateDecorations,
@ -5302,6 +5302,117 @@ OpFunctionEnd
EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState());
}
TEST_F(ValidateDecorations, InvalidStraddle) {
const std::string spirv = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpMemberDecorate %inner_struct 0 Offset 0
OpMemberDecorate %inner_struct 1 Offset 4
OpDecorate %outer_struct Block
OpMemberDecorate %outer_struct 0 Offset 0
OpMemberDecorate %outer_struct 1 Offset 8
OpDecorate %var DescriptorSet 0
OpDecorate %var Binding 0
%void = OpTypeVoid
%float = OpTypeFloat 32
%float2 = OpTypeVector %float 2
%inner_struct = OpTypeStruct %float %float2
%outer_struct = OpTypeStruct %float2 %inner_struct
%ptr_ssbo_outer = OpTypePointer StorageBuffer %outer_struct
%var = OpVariable %ptr_ssbo_outer StorageBuffer
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_1);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Structure id 2 decorated as Block for variable in "
"StorageBuffer storage class must follow relaxed "
"storage buffer layout rules: member 1 is an "
"improperly straddling vector at offset 12"));
}
TEST_F(ValidateDecorations, DescriptorArray) {
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
OpMemberDecorate %struct 1 Offset 1
OpDecorate %var DescriptorSet 0
OpDecorate %var Binding 0
%void = OpTypeVoid
%float = OpTypeFloat 32
%int = OpTypeInt 32 0
%int_2 = OpConstant %int 2
%float2 = OpTypeVector %float 2
%struct = OpTypeStruct %float %float2
%struct_array = OpTypeArray %struct %int_2
%ptr_ssbo_array = OpTypePointer StorageBuffer %struct_array
%var = OpVariable %ptr_ssbo_array StorageBuffer
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Structure id 2 decorated as Block for variable in "
"StorageBuffer storage class must follow standard "
"storage buffer layout rules: member 1 at offset 1 is "
"not aligned to 8"));
}
TEST_F(ValidateDecorations, DescriptorRuntimeArray) {
const std::string spirv = R"(
OpCapability Shader
OpCapability RuntimeDescriptorArrayEXT
OpExtension "SPV_KHR_storage_buffer_storage_class"
OpExtension "SPV_EXT_descriptor_indexing"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpDecorate %struct Block
OpMemberDecorate %struct 0 Offset 0
OpMemberDecorate %struct 1 Offset 1
OpDecorate %var DescriptorSet 0
OpDecorate %var Binding 0
%void = OpTypeVoid
%float = OpTypeFloat 32
%int = OpTypeInt 32 0
%float2 = OpTypeVector %float 2
%struct = OpTypeStruct %float %float2
%struct_array = OpTypeRuntimeArray %struct
%ptr_ssbo_array = OpTypePointer StorageBuffer %struct_array
%var = OpVariable %ptr_ssbo_array StorageBuffer
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Structure id 2 decorated as Block for variable in "
"StorageBuffer storage class must follow standard "
"storage buffer layout rules: member 1 at offset 1 is "
"not aligned to 8"));
}
} // namespace
} // namespace val
} // namespace spvtools