Skip to content

Commit d3fc6ed

Browse files
spencer-lunargspencer-lunarg
authored
spirv-val: Validate PhysicalStorageBuffer Stage Interface (KhronosGroup#6000)
1 parent f6b40e7 commit d3fc6ed

File tree

3 files changed

+169
-6
lines changed

3 files changed

+169
-6
lines changed

‎source/val/validate_interfaces.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,29 @@ bool is_interface_variable(const Instruction* inst, bool is_spv_1_4) {
4848
}
4949
}
5050

51+
// Special validation for varibles that are between shader stages
52+
spv_result_t ValidateInputOutputInterfaceVariables(ValidationState_t& _,
53+
const Instruction* var) {
54+
auto var_pointer = _.FindDef(var->GetOperandAs<uint32_t>(0));
55+
uint32_t pointer_id = var_pointer->GetOperandAs<uint32_t>(2);
56+
57+
const auto isPhysicalStorageBuffer = [](const Instruction* insn) {
58+
return insn->opcode() == spv::Op::OpTypePointer &&
59+
insn->GetOperandAs<spv::StorageClass>(1) ==
60+
spv::StorageClass::PhysicalStorageBuffer;
61+
};
62+
63+
if (_.ContainsType(pointer_id, isPhysicalStorageBuffer)) {
64+
return _.diag(SPV_ERROR_INVALID_ID, var)
65+
<< _.VkErrorID(9557) << "Input/Output interface variable id <"
66+
<< var->id()
67+
<< "> contains a PhysicalStorageBuffer pointer, which is not "
68+
"allowed. If you want to interface shader stages with a "
69+
"PhysicalStorageBuffer, cast to a uint64 or uvec2 instead.";
70+
}
71+
return SPV_SUCCESS;
72+
}
73+
5174
// Checks that \c var is listed as an interface in all the entry points that use
5275
// it.
5376
spv_result_t check_interface_variable(ValidationState_t& _,
@@ -107,6 +130,12 @@ spv_result_t check_interface_variable(ValidationState_t& _,
107130
}
108131
}
109132

133+
if (var->GetOperandAs<spv::StorageClass>(2) == spv::StorageClass::Input ||
134+
var->GetOperandAs<spv::StorageClass>(2) == spv::StorageClass::Output) {
135+
if (auto error = ValidateInputOutputInterfaceVariables(_, var))
136+
return error;
137+
}
138+
110139
return SPV_SUCCESS;
111140
}
112141

‎source/val/validation_state.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2523,6 +2523,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
25232523
return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-08722);
25242524
case 8973:
25252525
return VUID_WRAP(VUID-StandaloneSpirv-Pointer-08973);
2526+
case 9557:
2527+
return VUID_WRAP(VUID-StandaloneSpirv-Input-09557);
25262528
case 9638:
25272529
return VUID_WRAP(VUID-StandaloneSpirv-OpTypeImage-09638);
25282530
case 9658:

‎test/val/val_interfaces_test.cpp

Lines changed: 138 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,7 +1668,7 @@ TEST_F(ValidateInterfacesTest, InvalidLocationTypePointer) {
16681668
HasSubstr("Invalid type to assign a location"));
16691669
}
16701670

