From d94a2077d68c8a37b3373a8e1b3734f9ec97d6b9 Mon Sep 17 00:00:00 2001 From: alan-baker <33432579+alan-baker@users.noreply.github.com> Date: Mon, 27 Aug 2018 11:06:09 -0400 Subject: [PATCH] Remove idUsage * Moved remaining validation out of idUsage and deleted it * Deleted unused functions --- source/val/validate.cpp | 32 -------- source/val/validate.h | 13 ---- source/val/validate_id.cpp | 139 ---------------------------------- source/val/validate_image.cpp | 41 ++++++++++ 4 files changed, 41 insertions(+), 184 deletions(-) diff --git a/source/val/validate.cpp b/source/val/validate.cpp index 47e572b42..84ec193a1 100644 --- a/source/val/validate.cpp +++ b/source/val/validate.cpp @@ -46,16 +46,6 @@ namespace spvtools { namespace val { namespace { -spv_result_t spvValidateIDs(const spv_instruction_t* pInsts, - const uint64_t count, - const ValidationState_t& state, - spv_position position) { - position->index = SPV_INDEX_INSTRUCTION; - if (auto error = spvValidateInstructionIDs(pInsts, count, state, position)) - return error; - return SPV_SUCCESS; -} - // TODO(umar): Validate header // TODO(umar): The binary parser validates the magic word, and the length of the // header, but nothing else. @@ -364,28 +354,6 @@ spv_result_t ValidateBinaryUsingContextAndValidationState( if (auto error = ValidateExecutionLimitations(*vstate, &inst)) return error; } - // NOTE: Copy each instruction for easier processing - std::vector instructions; - // Expect average instruction length to be a bit over 2 words. - instructions.reserve(binary->wordCount / 2); - uint64_t index = SPV_INDEX_INSTRUCTION; - while (index < binary->wordCount) { - uint16_t wordCount; - uint16_t opcode; - spvOpcodeSplit(spvFixWord(binary->code[index], endian), &wordCount, - &opcode); - spv_instruction_t inst; - spvInstructionCopy(&binary->code[index], static_cast(opcode), - wordCount, endian, &inst); - instructions.emplace_back(std::move(inst)); - index += wordCount; - } - - position.index = SPV_INDEX_INSTRUCTION; - if (auto error = spvValidateIDs(instructions.data(), instructions.size(), - *vstate, &position)) - return error; - return SPV_SUCCESS; } diff --git a/source/val/validate.h b/source/val/validate.h index 01281180b..4599c4a86 100644 --- a/source/val/validate.h +++ b/source/val/validate.h @@ -205,19 +205,6 @@ spv_result_t FunctionPass(ValidationState_t& _, const Instruction* inst); spv_result_t ValidateExecutionLimitations(ValidationState_t& _, const Instruction* inst); -/// @brief Validate the ID usage of the instruction stream -/// -/// @param[in] pInsts stream of instructions -/// @param[in] instCount number of instructions -/// @param[in] usedefs use-def info from module parsing -/// @param[in,out] position current position in the stream -/// -/// @return result code -spv_result_t spvValidateInstructionIDs(const spv_instruction_t* pInsts, - const uint64_t instCount, - const ValidationState_t& state, - spv_position position); - /// @brief Validate the ID's within a SPIR-V binary /// /// @param[in] pInstructions array of instructions diff --git a/source/val/validate_id.cpp b/source/val/validate_id.cpp index 9510bb0b3..4cb1431d9 100644 --- a/source/val/validate_id.cpp +++ b/source/val/validate_id.cpp @@ -36,132 +36,6 @@ namespace spvtools { namespace val { -namespace { - -class idUsage { - public: - idUsage(spv_const_context context, const spv_instruction_t* pInsts, - const uint64_t instCountArg, const SpvMemoryModel memoryModelArg, - const SpvAddressingModel addressingModelArg, - const ValidationState_t& module, - const std::vector& entry_points, spv_position positionArg, - const MessageConsumer& consumer) - : targetEnv(context->target_env), - opcodeTable(context->opcode_table), - operandTable(context->operand_table), - extInstTable(context->ext_inst_table), - firstInst(pInsts), - instCount(instCountArg), - memoryModel(memoryModelArg), - addressingModel(addressingModelArg), - position(positionArg), - consumer_(consumer), - module_(module), - entry_points_(entry_points) {} - - bool isValid(const spv_instruction_t* inst); - - template - bool isValid(const spv_instruction_t* inst, const spv_opcode_desc); - - private: - const spv_target_env targetEnv; - const spv_opcode_table opcodeTable; - const spv_operand_table operandTable; - const spv_ext_inst_table extInstTable; - const spv_instruction_t* const firstInst; - const uint64_t instCount; - const SpvMemoryModel memoryModel; - const SpvAddressingModel addressingModel; - spv_position position; - const MessageConsumer& consumer_; - const ValidationState_t& module_; - std::vector entry_points_; -}; - -#define DIAG(inst) \ - position->index = inst ? inst->LineNum() : -1; \ - std::string disassembly; \ - if (inst) { \ - disassembly = module_.Disassemble( \ - inst->words().data(), static_cast(inst->words().size())); \ - } \ - DiagnosticStream helper(*position, consumer_, disassembly, \ - SPV_ERROR_INVALID_DIAGNOSTIC); \ - helper - -template <> -bool idUsage::isValid(const spv_instruction_t* inst, - const spv_opcode_desc) { - auto resultTypeIndex = 2; - auto resultID = inst->words[resultTypeIndex]; - auto sampledImageInstr = module_.FindDef(resultID); - // We need to validate 2 things: - // * All OpSampledImage instructions must be in the same block in which their - // Result are consumed. - // * Result from OpSampledImage instructions must not appear as operands - // to OpPhi instructions or OpSelect instructions, or any instructions other - // than the image lookup and query instructions specified to take an operand - // whose type is OpTypeSampledImage. - std::vector consumers = module_.getSampledImageConsumers(resultID); - if (!consumers.empty()) { - for (auto consumer_id : consumers) { - auto consumer_instr = module_.FindDef(consumer_id); - auto consumer_opcode = consumer_instr->opcode(); - if (consumer_instr->block() != sampledImageInstr->block()) { - DIAG(sampledImageInstr) - << "All OpSampledImage instructions must be in the same block in " - "which their Result are consumed. OpSampledImage Result " - "Type '" - << module_.getIdName(resultID) - << "' has a consumer in a different basic " - "block. The consumer instruction is '" - << module_.getIdName(consumer_id) << "'."; - return false; - } - // TODO: The following check is incomplete. We should also check that the - // Sampled Image is not used by instructions that should not take - // SampledImage as an argument. We could find the list of valid - // instructions by scanning for "Sampled Image" in the operand description - // field in the grammar file. - if (consumer_opcode == SpvOpPhi || consumer_opcode == SpvOpSelect) { - DIAG(sampledImageInstr) - << "Result from OpSampledImage instruction must not appear as " - "operands of Op" - << spvOpcodeString(static_cast(consumer_opcode)) << "." - << " Found result '" << module_.getIdName(resultID) - << "' as an operand of '" << module_.getIdName(consumer_id) - << "'."; - return false; - } - } - } - return true; -} - -bool idUsage::isValid(const spv_instruction_t* inst) { - spv_opcode_desc opcodeEntry = nullptr; - if (spvOpcodeTableValueLookup(targetEnv, opcodeTable, inst->opcode, - &opcodeEntry)) - return false; -#define CASE(OpCode) \ - case Spv##OpCode: \ - return isValid(inst, opcodeEntry); - switch (inst->opcode) { - CASE(OpSampledImage) - // Other composite opcodes are validated in validate_composites.cpp. - // Arithmetic opcodes are validated in validate_arithmetics.cpp. - // Bitwise opcodes are validated in validate_bitwise.cpp. - // Logical opcodes are validated in validate_logicals.cpp. - // Derivative opcodes are validated in validate_derivatives.cpp. - default: - return true; - } -#undef TODO -#undef CASE -} - -} // namespace spv_result_t UpdateIdUse(ValidationState_t& _, const Instruction* inst) { for (auto& operand : inst->operands()) { @@ -312,18 +186,5 @@ spv_result_t IdPass(ValidationState_t& _, Instruction* inst) { return SPV_SUCCESS; } -spv_result_t spvValidateInstructionIDs(const spv_instruction_t* pInsts, - const uint64_t instCount, - const ValidationState_t& state, - spv_position position) { - idUsage idUsage(state.context(), pInsts, instCount, state.memory_model(), - state.addressing_model(), state, state.entry_points(), - position, state.context()->consumer); - for (uint64_t instIndex = 0; instIndex < instCount; ++instIndex) { - if (!idUsage.isValid(&pInsts[instIndex])) return SPV_ERROR_INVALID_ID; - } - return SPV_SUCCESS; -} - } // namespace val } // namespace spvtools diff --git a/source/val/validate_image.cpp b/source/val/validate_image.cpp index 0d2dd00c0..2c020ed1b 100644 --- a/source/val/validate_image.cpp +++ b/source/val/validate_image.cpp @@ -737,6 +737,47 @@ spv_result_t ValidateSampledImage(ValidationState_t& _, return _.diag(SPV_ERROR_INVALID_DATA, inst) << "Expected Sampler to be of type OpTypeSampler"; } + + // We need to validate 2 things: + // * All OpSampledImage instructions must be in the same block in which their + // Result are consumed. + // * Result from OpSampledImage instructions must not appear as operands + // to OpPhi instructions or OpSelect instructions, or any instructions other + // than the image lookup and query instructions specified to take an operand + // whose type is OpTypeSampledImage. + std::vector consumers = _.getSampledImageConsumers(inst->id()); + if (!consumers.empty()) { + for (auto consumer_id : consumers) { + const auto consumer_instr = _.FindDef(consumer_id); + const auto consumer_opcode = consumer_instr->opcode(); + if (consumer_instr->block() != inst->block()) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "All OpSampledImage instructions must be in the same block " + "in " + "which their Result are consumed. OpSampledImage Result " + "Type '" + << _.getIdName(inst->id()) + << "' has a consumer in a different basic " + "block. The consumer instruction is '" + << _.getIdName(consumer_id) << "'."; + } + // TODO: The following check is incomplete. We should also check that the + // Sampled Image is not used by instructions that should not take + // SampledImage as an argument. We could find the list of valid + // instructions by scanning for "Sampled Image" in the operand description + // field in the grammar file. + if (consumer_opcode == SpvOpPhi || consumer_opcode == SpvOpSelect) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "Result from OpSampledImage instruction must not appear " + "as " + "operands of Op" + << spvOpcodeString(static_cast(consumer_opcode)) << "." + << " Found result '" << _.getIdName(inst->id()) + << "' as an operand of '" << _.getIdName(consumer_id) + << "'."; + } + } + } return SPV_SUCCESS; }