From f2a990022a4b99d11c5fd0567a7c16f67d4e3b59 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Fri, 3 Aug 2018 11:38:51 -0400 Subject: [PATCH] Move type instruction validation into separate file * Moved type instruction validation out of validation idUsage into a new file * Consolidate type unique pass into new file * Removed one bad test * Reworked validation ordering --- Android.mk | 2 +- BUILD.gn | 2 +- source/CMakeLists.txt | 2 +- source/val/validate.cpp | 38 ++-- source/val/validate.h | 6 +- source/val/validate_id.cpp | 258 ------------------------ source/val/validate_type.cpp | 301 ++++++++++++++++++++++++++++ source/val/validate_type_unique.cpp | 57 ------ test/val/val_id_test.cpp | 17 -- 9 files changed, 331 insertions(+), 352 deletions(-) create mode 100644 source/val/validate_type.cpp delete mode 100644 source/val/validate_type_unique.cpp diff --git a/Android.mk b/Android.mk index 6c5564447..1c594886c 100644 --- a/Android.mk +++ b/Android.mk @@ -59,7 +59,7 @@ SPVTOOLS_SRC_FILES := \ source/val/validate_logicals.cpp \ source/val/validate_non_uniform.cpp \ source/val/validate_primitives.cpp \ - source/val/validate_type_unique.cpp + source/val/validate_type.cpp SPVTOOLS_OPT_SRC_FILES := \ source/opt/aggressive_dead_code_elim_pass.cpp \ diff --git a/BUILD.gn b/BUILD.gn index f19ccf191..91b6929b8 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -382,7 +382,7 @@ static_library("spvtools_val") { "source/val/validate_memory.cpp", "source/val/validate_non_uniform.cpp", "source/val/validate_primitives.cpp", - "source/val/validate_type_unique.cpp", + "source/val/validate_type.cpp", "source/val/validation_state.cpp", ] diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 8dc4cb919..409946f14 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -306,7 +306,7 @@ set(SPIRV_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_memory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_non_uniform.cpp ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_primitives.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_type_unique.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/val/validate_type.cpp ${CMAKE_CURRENT_SOURCE_DIR}/val/decoration.h ${CMAKE_CURRENT_SOURCE_DIR}/val/basic_block.cpp ${CMAKE_CURRENT_SOURCE_DIR}/val/construct.cpp diff --git a/source/val/validate.cpp b/source/val/validate.cpp index d642ec0d7..1996eacb9 100644 --- a/source/val/validate.cpp +++ b/source/val/validate.cpp @@ -317,23 +317,35 @@ spv_result_t ValidateBinaryUsingContextAndValidationState( // Validate individual opcodes. for (size_t i = 0; i < vstate->ordered_instructions().size(); ++i) { auto& instruction = vstate->ordered_instructions()[i]; - if (auto error = TypeUniquePass(*vstate, &instruction)) return error; - if (auto error = ArithmeticsPass(*vstate, &instruction)) return error; - if (auto error = CompositesPass(*vstate, &instruction)) return error; - if (auto error = ConversionPass(*vstate, &instruction)) return error; - if (auto error = DerivativesPass(*vstate, &instruction)) return error; - if (auto error = LogicalsPass(*vstate, &instruction)) return error; - if (auto error = BitwisePass(*vstate, &instruction)) return error; + + // Keep these passes in the order they appear in the SPIR-V specification + // sections to maintain test consistency. + // Miscellaneous + // Debug + // Annotation if (auto error = ExtInstPass(*vstate, &instruction)) return error; - if (auto error = ImagePass(*vstate, &instruction)) return error; - if (auto error = AtomicsPass(*vstate, &instruction)) return error; - if (auto error = BarriersPass(*vstate, &instruction)) return error; - if (auto error = PrimitivesPass(*vstate, &instruction)) return error; - if (auto error = LiteralsPass(*vstate, &instruction)) return error; - if (auto error = NonUniformPass(*vstate, &instruction)) return error; + // Mode Setting + if (auto error = TypePass(*vstate, &instruction)) return error; + // Constants if (auto error = ValidateMemoryInstructions(*vstate, &instruction)) return error; + // Functions + if (auto error = ImagePass(*vstate, &instruction)) return error; + if (auto error = ConversionPass(*vstate, &instruction)) return error; + if (auto error = CompositesPass(*vstate, &instruction)) return error; + if (auto error = ArithmeticsPass(*vstate, &instruction)) return error; + if (auto error = BitwisePass(*vstate, &instruction)) return error; + if (auto error = LogicalsPass(*vstate, &instruction)) return error; + if (auto error = DerivativesPass(*vstate, &instruction)) return error; + if (auto error = AtomicsPass(*vstate, &instruction)) return error; + if (auto error = PrimitivesPass(*vstate, &instruction)) return error; + if (auto error = BarriersPass(*vstate, &instruction)) return error; + // Group + // Device-Side Enqueue + // Pipe + if (auto error = NonUniformPass(*vstate, &instruction)) return error; + if (auto error = LiteralsPass(*vstate, &instruction)) return error; // Validate the preconditions involving adjacent instructions. e.g. SpvOpPhi // must only be preceeded by SpvOpLabel, SpvOpPhi, or SpvOpLine. if (auto error = ValidateAdjacency(*vstate, i)) return error; diff --git a/source/val/validate.h b/source/val/validate.h index 3c64fb6ac..63eec7f3d 100644 --- a/source/val/validate.h +++ b/source/val/validate.h @@ -136,10 +136,8 @@ spv_result_t ValidateDecorations(ValidationState_t& _); /// Performs validation of built-in variables. spv_result_t ValidateBuiltIns(const ValidationState_t& _); -/// Validates that type declarations are unique, unless multiple declarations -/// of the same data type are allowed by the specification. -/// (see section 2.8 Types and Variables) -spv_result_t TypeUniquePass(ValidationState_t& _, const Instruction* inst); +/// Validates type instructions. +spv_result_t TypePass(ValidationState_t& _, const Instruction* inst); /// Validates correctness of arithmetic instructions. spv_result_t ArithmeticsPass(ValidationState_t& _, const Instruction* inst); diff --git a/source/val/validate_id.cpp b/source/val/validate_id.cpp index ee1f0b549..3fe864669 100644 --- a/source/val/validate_id.cpp +++ b/source/val/validate_id.cpp @@ -303,255 +303,6 @@ bool idUsage::isValid(const spv_instruction_t* inst, return true; } -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - auto componentIndex = 2; - auto componentType = module_.FindDef(inst->words[componentIndex]); - if (!componentType || !spvOpcodeIsScalarType(componentType->opcode())) { - DIAG(componentType) << "OpTypeVector Component Type '" - << module_.getIdName(inst->words[componentIndex]) - << "' is not a scalar type."; - return false; - } - return true; -} - -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - auto columnTypeIndex = 2; - auto columnType = module_.FindDef(inst->words[columnTypeIndex]); - if (!columnType || SpvOpTypeVector != columnType->opcode()) { - DIAG(columnType) << "OpTypeMatrix Column Type '" - << module_.getIdName(inst->words[columnTypeIndex]) - << "' is not a vector."; - return false; - } - return true; -} - -template <> -bool idUsage::isValid(const spv_instruction_t*, - const spv_opcode_desc) { - // OpTypeSampler takes no arguments in Rev31 and beyond. - return true; -} - -// True if the integer constant is > 0. constWords are words of the -// constant-defining instruction (either OpConstant or -// OpSpecConstant). typeWords are the words of the constant's-type-defining -// OpTypeInt. -bool aboveZero(const std::vector& constWords, - const std::vector& typeWords) { - const uint32_t width = typeWords[2]; - const bool is_signed = typeWords[3] > 0; - const uint32_t loWord = constWords[3]; - if (width > 32) { - // The spec currently doesn't allow integers wider than 64 bits. - const uint32_t hiWord = constWords[4]; // Must exist, per spec. - if (is_signed && (hiWord >> 31)) return false; - return (loWord | hiWord) > 0; - } else { - if (is_signed && (loWord >> 31)) return false; - return loWord > 0; - } -} - -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - auto elementTypeIndex = 2; - auto elementType = module_.FindDef(inst->words[elementTypeIndex]); - if (!elementType || !spvOpcodeGeneratesType(elementType->opcode())) { - DIAG(elementType) << "OpTypeArray Element Type '" - << module_.getIdName(inst->words[elementTypeIndex]) - << "' is not a type."; - return false; - } - auto lengthIndex = 3; - auto length = module_.FindDef(inst->words[lengthIndex]); - if (!length || !spvOpcodeIsConstant(length->opcode())) { - DIAG(length) << "OpTypeArray Length '" - << module_.getIdName(inst->words[lengthIndex]) - << "' is not a scalar constant type."; - return false; - } - - // NOTE: Check the initialiser value of the constant - auto constInst = length->words(); - auto constResultTypeIndex = 1; - auto constResultType = module_.FindDef(constInst[constResultTypeIndex]); - if (!constResultType || SpvOpTypeInt != constResultType->opcode()) { - DIAG(length) << "OpTypeArray Length '" - << module_.getIdName(inst->words[lengthIndex]) - << "' is not a constant integer type."; - return false; - } - - switch (length->opcode()) { - case SpvOpSpecConstant: - case SpvOpConstant: - if (aboveZero(length->words(), constResultType->words())) break; - // Else fall through! - case SpvOpConstantNull: { - DIAG(length) << "OpTypeArray Length '" - << module_.getIdName(inst->words[lengthIndex]) - << "' default value must be at least 1."; - return false; - } - case SpvOpSpecConstantOp: - // Assume it's OK, rather than try to evaluate the operation. - break; - default: - assert(0 && "bug in spvOpcodeIsConstant() or result type isn't int"); - } - return true; -} - -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - auto elementTypeIndex = 2; - auto elementType = module_.FindDef(inst->words[elementTypeIndex]); - if (!elementType || !spvOpcodeGeneratesType(elementType->opcode())) { - DIAG(elementType) << "OpTypeRuntimeArray Element Type '" - << module_.getIdName(inst->words[elementTypeIndex]) - << "' is not a type."; - return false; - } - return true; -} - -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - ValidationState_t& vstate = const_cast(module_); - const uint32_t struct_id = inst->words[1]; - auto structType = module_.FindDef(struct_id); - for (size_t memberTypeIndex = 2; memberTypeIndex < inst->words.size(); - ++memberTypeIndex) { - auto memberTypeId = inst->words[memberTypeIndex]; - auto memberType = module_.FindDef(memberTypeId); - if (!memberType || !spvOpcodeGeneratesType(memberType->opcode())) { - DIAG(memberType) << "OpTypeStruct Member Type '" - << module_.getIdName(inst->words[memberTypeIndex]) - << "' is not a type."; - return false; - } - if (SpvOpTypeStruct == memberType->opcode() && - module_.IsStructTypeWithBuiltInMember(memberTypeId)) { - DIAG(memberType) - << "Structure " << module_.getIdName(memberTypeId) - << " contains members with BuiltIn decoration. Therefore this " - "structure may not be contained as a member of another structure " - "type. Structure " - << module_.getIdName(struct_id) << " contains structure " - << module_.getIdName(memberTypeId) << "."; - return false; - } - if (module_.IsForwardPointer(memberTypeId)) { - if (memberType->opcode() != SpvOpTypePointer) { - DIAG(memberType) << "Found a forward reference to a non-pointer " - "type in OpTypeStruct instruction."; - return false; - } - // If we're dealing with a forward pointer: - // Find out the type that the pointer is pointing to (must be struct) - // word 3 is the of the type being pointed to. - auto typePointingTo = module_.FindDef(memberType->words()[3]); - if (typePointingTo && typePointingTo->opcode() != SpvOpTypeStruct) { - // Forward declared operands of a struct may only point to a struct. - DIAG(memberType) - << "A forward reference operand in an OpTypeStruct must be an " - "OpTypePointer that points to an OpTypeStruct. " - "Found OpTypePointer that points to Op" - << spvOpcodeString(static_cast(typePointingTo->opcode())) - << "."; - return false; - } - } - } - std::unordered_set built_in_members; - for (auto decoration : vstate.id_decorations(struct_id)) { - if (decoration.dec_type() == SpvDecorationBuiltIn && - decoration.struct_member_index() != Decoration::kInvalidMember) { - built_in_members.insert(decoration.struct_member_index()); - } - } - int num_struct_members = static_cast(inst->words.size() - 2); - int num_builtin_members = static_cast(built_in_members.size()); - if (num_builtin_members > 0 && num_builtin_members != num_struct_members) { - DIAG(structType) - << "When BuiltIn decoration is applied to a structure-type member, " - "all members of that structure type must also be decorated with " - "BuiltIn (No allowed mixing of built-in variables and " - "non-built-in variables within a single structure). Structure id " - << struct_id << " does not meet this requirement."; - return false; - } - if (num_builtin_members > 0) { - vstate.RegisterStructTypeWithBuiltInMember(struct_id); - } - return true; -} - -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - auto typeIndex = 3; - auto type = module_.FindDef(inst->words[typeIndex]); - if (!type || !spvOpcodeGeneratesType(type->opcode())) { - DIAG(type) << "OpTypePointer Type '" - << module_.getIdName(inst->words[typeIndex]) - << "' is not a type."; - return false; - } - return true; -} - -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - auto returnTypeIndex = 2; - auto returnType = module_.FindDef(inst->words[returnTypeIndex]); - if (!returnType || !spvOpcodeGeneratesType(returnType->opcode())) { - DIAG(returnType) << "OpTypeFunction Return Type '" - << module_.getIdName(inst->words[returnTypeIndex]) - << "' is not a type."; - return false; - } - size_t num_args = 0; - for (size_t paramTypeIndex = 3; paramTypeIndex < inst->words.size(); - ++paramTypeIndex, ++num_args) { - auto paramType = module_.FindDef(inst->words[paramTypeIndex]); - if (!paramType || !spvOpcodeGeneratesType(paramType->opcode())) { - DIAG(paramType) << "OpTypeFunction Parameter Type '" - << module_.getIdName(inst->words[paramTypeIndex]) - << "' is not a type."; - return false; - } - } - const uint32_t num_function_args_limit = - module_.options()->universal_limits_.max_function_args; - if (num_args > num_function_args_limit) { - DIAG(returnType) << "OpTypeFunction may not take more than " - << num_function_args_limit - << " arguments. OpTypeFunction '" - << module_.getIdName(inst->words[1]) << "' has " - << num_args << " arguments."; - return false; - } - return true; -} - -template <> -bool idUsage::isValid(const spv_instruction_t*, - const spv_opcode_desc) { - // OpTypePipe has no ID arguments. - return true; -} - template <> bool idUsage::isValid(const spv_instruction_t* inst, const spv_opcode_desc) { @@ -1441,15 +1192,6 @@ bool idUsage::isValid(const spv_instruction_t* inst) { CASE(OpGroupMemberDecorate) CASE(OpEntryPoint) CASE(OpExecutionMode) - CASE(OpTypeVector) - CASE(OpTypeMatrix) - CASE(OpTypeSampler) - CASE(OpTypeArray) - CASE(OpTypeRuntimeArray) - CASE(OpTypeStruct) - CASE(OpTypePointer) - CASE(OpTypeFunction) - CASE(OpTypePipe) CASE(OpConstantTrue) CASE(OpConstantFalse) CASE(OpConstantComposite) diff --git a/source/val/validate_type.cpp b/source/val/validate_type.cpp new file mode 100644 index 000000000..4979c1249 --- /dev/null +++ b/source/val/validate_type.cpp @@ -0,0 +1,301 @@ +// Copyright (c) 2018 Google LLC. +// +// 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 type declarations are unique unless allowed by the specification. + +#include "source/val/validate.h" + +#include "source/opcode.h" +#include "source/val/instruction.h" +#include "source/val/validation_state.h" + +namespace spvtools { +namespace val { +namespace { + +// True if the integer constant is > 0. |const_words| are words of the +// constant-defining instruction (either OpConstant or +// OpSpecConstant). typeWords are the words of the constant's-type-defining +// OpTypeInt. +bool AboveZero(const std::vector& const_words, + const std::vector& type_words) { + const uint32_t width = type_words[2]; + const bool is_signed = type_words[3] > 0; + const uint32_t lo_word = const_words[3]; + if (width > 32) { + // The spec currently doesn't allow integers wider than 64 bits. + const uint32_t hi_word = const_words[4]; // Must exist, per spec. + if (is_signed && (hi_word >> 31)) return false; + return (lo_word | hi_word) > 0; + } else { + if (is_signed && (lo_word >> 31)) return false; + return lo_word > 0; + } +} + +// Validates that type declarations are unique, unless multiple declarations +// of the same data type are allowed by the specification. +// (see section 2.8 Types and Variables) +// Doesn't do anything if SPV_VAL_ignore_type_decl_unique was declared in the +// module. +spv_result_t ValidateUniqueness(ValidationState_t& _, const Instruction* inst) { + if (_.HasExtension(Extension::kSPV_VALIDATOR_ignore_type_decl_unique)) + return SPV_SUCCESS; + + const auto opcode = inst->opcode(); + if (opcode != SpvOpTypeArray && opcode != SpvOpTypeRuntimeArray && + opcode != SpvOpTypeStruct && opcode != SpvOpTypePointer && + !_.RegisterUniqueTypeDeclaration(inst)) { + return _.diag(SPV_ERROR_INVALID_DATA, inst) + << "Duplicate non-aggregate type declarations are not allowed. " + "Opcode: " + << spvOpcodeString(opcode) << " id: " << inst->id(); + } + + return SPV_SUCCESS; +} + +spv_result_t ValidateTypeVector(ValidationState_t& _, const Instruction* inst) { + const auto component_index = 1; + const auto component_id = inst->GetOperandAs(component_index); + const auto component_type = _.FindDef(component_id); + if (!component_type || !spvOpcodeIsScalarType(component_type->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeVector Component Type '" << _.getIdName(component_id) + << "' is not a scalar type."; + } + return SPV_SUCCESS; +} + +spv_result_t ValidateTypeMatrix(ValidationState_t& _, const Instruction* inst) { + const auto column_type_index = 1; + const auto column_type_id = inst->GetOperandAs(column_type_index); + const auto column_type = _.FindDef(column_type_id); + if (!column_type || SpvOpTypeVector != column_type->opcode()) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeMatrix Column Type '" << _.getIdName(column_type_id) + << "' is not a vector."; + } + return SPV_SUCCESS; +} + +spv_result_t ValidateTypeArray(ValidationState_t& _, const Instruction* inst) { + const auto element_type_index = 1; + const auto element_type_id = inst->GetOperandAs(element_type_index); + const auto element_type = _.FindDef(element_type_id); + if (!element_type || !spvOpcodeGeneratesType(element_type->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeArray Element Type '" << _.getIdName(element_type_id) + << "' is not a type."; + } + const auto length_index = 2; + const auto length_id = inst->GetOperandAs(length_index); + const auto length = _.FindDef(length_id); + if (!length || !spvOpcodeIsConstant(length->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeArray Length '" << _.getIdName(length_id) + << "' is not a scalar constant type."; + } + + // NOTE: Check the initialiser value of the constant + const auto const_inst = length->words(); + const auto const_result_type_index = 1; + const auto const_result_type = _.FindDef(const_inst[const_result_type_index]); + if (!const_result_type || SpvOpTypeInt != const_result_type->opcode()) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeArray Length '" << _.getIdName(length_id) + << "' is not a constant integer type."; + } + + switch (length->opcode()) { + case SpvOpSpecConstant: + case SpvOpConstant: + if (AboveZero(length->words(), const_result_type->words())) break; + // Else fall through! + case SpvOpConstantNull: { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeArray Length '" << _.getIdName(length_id) + << "' default value must be at least 1."; + } + case SpvOpSpecConstantOp: + // Assume it's OK, rather than try to evaluate the operation. + break; + default: + assert(0 && "bug in spvOpcodeIsConstant() or result type isn't int"); + } + return SPV_SUCCESS; +} + +spv_result_t ValidateTypeRuntimeArray(ValidationState_t& _, + const Instruction* inst) { + const auto element_type_index = 1; + const auto element_id = inst->GetOperandAs(element_type_index); + const auto element_type = _.FindDef(element_id); + if (!element_type || !spvOpcodeGeneratesType(element_type->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeRuntimeArray Element Type '" + << _.getIdName(element_id) << "' is not a type."; + } + return SPV_SUCCESS; +} + +spv_result_t ValidateTypeStruct(ValidationState_t& _, const Instruction* inst) { + const uint32_t struct_id = inst->GetOperandAs(0); + for (size_t member_type_index = 1; + member_type_index < inst->operands().size(); ++member_type_index) { + auto member_type_id = inst->GetOperandAs(member_type_index); + auto member_type = _.FindDef(member_type_id); + if (!member_type || !spvOpcodeGeneratesType(member_type->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeStruct Member Type '" << _.getIdName(member_type_id) + << "' is not a type."; + } + if (SpvOpTypeStruct == member_type->opcode() && + _.IsStructTypeWithBuiltInMember(member_type_id)) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "Structure " << _.getIdName(member_type_id) + << " contains members with BuiltIn decoration. Therefore this " + "structure may not be contained as a member of another " + "structure " + "type. Structure " + << _.getIdName(struct_id) << " contains structure " + << _.getIdName(member_type_id) << "."; + } + if (_.IsForwardPointer(member_type_id)) { + if (member_type->opcode() != SpvOpTypePointer) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "Found a forward reference to a non-pointer " + "type in OpTypeStruct instruction."; + } + // If we're dealing with a forward pointer: + // Find out the type that the pointer is pointing to (must be struct) + // word 3 is the of the type being pointed to. + auto type_pointing_to = _.FindDef(member_type->words()[3]); + if (type_pointing_to && type_pointing_to->opcode() != SpvOpTypeStruct) { + // Forward declared operands of a struct may only point to a struct. + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "A forward reference operand in an OpTypeStruct must be an " + "OpTypePointer that points to an OpTypeStruct. " + "Found OpTypePointer that points to Op" + << spvOpcodeString( + static_cast(type_pointing_to->opcode())) + << "."; + } + } + } + std::unordered_set built_in_members; + for (auto decoration : _.id_decorations(struct_id)) { + if (decoration.dec_type() == SpvDecorationBuiltIn && + decoration.struct_member_index() != Decoration::kInvalidMember) { + built_in_members.insert(decoration.struct_member_index()); + } + } + int num_struct_members = static_cast(inst->operands().size() - 1); + int num_builtin_members = static_cast(built_in_members.size()); + if (num_builtin_members > 0 && num_builtin_members != num_struct_members) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "When BuiltIn decoration is applied to a structure-type member, " + "all members of that structure type must also be decorated with " + "BuiltIn (No allowed mixing of built-in variables and " + "non-built-in variables within a single structure). Structure id " + << struct_id << " does not meet this requirement."; + } + if (num_builtin_members > 0) { + _.RegisterStructTypeWithBuiltInMember(struct_id); + } + return SPV_SUCCESS; +} + +spv_result_t ValidateTypePointer(ValidationState_t& _, + const Instruction* inst) { + const auto type_id = inst->GetOperandAs(2); + const auto type = _.FindDef(type_id); + if (!type || !spvOpcodeGeneratesType(type->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypePointer Type '" << _.getIdName(type_id) + << "' is not a type."; + } + return SPV_SUCCESS; +} + +spv_result_t ValidateTypeFunction(ValidationState_t& _, + const Instruction* inst) { + const auto return_type_id = inst->GetOperandAs(1); + const auto return_type = _.FindDef(return_type_id); + if (!return_type || !spvOpcodeGeneratesType(return_type->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeFunction Return Type '" << _.getIdName(return_type_id) + << "' is not a type."; + } + size_t num_args = 0; + for (size_t param_type_index = 2; param_type_index < inst->operands().size(); + ++param_type_index, ++num_args) { + const auto param_id = inst->GetOperandAs(param_type_index); + const auto param_type = _.FindDef(param_id); + if (!param_type || !spvOpcodeGeneratesType(param_type->opcode())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeFunction Parameter Type '" << _.getIdName(param_id) + << "' is not a type."; + } + } + const uint32_t num_function_args_limit = + _.options()->universal_limits_.max_function_args; + if (num_args > num_function_args_limit) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "OpTypeFunction may not take more than " + << num_function_args_limit << " arguments. OpTypeFunction '" + << _.getIdName(inst->GetOperandAs(0)) << "' has " + << num_args << " arguments."; + } + return SPV_SUCCESS; +} + +} // namespace + +spv_result_t TypePass(ValidationState_t& _, const Instruction* inst) { + if (!spvOpcodeGeneratesType(inst->opcode())) return SPV_SUCCESS; + + if (auto error = ValidateUniqueness(_, inst)) return error; + + switch (inst->opcode()) { + case SpvOpTypeVector: + if (auto error = ValidateTypeVector(_, inst)) return error; + break; + case SpvOpTypeMatrix: + if (auto error = ValidateTypeMatrix(_, inst)) return error; + break; + case SpvOpTypeArray: + if (auto error = ValidateTypeArray(_, inst)) return error; + break; + case SpvOpTypeRuntimeArray: + if (auto error = ValidateTypeRuntimeArray(_, inst)) return error; + break; + case SpvOpTypeStruct: + if (auto error = ValidateTypeStruct(_, inst)) return error; + break; + case SpvOpTypePointer: + if (auto error = ValidateTypePointer(_, inst)) return error; + break; + case SpvOpTypeFunction: + if (auto error = ValidateTypeFunction(_, inst)) return error; + break; + default: + break; + } + + return SPV_SUCCESS; +} + +} // namespace val +} // namespace spvtools diff --git a/source/val/validate_type_unique.cpp b/source/val/validate_type_unique.cpp deleted file mode 100644 index 1f4de1054..000000000 --- a/source/val/validate_type_unique.cpp +++ /dev/null @@ -1,57 +0,0 @@ -// 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. - -// Ensures type declarations are unique unless allowed by the specification. - -#include "source/val/validate.h" - -#include "source/diagnostic.h" -#include "source/opcode.h" -#include "source/val/instruction.h" -#include "source/val/validation_state.h" - -namespace spvtools { -namespace val { - -// Validates that type declarations are unique, unless multiple declarations -// of the same data type are allowed by the specification. -// (see section 2.8 Types and Variables) -// Doesn't do anything if SPV_VAL_ignore_type_decl_unique was declared in the -// module. -spv_result_t TypeUniquePass(ValidationState_t& _, const Instruction* inst) { - if (_.HasExtension(Extension::kSPV_VALIDATOR_ignore_type_decl_unique)) - return SPV_SUCCESS; - - const SpvOp opcode = inst->opcode(); - - if (spvOpcodeGeneratesType(opcode)) { - if (opcode == SpvOpTypeArray || opcode == SpvOpTypeRuntimeArray || - opcode == SpvOpTypeStruct || opcode == SpvOpTypePointer) { - // Duplicate declarations of aggregates are allowed. - return SPV_SUCCESS; - } - - if (!_.RegisterUniqueTypeDeclaration(inst)) { - return _.diag(SPV_ERROR_INVALID_DATA, inst) - << "Duplicate non-aggregate type declarations are not allowed." - << " Opcode: " << spvOpcodeString(SpvOp(inst->opcode())) - << " id: " << inst->id(); - } - } - - return SPV_SUCCESS; -} - -} // namespace val -} // namespace spvtools diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp index 6d4832747..e577962a0 100644 --- a/test/val/val_id_test.cpp +++ b/test/val/val_id_test.cpp @@ -1037,23 +1037,6 @@ TEST_F(ValidateIdWithMessage, "component count does not match Result Type '4's " "vector component count.")); } -TEST_F(ValidateIdWithMessage, OpConstantCompositeMatrixColumnTypeBad) { - std::string spirv = kGLSL450MemoryModel + R"( - %1 = OpTypeInt 32 0 - %2 = OpTypeFloat 32 - %3 = OpTypeVector %1 2 - %4 = OpTypeVector %3 2 - %5 = OpTypeMatrix %2 2 - %6 = OpConstant %1 42 - %7 = OpConstant %2 3.14 - %8 = OpConstantComposite %3 %6 %6 - %9 = OpConstantComposite %4 %7 %7 -%10 = OpConstantComposite %5 %8 %9)"; - CompileSuccessfully(spirv.c_str()); - EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); - EXPECT_THAT(getDiagnosticString(), - HasSubstr("Columns in a matrix must be of type vector.")); -} TEST_F(ValidateIdWithMessage, OpConstantCompositeArrayGood) { std::string spirv = kGLSL450MemoryModel + R"( %1 = OpTypeInt 32 0