1671-
TEST_F(ValidateInterfacesTest, ValidLocationTypePhysicalStorageBufferPointer) {
1671+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointer) {
16721672
const std::string text = R"(
16731673
OpCapability Shader
16741674
OpCapability PhysicalStorageBufferAddresses
@@ -1677,18 +1677,150 @@ OpEntryPoint Vertex %main "main" %var
16771677
OpDecorate %var Location 0
16781678
OpDecorate %var RestrictPointer
16791679
%void = OpTypeVoid
1680-
%int = OpTypeInt 32 0
1681-
%ptr = OpTypePointer PhysicalStorageBuffer %int
1682-
%ptr2 = OpTypePointer Input %ptr
1683-
%var = OpVariable %ptr2 Input
1680+
%uint = OpTypeInt 32 0
1681+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
1682+
%in_ptr = OpTypePointer Input %psb_ptr
1683+
%var = OpVariable %in_ptr Input
16841684
%void_fn = OpTypeFunction %void
16851685
%main = OpFunction %void None %void_fn
16861686
%entry = OpLabel
16871687
OpReturn
16881688
OpFunctionEnd
16891689
)";
16901690
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1691-
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1691+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1692+
EXPECT_THAT(getDiagnosticString(),
1693+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1694+
EXPECT_THAT(getDiagnosticString(),
1695+
HasSubstr("Input/Output interface variable id <2> contains a "
1696+
"PhysicalStorageBuffer pointer, which is not allowed"));
1697+
}
1698+
1699+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArray) {
1700+
const std::string text = R"(
1701+
OpCapability Shader
1702+
OpCapability PhysicalStorageBufferAddresses
1703+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1704+
OpEntryPoint Vertex %main "main" %var
1705+
OpDecorate %var Location 0
1706+
OpDecorate %var RestrictPointer
1707+
%void = OpTypeVoid
1708+
%uint = OpTypeInt 32 0
1709+
%uint_3 = OpConstant %uint 3
1710+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
1711+
%array = OpTypeArray %psb_ptr %uint_3
1712+
%in_ptr = OpTypePointer Input %array
1713+
%var = OpVariable %in_ptr Input
1714+
%void_fn = OpTypeFunction %void
1715+
%main = OpFunction %void None %void_fn
1716+
%entry = OpLabel
1717+
OpReturn
1718+
OpFunctionEnd
1719+
)";
1720+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1721+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1722+
EXPECT_THAT(getDiagnosticString(),
1723+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1724+
EXPECT_THAT(getDiagnosticString(),
1725+
HasSubstr("Input/Output interface variable id <2> contains a "
1726+
"PhysicalStorageBuffer pointer, which is not allowed"));
1727+
}
1728+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerStruct) {
1729+
const std::string text = R"(
1730+
OpCapability Shader
1731+
OpCapability PhysicalStorageBufferAddresses
1732+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1733+
OpEntryPoint Vertex %main "main" %var
1734+
OpDecorate %var Location 0
1735+
OpDecorate %var RestrictPointer
1736+
%void = OpTypeVoid
1737+
%int = OpTypeInt 32 1
1738+
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
1739+
%struct_0 = OpTypeStruct %int %psb_ptr
1740+
%struct_1 = OpTypeStruct %int %int
1741+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
1742+
%in_ptr = OpTypePointer Input %struct_0
1743+
%var = OpVariable %in_ptr Input
1744+
%void_fn = OpTypeFunction %void
1745+
%main = OpFunction %void None %void_fn
1746+
%entry = OpLabel
1747+
OpReturn
1748+
OpFunctionEnd
1749+
)";
1750+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1751+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1752+
EXPECT_THAT(getDiagnosticString(),
1753+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1754+
EXPECT_THAT(getDiagnosticString(),
1755+
HasSubstr("Input/Output interface variable id <2> contains a "
1756+
"PhysicalStorageBuffer pointer, which is not allowed"));
1757+
}
1758+
1759+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArrayOfStruct) {
1760+
const std::string text = R"(
1761+
OpCapability Shader
1762+
OpCapability PhysicalStorageBufferAddresses
1763+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1764+
OpEntryPoint Vertex %main "main" %var
1765+
OpDecorate %var Location 0
1766+
OpDecorate %var RestrictPointer
1767+
%void = OpTypeVoid
1768+
%int = OpTypeInt 32 1
1769+
%uint = OpTypeInt 32 0
1770+
%uint_3 = OpConstant %uint 3
1771+
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
1772+
%array_1 = OpTypeArray %psb_ptr %uint_3
1773+
%struct_0 = OpTypeStruct %int %array_1
1774+
%struct_1 = OpTypeStruct %int %int
1775+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
1776+
%array_0 = OpTypeArray %struct_0 %uint_3
1777+
%in_ptr = OpTypePointer Input %array_0
1778+
%var = OpVariable %in_ptr Input
1779+
%void_fn = OpTypeFunction %void
1780+
%main = OpFunction %void None %void_fn
1781+
%entry = OpLabel
1782+
OpReturn
1783+
OpFunctionEnd
1784+
)";
1785+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1786+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1787+
EXPECT_THAT(getDiagnosticString(),
1788+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1789+
EXPECT_THAT(getDiagnosticString(),
1790+
HasSubstr("Input/Output interface variable id <2> contains a "
1791+
"PhysicalStorageBuffer pointer, which is not allowed"));
1792+
}
1793+
1794+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerNestedStruct) {
1795+
const std::string text = R"(
1796+
OpCapability Shader
1797+
OpCapability PhysicalStorageBufferAddresses
1798+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1799+
OpEntryPoint Vertex %main "main" %var
1800+
OpDecorate %var Location 0
1801+
OpDecorate %var RestrictPointer
1802+
%void = OpTypeVoid
1803+
%int = OpTypeInt 32 1
1804+
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
1805+
%struct_0 = OpTypeStruct %int %psb_ptr
1806+
%struct_1 = OpTypeStruct %int %int
1807+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
1808+
%struct_2 = OpTypeStruct %int %struct_0
1809+
%in_ptr = OpTypePointer Input %struct_2
1810+
%var = OpVariable %in_ptr Input
1811+
%void_fn = OpTypeFunction %void
1812+
%main = OpFunction %void None %void_fn
1813+
%entry = OpLabel
1814+
OpReturn
1815+
OpFunctionEnd
1816+
)";
1817+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1818+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1819+
EXPECT_THAT(getDiagnosticString(),
1820+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1821+
EXPECT_THAT(getDiagnosticString(),
1822+
HasSubstr("Input/Output interface variable id <2> contains a "
1823+
"PhysicalStorageBuffer pointer, which is not allowed"));
16921824
}
16931825

16941826
TEST_F(ValidateInterfacesTest, UntypedVariableInputMissing) {

0 commit comments

Comments
 (0)