From 1c43cb855070c83f784e7b8f12a76b53e7634b1e Mon Sep 17 00:00:00 2001 From: Andrey Tuganov Date: Wed, 8 Mar 2017 13:59:01 -0500 Subject: [PATCH] Validator parses and registers OpExtension Known extensions are saved in validation state. Unknown extension produce a dignostic message, but do not fail the validation. Moved extension definitions to their own file. --- source/CMakeLists.txt | 1 + source/extensions.cpp | 51 +++++++++++++++++++++++++ source/extensions.h | 50 +++++++++++++++++++++++++ source/table.h | 16 ++------ source/val/validation_state.cpp | 7 ++++ source/val/validation_state.h | 11 ++++-- source/validate_instruction.cpp | 25 +++++++++++++ test/val/CMakeLists.txt | 6 +++ test/val/val_extensions_test.cpp | 80 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 231 insertions(+), 16 deletions(-) create mode 100644 source/extensions.cpp create mode 100644 source/extensions.h create mode 100644 test/val/val_extensions_test.cpp diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 05325d2..0314895 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -177,6 +177,7 @@ set(SPIRV_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/diagnostic.cpp ${CMAKE_CURRENT_SOURCE_DIR}/disassemble.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ext_inst.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/extensions.cpp ${CMAKE_CURRENT_SOURCE_DIR}/libspirv.cpp ${CMAKE_CURRENT_SOURCE_DIR}/message.cpp ${CMAKE_CURRENT_SOURCE_DIR}/name_mapper.cpp diff --git a/source/extensions.cpp b/source/extensions.cpp new file mode 100644 index 0000000..5f0bf99 --- /dev/null +++ b/source/extensions.cpp @@ -0,0 +1,51 @@ +// Copyright (c) 2017 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "extensions.h" + +#include + +namespace libspirv { + +bool ParseSpvExtensionFromString(const std::string& str, Extension* extension) { + if (str == "SPV_KHR_shader_ballot") { + *extension = Extension::kSPV_KHR_shader_ballot; + } else if (str == "SPV_KHR_shader_draw_parameters") { + *extension = Extension::kSPV_KHR_shader_draw_parameters; + } else if (str == "SPV_KHR_subgroup_vote") { + *extension = Extension::kSPV_KHR_subgroup_vote; + } else if (str == "SPV_KHR_16bit_storage") { + *extension = Extension::kSPV_KHR_16bit_storage; + } else if (str == "SPV_KHR_device_group") { + *extension = Extension::kSPV_KHR_device_group; + } else if (str == "SPV_KHR_multiview") { + *extension = Extension::kSPV_KHR_multiview; + } else if (str == "SPV_NV_sample_mask_override_coverage") { + *extension = Extension::kSPV_NV_sample_mask_override_coverage; + } else if (str == "SPV_NV_geometry_shader_passthrough") { + *extension = Extension::kSPV_NV_geometry_shader_passthrough; + } else if (str == "SPV_NV_viewport_array2") { + *extension = Extension::kSPV_NV_viewport_array2; + } else if (str == "SPV_NV_stereo_view_rendering") { + *extension = Extension::kSPV_NV_stereo_view_rendering; + } else if (str == "SPV_NVX_multiview_per_view_attributes") { + *extension = Extension::kSPV_NVX_multiview_per_view_attributes; + } else { + return false; + } + + return true; +} + +} // namespace libspirv diff --git a/source/extensions.h b/source/extensions.h new file mode 100644 index 0000000..0be5512 --- /dev/null +++ b/source/extensions.h @@ -0,0 +1,50 @@ +// Copyright (c) 2017 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef LIBSPIRV_EXTENSIONS_H_ +#define LIBSPIRV_EXTENSIONS_H_ + +#include + +#include "enum_set.h" +#include "message.h" +#include "spirv-tools/libspirv.hpp" + +namespace libspirv { + +// The known SPIR-V extensions. +// TODO(dneto): Consider auto-generating this list? +// When updating this list, consider also updating ParseSpvExtensionFromString. +enum class Extension { + kSPV_KHR_shader_ballot, + kSPV_KHR_shader_draw_parameters, + kSPV_KHR_subgroup_vote, + kSPV_KHR_16bit_storage, + kSPV_KHR_device_group, + kSPV_KHR_multiview, + kSPV_NV_sample_mask_override_coverage, + kSPV_NV_geometry_shader_passthrough, + kSPV_NV_viewport_array2, + kSPV_NV_stereo_view_rendering, + kSPV_NVX_multiview_per_view_attributes, +}; + +using ExtensionSet = EnumSet; + +// Finds Extension enum corresponding to |str|. Returns false if not found. +bool ParseSpvExtensionFromString(const std::string& str, Extension* extension); + +} // namespace libspirv + +#endif // LIBSPIRV_EXTENSIONS_H_ diff --git a/source/table.h b/source/table.h index 4b21587..6dca9e7 100644 --- a/source/table.h +++ b/source/table.h @@ -15,24 +15,14 @@ #ifndef LIBSPIRV_TABLE_H_ #define LIBSPIRV_TABLE_H_ +#include + #include "spirv/1.1/spirv.h" -#include "enum_set.h" +#include "extensions.h" #include "message.h" #include "spirv-tools/libspirv.hpp" -namespace libspirv { - -// The known SPIR-V extensions. -// TODO(dneto): Consider auto-generating this list? -enum class Extension { - kSPV_KHR_shader_ballot, -}; - -using ExtensionSet = EnumSet; - -} // namespace libspirv - typedef struct spv_opcode_desc_t { const char* name; const SpvOp opcode; diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp index db3ceb8..5845cb1 100644 --- a/source/val/validation_state.cpp +++ b/source/val/validation_state.cpp @@ -139,6 +139,7 @@ ValidationState_t::ValidationState_t(const spv_const_context ctx, current_layout_section_(kLayoutCapabilities), module_functions_(), module_capabilities_(), + module_extensions_(), ordered_instructions_(), all_definitions_(), global_vars_(), @@ -304,6 +305,12 @@ void ValidationState_t::RegisterCapability(SpvCapability cap) { } } +void ValidationState_t::RegisterExtension(Extension ext) { + if (module_extensions_.Contains(ext)) return; + + module_extensions_.Add(ext); +} + bool ValidationState_t::HasAnyOf(const CapabilitySet& capabilities) const { bool found = false; bool any_queried = false; diff --git a/source/val/validation_state.h b/source/val/validation_state.h index 30a1b2d..dbc4ca6 100644 --- a/source/val/validation_state.h +++ b/source/val/validation_state.h @@ -175,6 +175,9 @@ class ValidationState_t { /// Registers the capability and its dependent capabilities void RegisterCapability(SpvCapability cap); + /// Registers the extension. + void RegisterExtension(Extension ext); + /// Registers the function in the module. Subsequent instructions will be /// called against this function spv_result_t RegisterFunction(uint32_t id, uint32_t ret_type_id, @@ -342,9 +345,11 @@ class ValidationState_t { /// A list of functions in the module std::deque module_functions_; - /// The capabilities available in the module - libspirv::CapabilitySet - module_capabilities_; /// Module's declared capabilities. + /// Capabilities declared in the module + libspirv::CapabilitySet module_capabilities_; + + /// Extensions declared in the module + libspirv::ExtensionSet module_extensions_; /// List of all instructions in the order they appear in the binary std::deque ordered_instructions_; diff --git a/source/validate_instruction.cpp b/source/validate_instruction.cpp index d3b448c..502ae3b 100644 --- a/source/validate_instruction.cpp +++ b/source/validate_instruction.cpp @@ -22,6 +22,7 @@ #include #include +#include "binary.h" #include "diagnostic.h" #include "enum_set.h" #include "opcode.h" @@ -340,9 +341,33 @@ spv_result_t RegisterDecorations(ValidationState_t& _, return SPV_SUCCESS; } +// Parses OpExtension instruction and registers extension. +void RegisterExtension(ValidationState_t& _, + const spv_parsed_instruction_t* inst) { + assert(inst->opcode == SpvOpExtension); + assert(inst->num_operands == 1); + + const auto& operand = inst->operands[0]; + assert(operand.type == SPV_OPERAND_TYPE_LITERAL_STRING); + assert(inst->num_words > operand.offset); + + const char* extension_str = + reinterpret_cast(inst->words + operand.offset); + + Extension extension; + if (!ParseSpvExtensionFromString(extension_str, &extension)) { + _.diag(SPV_SUCCESS) << "Failed to parse OpExtension " << extension_str; + return; + } + + _.RegisterExtension(extension); +} + spv_result_t InstructionPass(ValidationState_t& _, const spv_parsed_instruction_t* inst) { const SpvOp opcode = static_cast(inst->opcode); + if (opcode == SpvOpExtension) + RegisterExtension(_, inst); if (opcode == SpvOpCapability) _.RegisterCapability( static_cast(inst->words[inst->operands[0].offset])); diff --git a/test/val/CMakeLists.txt b/test/val/CMakeLists.txt index 3b9229d..9f59eb1 100644 --- a/test/val/CMakeLists.txt +++ b/test/val/CMakeLists.txt @@ -98,3 +98,9 @@ add_spvtools_unittest(TARGET val_instructions ${VAL_TEST_COMMON_SRCS} LIBS ${SPIRV_TOOLS} ) + +add_spvtools_unittest(TARGET val_extensions + SRCS val_extensions_test.cpp + ${VAL_TEST_COMMON_SRCS} + LIBS ${SPIRV_TOOLS} +) diff --git a/test/val/val_extensions_test.cpp b/test/val/val_extensions_test.cpp new file mode 100644 index 0000000..fa30565 --- /dev/null +++ b/test/val/val_extensions_test.cpp @@ -0,0 +1,80 @@ +// Copyright (c) 2017 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Tests for OpExtension validator rules. + +#include + +#include "gmock/gmock.h" +#include "test_fixture.h" +#include "unit_spirv.h" +#include "val_fixtures.h" + +namespace { + +using ::testing::HasSubstr; +using ::testing::Not; +using ::testing::Values; + +using std::string; + +using ValidateKnownExtensions = spvtest::ValidateBase; +using ValidateUnknownExtensions = spvtest::ValidateBase; + +// Returns expected error string if |extension| is not recognized. +string GetErrorString(const std::string& extension) { + return "Failed to parse OpExtension " + extension; +} + +INSTANTIATE_TEST_CASE_P(ExpectSuccess, ValidateKnownExtensions, Values( + "SPV_KHR_shader_ballot", + "SPV_KHR_shader_draw_parameters", + "SPV_KHR_subgroup_vote", + "SPV_KHR_16bit_storage", + "SPV_KHR_device_group", + "SPV_KHR_multiview", + "SPV_NV_sample_mask_override_coverage", + "SPV_NV_geometry_shader_passthrough", + "SPV_NV_viewport_array2", + "SPV_NV_stereo_view_rendering", + "SPV_NVX_multiview_per_view_attributes" + )); + +INSTANTIATE_TEST_CASE_P(FailSilently, ValidateUnknownExtensions, Values( + "ERROR_unknown_extension", + "SPV_KHR_", + "SPV_KHR_shader_ballot_ERROR" + )); + +TEST_P(ValidateKnownExtensions, ExpectSuccess) { + const std::string extension = GetParam(); + const string str = + "OpCapability Shader\nOpCapability Linkage\nOpExtension \"" + extension + + "\"\nOpMemoryModel Logical GLSL450"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), Not(HasSubstr(GetErrorString(extension)))); +} + +TEST_P(ValidateUnknownExtensions, FailSilently) { + const std::string extension = GetParam(); + const string str = + "OpCapability Shader\nOpCapability Linkage\nOpExtension \"" + extension + + "\"\nOpMemoryModel Logical GLSL450"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr(GetErrorString(extension))); +} + +} // anonymous namespace -- 2.7.4