Skip to content

Commit 784b064

Browse files
spencer-lunargspencer-lunarg
authored
spirv-val: Validate PhysicalStorageBuffer Stage Interface (KhronosGroup#5539)
Disallow PhysicalStorageBuffer pointers in Input and Output storage classes.
1 parent a8959dc commit 784b064

File tree

3 files changed

+170
-6
lines changed

3 files changed

+170
-6
lines changed

‎source/val/validate_interfaces.cpp

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

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

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

‎source/val/validation_state.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2299,6 +2299,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
22992299
return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-08722);
23002300
case 8973:
23012301
return VUID_WRAP(VUID-StandaloneSpirv-Pointer-08973);
2302+
case 9557:
2303+
return VUID_WRAP(VUID-StandaloneSpirv-Input-09557);
23022304
default:
23032305
return ""; // unknown id
23042306
}

‎test/val/val_interfaces_test.cpp

Lines changed: 139 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1599,7 +1599,7 @@ TEST_F(ValidateInterfacesTest, InvalidLocationTypePointer) {
15991599
HasSubstr("Invalid type to assign a location"));
16001600
}
16011601

1602-
TEST_F(ValidateInterfacesTest, ValidLocationTypePhysicalStorageBufferPointer) {
1602+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointer) {
16031603
const std::string text = R"(
16041604
OpCapability Shader
16051605
OpCapability PhysicalStorageBufferAddresses
@@ -1608,18 +1608,151 @@ OpEntryPoint Vertex %main "main" %var
16081608
OpDecorate %var Location 0
16091609
OpDecorate %var RestrictPointer
16101610
%void = OpTypeVoid
1611-
%int = OpTypeInt 32 0
1612-
%ptr = OpTypePointer PhysicalStorageBuffer %int
1613-
%ptr2 = OpTypePointer Input %ptr
1614-
%var = OpVariable %ptr2 Input
1611+
%uint = OpTypeInt 32 0
1612+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
1613+
%in_ptr = OpTypePointer Input %psb_ptr
1614+
%var = OpVariable %in_ptr Input
1615+
%void_fn = OpTypeFunction %void
1616+
%main = OpFunction %void None %void_fn
1617+
%entry = OpLabel
1618+
OpReturn
1619+
OpFunctionEnd
1620+
)";
1621+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1622+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1623+
EXPECT_THAT(getDiagnosticString(),
1624+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1625+
EXPECT_THAT(getDiagnosticString(),
1626+
HasSubstr("Input/Output interface variable id <2> contains a "
1627+
"PhysicalStorageBuffer pointer, which is not allowed"));
1628+
}
1629+
1630+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArray) {
1631+
const std::string text = R"(
1632+
OpCapability Shader
1633+
OpCapability PhysicalStorageBufferAddresses
1634+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1635+
OpEntryPoint Vertex %main "main" %var
1636+
OpDecorate %var Location 0
1637+
OpDecorate %var RestrictPointer
1638+
%void = OpTypeVoid
1639+
%uint = OpTypeInt 32 0
1640+
%uint_3 = OpConstant %uint 3
1641+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
1642+
%array = OpTypeArray %psb_ptr %uint_3
1643+
%in_ptr = OpTypePointer Input %array
1644+
%var = OpVariable %in_ptr Input
1645+
%void_fn = OpTypeFunction %void
1646+
%main = OpFunction %void None %void_fn
1647+
%entry = OpLabel
1648+
OpReturn
1649+
OpFunctionEnd
1650+
)";
1651+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1652+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1653+
EXPECT_THAT(getDiagnosticString(),
1654+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1655+
EXPECT_THAT(getDiagnosticString(),
1656+
HasSubstr("Input/Output interface variable id <2> contains a "
1657+
"PhysicalStorageBuffer pointer, which is not allowed"));
1658+
}
1659+
1660+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerStruct) {
1661+
const std::string text = R"(
1662+
OpCapability Shader
1663+
OpCapability PhysicalStorageBufferAddresses
1664+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1665+
OpEntryPoint Vertex %main "main" %var
1666+
OpDecorate %var Location 0
1667+
OpDecorate %var RestrictPointer
1668+
%void = OpTypeVoid
1669+
%int = OpTypeInt 32 1
1670+
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
1671+
%struct_0 = OpTypeStruct %int %psb_ptr
1672+
%struct_1 = OpTypeStruct %int %int
1673+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
1674+
%in_ptr = OpTypePointer Input %struct_0
1675+
%var = OpVariable %in_ptr Input
16151676
%void_fn = OpTypeFunction %void
16161677
%main = OpFunction %void None %void_fn
16171678
%entry = OpLabel
16181679
OpReturn
16191680
OpFunctionEnd
16201681
)";
16211682
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1622-
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1683+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1684+
EXPECT_THAT(getDiagnosticString(),
1685+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1686+
EXPECT_THAT(getDiagnosticString(),
1687+
HasSubstr("Input/Output interface variable id <2> contains a "
1688+
"PhysicalStorageBuffer pointer, which is not allowed"));
1689+
}
1690+
1691+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArrayOfStruct) {
1692+
const std::string text = R"(
1693+
OpCapability Shader
1694+
OpCapability PhysicalStorageBufferAddresses
1695+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1696+
OpEntryPoint Vertex %main "main" %var
1697+
OpDecorate %var Location 0
1698+
OpDecorate %var RestrictPointer
1699+
%void = OpTypeVoid
1700+
%int = OpTypeInt 32 1
1701+
%uint = OpTypeInt 32 0
1702+
%uint_3 = OpConstant %uint 3
1703+
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
1704+
%array_1 = OpTypeArray %psb_ptr %uint_3
1705+
%struct_0 = OpTypeStruct %int %array_1
1706+
%struct_1 = OpTypeStruct %int %int
1707+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
1708+
%array_0 = OpTypeArray %struct_0 %uint_3
1709+
%in_ptr = OpTypePointer Input %array_0
1710+
%var = OpVariable %in_ptr Input
1711+
%void_fn = OpTypeFunction %void
1712+
%main = OpFunction %void None %void_fn
1713+
%entry = OpLabel
1714+
OpReturn
1715+
OpFunctionEnd
1716+
)";
1717+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1718+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1719+
EXPECT_THAT(getDiagnosticString(),
1720+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1721+
EXPECT_THAT(getDiagnosticString(),
1722+
HasSubstr("Input/Output interface variable id <2> contains a "
1723+
"PhysicalStorageBuffer pointer, which is not allowed"));
1724+
}
1725+
1726+
TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerNestedStruct) {
1727+
const std::string text = R"(
1728+
OpCapability Shader
1729+
OpCapability PhysicalStorageBufferAddresses
1730+
OpMemoryModel PhysicalStorageBuffer64 GLSL450
1731+
OpEntryPoint Vertex %main "main" %var
1732+
OpDecorate %var Location 0
1733+
OpDecorate %var RestrictPointer
1734+
%void = OpTypeVoid
1735+
%int = OpTypeInt 32 1
1736+
OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
1737+
%struct_0 = OpTypeStruct %int %psb_ptr
1738+
%struct_1 = OpTypeStruct %int %int
1739+
%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
1740+
%struct_2 = OpTypeStruct %int %struct_0
1741+
%in_ptr = OpTypePointer Input %struct_2
1742+
%var = OpVariable %in_ptr Input
1743+
%void_fn = OpTypeFunction %void
1744+
%main = OpFunction %void None %void_fn
1745+
%entry = OpLabel
1746+
OpReturn
1747+
OpFunctionEnd
1748+
)";
1749+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
1750+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
1751+
EXPECT_THAT(getDiagnosticString(),
1752+
AnyVUID("VUID-StandaloneSpirv-Input-09557"));
1753+
EXPECT_THAT(getDiagnosticString(),
1754+
HasSubstr("Input/Output interface variable id <2> contains a "
1755+
"PhysicalStorageBuffer pointer, which is not allowed"));
16231756
}
16241757

16251758
} // namespace

0 commit comments

Comments
 (0)