Skip to content

Commit

Permalink
spirv-val: Revert Validate PhysicalStorageBuffer Stage Interface
Browse files Browse the repository at this point in the history
  • Loading branch information
@spencer-lunarg
spencer-lunarg committed Feb 14, 2024
1 parent 20ad38c commit adaf84c
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 170 deletions.
29 changes: 0 additions & 29 deletions source/val/validate_interfaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,29 +46,6 @@ bool is_interface_variable(const Instruction* inst, bool is_spv_1_4) {
}
}

// Special validation for varibles that are between shader stages
spv_result_t ValidateInputOutputInterfaceVariables(ValidationState_t& _,
const Instruction* var) {
auto var_pointer = _.FindDef(var->GetOperandAs<uint32_t>(0));
uint32_t pointer_id = var_pointer->GetOperandAs<uint32_t>(2);

const auto isPhysicalStorageBuffer = [](const Instruction* insn) {
return insn->opcode() == spv::Op::OpTypePointer &&
insn->GetOperandAs<spv::StorageClass>(1) ==
spv::StorageClass::PhysicalStorageBuffer;
};

if (_.ContainsType(pointer_id, isPhysicalStorageBuffer)) {
return _.diag(SPV_ERROR_INVALID_ID, var)
<< _.VkErrorID(9557) << "Input/Output interface variable id <"
<< var->id()
<< "> contains a PhysicalStorageBuffer pointer, which is not "
"allowed. If you want to interface shader stages with a "
"PhysicalStorageBuffer, cast to a uint64 or uvec2 instead.";
}
return SPV_SUCCESS;
}

// Checks that \c var is listed as an interface in all the entry points that use
// it.
spv_result_t check_interface_variable(ValidationState_t& _,
Expand Down Expand Up @@ -128,12 +105,6 @@ spv_result_t check_interface_variable(ValidationState_t& _,
}
}

if (var->GetOperandAs<spv::StorageClass>(2) == spv::StorageClass::Input ||
var->GetOperandAs<spv::StorageClass>(2) == spv::StorageClass::Output) {
if (auto error = ValidateInputOutputInterfaceVariables(_, var))
return error;
}

return SPV_SUCCESS;
}

Expand Down
2 changes: 0 additions & 2 deletions source/val/validation_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2307,8 +2307,6 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-08722);
case 8973:
return VUID_WRAP(VUID-StandaloneSpirv-Pointer-08973);
case 9557:
return VUID_WRAP(VUID-StandaloneSpirv-Input-09557);
default:
return ""; // unknown id
}
Expand Down
145 changes: 6 additions & 139 deletions test/val/val_interfaces_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1599,7 +1599,7 @@ TEST_F(ValidateInterfacesTest, InvalidLocationTypePointer) {
HasSubstr("Invalid type to assign a location"));
}

TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointer) {
TEST_F(ValidateInterfacesTest, ValidLocationTypePhysicalStorageBufferPointer) {
const std::string text = R"(
OpCapability Shader
OpCapability PhysicalStorageBufferAddresses
Expand All @@ -1608,151 +1608,18 @@ OpEntryPoint Vertex %main "main" %var
OpDecorate %var Location 0
OpDecorate %var RestrictPointer
%void = OpTypeVoid
%uint = OpTypeInt 32 0
%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
%in_ptr = OpTypePointer Input %psb_ptr
%var = OpVariable %in_ptr Input
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Input/Output interface variable id <2> contains a "
"PhysicalStorageBuffer pointer, which is not allowed"));
}

TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArray) {
const std::string text = R"(
OpCapability Shader
OpCapability PhysicalStorageBufferAddresses
OpMemoryModel PhysicalStorageBuffer64 GLSL450
OpEntryPoint Vertex %main "main" %var
OpDecorate %var Location 0
OpDecorate %var RestrictPointer
%void = OpTypeVoid
%uint = OpTypeInt 32 0
%uint_3 = OpConstant %uint 3
%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
%array = OpTypeArray %psb_ptr %uint_3
%in_ptr = OpTypePointer Input %array
%var = OpVariable %in_ptr Input
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Input/Output interface variable id <2> contains a "
"PhysicalStorageBuffer pointer, which is not allowed"));
}

TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerStruct) {
const std::string text = R"(
OpCapability Shader
OpCapability PhysicalStorageBufferAddresses
OpMemoryModel PhysicalStorageBuffer64 GLSL450
OpEntryPoint Vertex %main "main" %var
OpDecorate %var Location 0
OpDecorate %var RestrictPointer
%void = OpTypeVoid
%int = OpTypeInt 32 1
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
%struct_0 = OpTypeStruct %int %psb_ptr
%struct_1 = OpTypeStruct %int %int
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
%in_ptr = OpTypePointer Input %struct_0
%var = OpVariable %in_ptr Input
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Input/Output interface variable id <2> contains a "
"PhysicalStorageBuffer pointer, which is not allowed"));
}

TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArrayOfStruct) {
const std::string text = R"(
OpCapability Shader
OpCapability PhysicalStorageBufferAddresses
OpMemoryModel PhysicalStorageBuffer64 GLSL450
OpEntryPoint Vertex %main "main" %var
OpDecorate %var Location 0
OpDecorate %var RestrictPointer
%void = OpTypeVoid
%int = OpTypeInt 32 1
%uint = OpTypeInt 32 0
%uint_3 = OpConstant %uint 3
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
%array_1 = OpTypeArray %psb_ptr %uint_3
%struct_0 = OpTypeStruct %int %array_1
%struct_1 = OpTypeStruct %int %int
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
%array_0 = OpTypeArray %struct_0 %uint_3
%in_ptr = OpTypePointer Input %array_0
%var = OpVariable %in_ptr Input
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Input/Output interface variable id <2> contains a "
"PhysicalStorageBuffer pointer, which is not allowed"));
}

TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerNestedStruct) {
const std::string text = R"(
OpCapability Shader
OpCapability PhysicalStorageBufferAddresses
OpMemoryModel PhysicalStorageBuffer64 GLSL450
OpEntryPoint Vertex %main "main" %var
OpDecorate %var Location 0
OpDecorate %var RestrictPointer
%void = OpTypeVoid
%int = OpTypeInt 32 1
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
%struct_0 = OpTypeStruct %int %psb_ptr
%struct_1 = OpTypeStruct %int %int
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
%struct_2 = OpTypeStruct %int %struct_0
%in_ptr = OpTypePointer Input %struct_2
%var = OpVariable %in_ptr Input
%int = OpTypeInt 32 0
%ptr = OpTypePointer PhysicalStorageBuffer %int
%ptr2 = OpTypePointer Input %ptr
%var = OpVariable %ptr2 Input
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Input/Output interface variable id <2> contains a "
"PhysicalStorageBuffer pointer, which is not allowed"));
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
}

} // namespace
Expand Down

0 comments on commit adaf84c

Please sign in to comment.