From 6c899a52f9ce673e9530bcaa4da27ff24d10c0e9 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Fri, 4 Nov 2016 18:31:21 -0400 Subject: [PATCH] Adding validation for vector data rule. Number of components in a vector can be 2 or 3 or 4. If Vector16 capability is used, 8 and 16 components are also allowed. Also added unit tests for vector data rule. --- include/spirv-tools/libspirv.h | 1 + source/CMakeLists.txt | 1 + source/validate.cpp | 3 +- source/validate.h | 6 ++ source/validate_datarules.cpp | 78 +++++++++++++++++++++ test/val/CMakeLists.txt | 7 ++ test/val/Validate.Data.cpp | 119 +++++++++++++++++++++++++++++++++ test/val/ValidateID.cpp | 3 +- 8 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 source/validate_datarules.cpp create mode 100644 test/val/Validate.Data.cpp diff --git a/include/spirv-tools/libspirv.h b/include/spirv-tools/libspirv.h index 84a3b29d9..ac63e9be5 100644 --- a/include/spirv-tools/libspirv.h +++ b/include/spirv-tools/libspirv.h @@ -53,6 +53,7 @@ typedef enum spv_result_t { SPV_ERROR_INVALID_CFG = -11, SPV_ERROR_INVALID_LAYOUT = -12, SPV_ERROR_INVALID_CAPABILITY = -13, + SPV_ERROR_INVALID_DATA = -14, // Indicates data rules validation failure. SPV_FORCE_32_BIT_ENUM(spv_result_t) } spv_result_t; diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 4b9c52d90..5439ecf74 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -158,6 +158,7 @@ set(SPIRV_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/validate_cfg.cpp ${CMAKE_CURRENT_SOURCE_DIR}/validate_id.cpp ${CMAKE_CURRENT_SOURCE_DIR}/validate_instruction.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/validate_datarules.cpp ${CMAKE_CURRENT_SOURCE_DIR}/validate_layout.cpp ${CMAKE_CURRENT_SOURCE_DIR}/val/BasicBlock.cpp ${CMAKE_CURRENT_SOURCE_DIR}/val/Construct.cpp diff --git a/source/validate.cpp b/source/validate.cpp index 71d023524..d253d1753 100644 --- a/source/validate.cpp +++ b/source/validate.cpp @@ -47,6 +47,7 @@ using std::vector; using libspirv::CfgPass; using libspirv::InstructionPass; using libspirv::ModuleLayoutPass; +using libspirv::DataRulesPass; using libspirv::IdPass; using libspirv::ValidationState_t; @@ -122,7 +123,7 @@ spv_result_t ProcessInstruction(void* user_data, _.entry_points().push_back(inst->words[2]); DebugInstructionPass(_, inst); - // TODO(umar): Perform data rules pass + if (auto error = DataRulesPass(_, inst)) return error; if (auto error = IdPass(_, inst)) return error; if (auto error = ModuleLayoutPass(_, inst)) return error; if (auto error = CfgPass(_, inst)) return error; diff --git a/source/validate.h b/source/validate.h index a19e911a9..258b0ebbb 100644 --- a/source/validate.h +++ b/source/validate.h @@ -141,6 +141,12 @@ spv_result_t CfgPass(ValidationState_t& _, /// Performs Id and SSA validation of a module spv_result_t IdPass(ValidationState_t& _, const spv_parsed_instruction_t* inst); +/// Performs validation of the Data Rules subsection of 2.16.1 Universal +/// Validation Rules. +/// TODO(ehsann): add more comments here as more validation code is added. +spv_result_t DataRulesPass(ValidationState_t& _, + const spv_parsed_instruction_t* inst); + /// Performs instruction validation. spv_result_t InstructionPass(ValidationState_t& _, const spv_parsed_instruction_t* inst); diff --git a/source/validate_datarules.cpp b/source/validate_datarules.cpp new file mode 100644 index 000000000..d21c33b3d --- /dev/null +++ b/source/validate_datarules.cpp @@ -0,0 +1,78 @@ +// Copyright (c) 2016 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. + +// Ensures Data Rules are followed according to the specifications. + +#include "validate.h" + +#include +#include +#include + +#include "diagnostic.h" +#include "opcode.h" +#include "operand.h" +#include "val/ValidationState.h" + +using libspirv::CapabilitySet; +using libspirv::DiagnosticStream; +using libspirv::ValidationState_t; + +namespace { + +// Validates that the number of components in the vector type is legal. +// Vector types can only be parameterized as having 2, 3, or 4 components. +// If the Vector16 capability is added, 8 and 16 components are also allowed. +spv_result_t ValidateNumVecComponents(ValidationState_t& _, + const spv_parsed_instruction_t* inst) { + if (inst->opcode == SpvOpTypeVector) { + // operand 2 specifies the number of components in the vector. + const uint32_t num_components = inst->words[inst->operands[2].offset]; + if (num_components == 2 || num_components == 3 || num_components == 4) { + return SPV_SUCCESS; + } + if (num_components == 8 || num_components == 16) { + if (_.HasCapability(SpvCapabilityVector16)) { + return SPV_SUCCESS; + } else { + return _.diag(SPV_ERROR_INVALID_DATA) + << "Having " << num_components << " components for " + << spvOpcodeString(static_cast(inst->opcode)) + << " requires the Vector16 capability"; + } + } + return _.diag(SPV_ERROR_INVALID_DATA) + << "Illegal number of components (" << num_components << ") for " + << spvOpcodeString(static_cast(inst->opcode)); + } + + return SPV_SUCCESS; +} + +} // anonymous namespace + +namespace libspirv { + +// Validates that Data Rules are followed according to the specifications. +// (Data Rules subsection of 2.16.1 Universal Validation Rules) +spv_result_t DataRulesPass(ValidationState_t& _, + const spv_parsed_instruction_t* inst) { + if (auto error = ValidateNumVecComponents(_, inst)) return error; + + // TODO(ehsan): add more data rules validation here. + + return SPV_SUCCESS; +} + +} // namespace libspirv diff --git a/test/val/CMakeLists.txt b/test/val/CMakeLists.txt index 3b79367c7..7988fe569 100644 --- a/test/val/CMakeLists.txt +++ b/test/val/CMakeLists.txt @@ -62,3 +62,10 @@ add_spvtools_unittest(TARGET val_state ${VAL_TEST_COMMON_SRCS} LIBS ${SPIRV_TOOLS} ) + +add_spvtools_unittest(TARGET val_data + SRCS ${CMAKE_CURRENT_SOURCE_DIR}/Validate.Data.cpp + ${VAL_TEST_COMMON_SRCS} + LIBS ${SPIRV_TOOLS} +) + diff --git a/test/val/Validate.Data.cpp b/test/val/Validate.Data.cpp new file mode 100644 index 000000000..bd19171ba --- /dev/null +++ b/test/val/Validate.Data.cpp @@ -0,0 +1,119 @@ +// Copyright (c) 2016 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. + +// Validation tests for Data Rules. + +#include "unit_spirv.h" +#include "ValidateFixtures.h" +#include "gmock/gmock.h" + +#include +#include +#include + +using ::testing::HasSubstr; +using ::testing::MatchesRegex; + +using std::string; +using std::pair; +using std::stringstream; + +using ValidateData = spvtest::ValidateBase>; + +string header = R"( + OpCapability Shader + OpMemoryModel Logical GLSL450 +%1 = OpTypeFloat 32 +)"; +string header_with_vec16_cap = R"( + OpCapability Shader + OpCapability Vector16 + OpMemoryModel Logical GLSL450 +%1 = OpTypeFloat 32 +)"; + +string invalid_comp_error = "Illegal number of components"; +string missing_cap_error = "requires the Vector16 capability"; + +TEST_F(ValidateData, vec0) { + string str = header + "%2 = OpTypeVector %1 0"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr(invalid_comp_error)); +} + +TEST_F(ValidateData, vec1) { + string str = header + "%2 = OpTypeVector %1 1"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr(invalid_comp_error)); +} + +TEST_F(ValidateData, vec2) { + string str = header + "%2 = OpTypeVector %1 2"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateData, vec3) { + string str = header + "%2 = OpTypeVector %1 3"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateData, vec4) { + string str = header + "%2 = OpTypeVector %1 4"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateData, vec5) { + string str = header + "%2 = OpTypeVector %1 5"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr(invalid_comp_error)); +} + +TEST_F(ValidateData, vec8) { + string str = header + "%2 = OpTypeVector %1 8"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr(missing_cap_error)); +} + +TEST_F(ValidateData, vec8_with_capability) { + string str = header_with_vec16_cap + "%2 = OpTypeVector %1 8"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateData, vec16) { + string str = header + "%2 = OpTypeVector %1 16"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr(missing_cap_error)); +} + +TEST_F(ValidateData, vec16_with_capability) { + string str = header_with_vec16_cap + "%2 = OpTypeVector %1 16"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateData, vec15) { + string str = header + "%2 = OpTypeVector %1 15"; + CompileSuccessfully(str.c_str()); + ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr(invalid_comp_error)); +} diff --git a/test/val/ValidateID.cpp b/test/val/ValidateID.cpp index 8a91bc980..997ece598 100644 --- a/test/val/ValidateID.cpp +++ b/test/val/ValidateID.cpp @@ -44,6 +44,7 @@ const char kGLSL450MemoryModel[] = R"( OpCapability Pipes OpCapability LiteralSampler OpCapability DeviceEnqueue + OpCapability Vector16 OpMemoryModel Logical GLSL450 )"; @@ -746,7 +747,7 @@ TEST_F(ValidateID, OpConstantCompositeArrayConstConstituentBad) { %1 = OpTypeInt 32 0 %2 = OpConstant %1 4 %3 = OpTypeArray %1 %2 -%4 = OpConstantComposite %3 %2 %2 %2 %1)"; // Uses a type as operand +%4 = OpConstantComposite %3 %2 %2 %2 %1)"; // Uses a type as operand CHECK(spirv, SPV_ERROR_INVALID_ID); } TEST_F(ValidateID, OpConstantCompositeArrayConstituentTypeBad) {