Skip to content

Commit f9184c6

Browse files
spencer-lunargspencer-lunarg
authored
spirv-val: Revert Validate PhysicalStorageBuffer Stage Interface (KhronosGroup#5575)
1 parent 20ad38c commit f9184c6

File tree

3 files changed

+6
-170
lines changed

3 files changed

+6
-170
lines changed

‎source/val/validate_interfaces.cpp‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -46,29 +46,6 @@ 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-
7249
// Checks that \c var is listed as an interface in all the entry points that use
7350
// it.
7451
spv_result_t check_interface_variable(ValidationState_t& _,
@@ -128,12 +105,6 @@ spv_result_t check_interface_variable(ValidationState_t& _,
128105
}
129106
}
130107

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-
137108
return SPV_SUCCESS;
138109
}
139110

‎source/val/validation_state.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2307,8 +2307,6 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
23072307
return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-08722);
23082308
case 8973:
23092309
return VUID_WRAP(VUID-StandaloneSpirv-Pointer-08973);
2310-
case 9557:
2311-
return VUID_WRAP(VUID-StandaloneSpirv-Input-09557);
23122310
default:
23132311
return ""; // unknown id
23142312
}

‎test/val/val_interfaces_test.cpp‎

Lines changed: 6 additions & 139 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, PhysicalStorageBufferPointer) {
1602+
TEST_F(ValidateInterfacesTest, ValidLocationTypePhysicalStorageBufferPointer) {
16031603
const std::string text = R"(
16041604
OpCapability Shader
16051605
OpCapability PhysicalStorageBufferAddresses
@@ -1608,151 +1608,18 @@ OpEntryPoint Vertex %main "main" %var
16081608
OpDecorate %var Location 0
16091609
OpDecorate %var RestrictPointer
16101610
%void = OpTypeVoid
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
1676-
%void_fn = OpTypeFunction %void
1677-
%main = OpFunction %void None %void_fn
1678-
%entry = OpLabel
1679-
OpReturn
1680-
OpFunctionEnd
1681-
)";
1682-
CompileSuccessfully(text, 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
1611+
%int = OpTypeInt 32 0
1612+
%ptr = OpTypePointer PhysicalStorageBuffer %int
1613+
%ptr2 = OpTypePointer Input %ptr
1614+
%var = OpVariable %ptr2 Input
17431615
%void_fn = OpTypeFunction %void
17441616
%main = OpFunction %void None %void_fn
17451617
%entry = OpLabel
17461618
OpReturn
17471619
OpFunctionEnd
17481620
)";
17491621
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"));
1622+
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
17561623
}
17571624

17581625
} // namespace

0 commit comments

Comments
 (0)