diff --git a/source/diagnostic.cpp b/source/diagnostic.cpp index 78a2267a9..60502c027 100644 --- a/source/diagnostic.cpp +++ b/source/diagnostic.cpp @@ -88,5 +88,68 @@ DiagnosticStream::~DiagnosticStream() { *pDiagnostic_ = spvDiagnosticCreate(&position_, stream_.str().c_str()); } } +std::string +spvResultToString(spv_result_t res) { + std::string out; + switch (res) { + case SPV_SUCCESS: + out = "SPV_SUCCESS"; + break; + case SPV_UNSUPPORTED: + out = "SPV_UNSUPPORTED"; + break; + case SPV_END_OF_STREAM: + out = "SPV_END_OF_STREAM"; + break; + case SPV_WARNING: + out = "SPV_WARNING"; + break; + case SPV_FAILED_MATCH: + out = "SPV_FAILED_MATCH"; + break; + case SPV_REQUESTED_TERMINATION: + out = "SPV_REQUESTED_TERMINATION"; + break; + case SPV_ERROR_INTERNAL: + out = "SPV_ERROR_INTERNAL"; + break; + case SPV_ERROR_OUT_OF_MEMORY: + out = "SPV_ERROR_OUT_OF_MEMORY"; + break; + case SPV_ERROR_INVALID_POINTER: + out = "SPV_ERROR_INVALID_POINTER"; + break; + case SPV_ERROR_INVALID_BINARY: + out = "SPV_ERROR_INVALID_BINARY"; + break; + case SPV_ERROR_INVALID_TEXT: + out = "SPV_ERROR_INVALID_TEXT"; + break; + case SPV_ERROR_INVALID_TABLE: + out = "SPV_ERROR_INVALID_TABLE"; + break; + case SPV_ERROR_INVALID_VALUE: + out = "SPV_ERROR_INVALID_VALUE"; + break; + case SPV_ERROR_INVALID_DIAGNOSTIC: + out = "SPV_ERROR_INVALID_DIAGNOSTIC"; + break; + case SPV_ERROR_INVALID_LOOKUP: + out = "SPV_ERROR_INVALID_LOOKUP"; + break; + case SPV_ERROR_INVALID_ID: + out = "SPV_ERROR_INVALID_ID"; + break; + case SPV_ERROR_INVALID_CFG: + out = "SPV_ERROR_INVALID_CFG"; + break; + case SPV_ERROR_INVALID_LAYOUT: + out = "SPV_ERROR_INVALID_LAYOUT"; + break; + default: + out = "Unknown Error"; + } + return out; +} } // namespace libspirv diff --git a/source/diagnostic.h b/source/diagnostic.h index 9ee153296..c80b77d43 100644 --- a/source/diagnostic.h +++ b/source/diagnostic.h @@ -102,6 +102,8 @@ class DiagnosticStream { libspirv::diagnostic_helper helper(position, pDiagnostic); \ helper.stream() +std::string spvResultToString(spv_result_t res); + } // namespace libspirv #endif // LIBSPIRV_DIAGNOSTIC_H_ diff --git a/source/validate.cpp b/source/validate.cpp index a208e470b..0447f0dc1 100644 --- a/source/validate.cpp +++ b/source/validate.cpp @@ -58,6 +58,10 @@ using std::unordered_set; using std::vector; using libspirv::ValidationState_t; +using libspirv::kLayoutFunctionDeclarations; +using libspirv::kLayoutFunctionDefinitions; +using libspirv::kLayoutMemoryModel; +using libspirv::FunctionDecl; #define spvCheckReturn(expression) \ if (spv_result_t error = (expression)) return error; @@ -436,10 +440,7 @@ void DebugInstructionPass(ValidationState_t& _, } } -// TODO(umar): Check MemoryModel is in module -// TODO(umar): Check OpVariable storage class is not function in module section -// TODO(umar): Make sure function declarations appear before function -// definitions +// TODO(umar): Check linkage capabilities for function declarations // TODO(umar): Better error messages // NOTE: This function does not handle CFG related validation // Performs logical layout validation. See Section 2.4 @@ -448,22 +449,38 @@ spv_result_t ModuleLayoutPass(ValidationState_t& _, if (_.is_enabled(SPV_VALIDATE_LAYOUT_BIT)) { SpvOp opcode = inst->opcode; - if (libspirv::ModuleLayoutSection::kModule == _.getLayoutStage()) { + if (_.getLayoutStage() < kLayoutFunctionDeclarations) { // Module scoped instructions are processed by determining if the opcode // is part of the current stage. If it is not then the next stage is // checked. while (_.isOpcodeInCurrentLayoutStage(opcode) == false) { - // TODO(umar): Check if the MemoryModel instruction has executed _.progressToNextLayoutStageOrder(); - if (_.getLayoutStage() == libspirv::ModuleLayoutSection::kFunction) { + + if (_.getLayoutStage() == kLayoutMemoryModel && + opcode != SpvOpMemoryModel) { + return _.diag(SPV_ERROR_INVALID_LAYOUT) + << spvOpcodeString(opcode) + << " cannot appear before the memory model instruction"; + } + + if (_.getLayoutStage() == kLayoutFunctionDeclarations) { // All module stages have been processed. Recursivly call - // ModuleLayoutPass - // to process the next section of the module + // ModuleLayoutPass to process the next section of the module return ModuleLayoutPass(_, inst); } } + + if (opcode == SpvOpVariable) { + const uint32_t* storage_class = inst->words + inst->operands[2].offset; + if (*storage_class == SpvStorageClassFunction) { + return _.diag(SPV_ERROR_INVALID_LAYOUT) + << "Variables cannot have a function[7] storage class " + "outside of a function"; + } + } } else { - // Validate the function layout. + // This ensures no module instructions are called during function + // declarations switch (opcode) { case SpvOpCapability: case SpvOpExtension: @@ -520,24 +537,117 @@ spv_result_t ModuleLayoutPass(ValidationState_t& _, inst->words + inst->operands[2].offset; if (*storage_class != SpvStorageClassFunction) return _.diag(SPV_ERROR_INVALID_LAYOUT) - << "All OpVariable instructions in a function must have a " - "storage class of Function[7]"; - break; - } + << "All Variable instructions in a function must have a " + "storage class of function[7]"; + } break; default: - return SPV_SUCCESS; + break; } + if (_.getLayoutStage() == kLayoutFunctionDeclarations) { + switch (opcode) { + case SpvOpFunction: + if (_.in_function_body()) { + return _.diag(SPV_ERROR_INVALID_LAYOUT) + << "Cannot declare a function in a function body"; + } + spvCheckReturn(_.get_functions().RegisterFunction( + inst->result_id, inst->type_id, + inst->words[inst->operands[2].offset], + inst->words[inst->operands[3].offset])); + break; + case SpvOpFunctionParameter: + if (_.in_function_body() == false) { + return _.diag(SPV_ERROR_INVALID_LAYOUT) << "Function parameter " + "instructions must be " + "in a function body"; + } + spvCheckReturn(_.get_functions().RegisterFunctionParameter( + inst->result_id, inst->type_id)); + break; + case SpvOpLine: // ?? + break; + case SpvOpLabel: + if (_.in_function_body() == false) { + return _.diag(SPV_ERROR_INVALID_LAYOUT) + << "Label instructions must be in a function body"; + } + _.progressToNextLayoutStageOrder(); + spvCheckReturn(_.get_functions().RegisterSetFunctionDeclType( + FunctionDecl::kFunctionDeclDefinition)); + break; + case SpvOpFunctionEnd: + assert(_.get_functions().get_block_count() == + 0 // NOTE: This should not happen + && + "Function contains blocks in function declaration section."); + if (_.in_function_body() == false) { + return _.diag(SPV_ERROR_INVALID_LAYOUT) + << "Function end instructions must be in a function body"; + } + spvCheckReturn(_.get_functions().RegisterSetFunctionDeclType( + FunctionDecl::kFunctionDeclDeclaration)); + spvCheckReturn(_.get_functions().RegisterFunctionEnd()); + break; + default: + return _.diag(SPV_ERROR_INVALID_LAYOUT) + << "A function must begin with a label"; + break; + } + } + // NOTE: Function definitions are handled by the CFGPass } } return SPV_SUCCESS; } -// Shame -#define CHECK_RESULT(EXPRESSION) \ - do{ \ - spv_result_t ret = EXPRESSION; \ - if(ret) return ret; \ -} while(false); +// TODO(umar): Support for merge instructions +// TODO(umar): Structured control flow checks +spv_result_t CfgPass(ValidationState_t& _, + const spv_parsed_instruction_t* inst) { + if (_.getLayoutStage() == kLayoutFunctionDefinitions) { + SpvOp opcode = inst->opcode; + switch (opcode) { + case SpvOpFunction: + spvCheckReturn(_.get_functions().RegisterFunction( + inst->result_id, inst->type_id, + inst->words[inst->operands[2].offset], + inst->words[inst->operands[3].offset])); + spvCheckReturn(_.get_functions().RegisterSetFunctionDeclType( + FunctionDecl::kFunctionDeclDefinition)); + break; + case SpvOpFunctionParameter: + spvCheckReturn(_.get_functions().RegisterFunctionParameter( + inst->result_id, inst->type_id)); + break; + case SpvOpFunctionEnd: + if (_.get_functions().get_block_count() == 0) + return _.diag(SPV_ERROR_INVALID_LAYOUT) << "Function declarations " + "must appear before " + "function definitions."; + spvCheckReturn(_.get_functions().RegisterFunctionEnd()); + break; + case SpvOpLabel: + spvCheckReturn(_.get_functions().RegisterBlock(inst->result_id)); + break; + case SpvOpBranch: + case SpvOpBranchConditional: + case SpvOpSwitch: + case SpvOpKill: + case SpvOpReturn: + case SpvOpReturnValue: + case SpvOpUnreachable: + spvCheckReturn(_.get_functions().RegisterBlockEnd()); + break; + default: + if (_.in_block() == false) { + return _.diag(SPV_ERROR_INVALID_LAYOUT) << spvOpcodeString(opcode) + << " must appear in a block"; + } + break; + } + } + return SPV_SUCCESS; +} spv_result_t ProcessInstructions(void* user_data, const spv_parsed_instruction_t* inst) { @@ -549,16 +659,16 @@ spv_result_t ProcessInstructions(void* user_data, DebugInstructionPass(_, inst); - // TODO(umar): Perform CFG pass // TODO(umar): Perform data rules pass // TODO(umar): Perform instruction validation pass - CHECK_RESULT(ModuleLayoutPass(_, inst)) - CHECK_RESULT(SsaPass(_, can_have_forward_declared_ids, inst)) + spvCheckReturn(ModuleLayoutPass(_, inst)); + spvCheckReturn(CfgPass(_, inst)); + spvCheckReturn(SsaPass(_, can_have_forward_declared_ids, inst)); return SPV_SUCCESS; } -} // anonymous namespace +} // anonymous namespace spv_result_t spvValidate(const spv_const_context context, const spv_const_binary binary, const uint32_t options, @@ -581,12 +691,9 @@ spv_result_t spvValidate(const spv_const_context context, // NOTE: Parse the module and perform inline validation checks. These // checks do not require the the knowledge of the whole module. ValidationState_t vstate(pDiagnostic, options); - auto err = spvBinaryParse(context, &vstate, binary->code, binary->wordCount, - setHeader, ProcessInstructions, pDiagnostic); - - if (err) { - return err; - } + spvCheckReturn(spvBinaryParse(context, &vstate, binary->code, + binary->wordCount, setHeader, + ProcessInstructions, pDiagnostic)); // TODO(umar): Add validation checks which require the parsing of the entire // module. Use the information from the processInstructions pass to make diff --git a/source/validate_types.cpp b/source/validate_types.cpp index 51655fe2d..1f130c5e9 100644 --- a/source/validate_types.cpp +++ b/source/validate_types.cpp @@ -28,6 +28,7 @@ #include "validate_types.h" #include +#include #include #include #include @@ -115,16 +116,15 @@ const vector>& GetModuleOrder() { namespace libspirv { -ValidationState_t::ValidationState_t(spv_diagnostic* diagnostic, - uint32_t options) - : diagnostic_(diagnostic), +ValidationState_t::ValidationState_t(spv_diagnostic* diag, uint32_t options) + : diagnostic_(diag), instruction_counter_(0), defined_ids_{}, unresolved_forward_ids_{}, validation_flags_(options), operand_names_{}, - module_layout_order_stage_(0), - current_layout_stage_(ModuleLayoutSection::kModule) {} + current_layout_stage_(kLayoutCapabilities), + module_functions_(*this) {} spv_result_t ValidationState_t::defineId(uint32_t id) { if (defined_ids_.find(id) == end(defined_ids_)) { @@ -186,23 +186,134 @@ ModuleLayoutSection ValidationState_t::getLayoutStage() const { } void ValidationState_t::progressToNextLayoutStageOrder() { - module_layout_order_stage_ += - module_layout_order_stage_ < GetModuleOrder().size(); - if (module_layout_order_stage_ >= GetModuleOrder().size()) { - current_layout_stage_ = libspirv::ModuleLayoutSection::kFunction; + if (current_layout_stage_ <= GetModuleOrder().size()) { + current_layout_stage_ = + static_cast(current_layout_stage_ + 1); } } bool ValidationState_t::isOpcodeInCurrentLayoutStage(SpvOp op) { - const vector& currentStage = - GetModuleOrder()[module_layout_order_stage_]; + const vector& currentStage = GetModuleOrder()[current_layout_stage_]; return end(currentStage) != find(begin(currentStage), end(currentStage), op); } -libspirv::DiagnosticStream ValidationState_t::diag( - spv_result_t error_code) const { +DiagnosticStream ValidationState_t::diag(spv_result_t error_code) const { return libspirv::DiagnosticStream( {0, 0, static_cast(instruction_counter_)}, diagnostic_, error_code); } + +Functions& ValidationState_t::get_functions() { return module_functions_; } + +bool ValidationState_t::in_function_body() const { + return module_functions_.in_function_body(); +} + +bool ValidationState_t::in_block() const { + return module_functions_.in_block(); +} + +Functions::Functions(ValidationState_t& module) + : module_(module), in_function_(false), in_block_(false) {} + +bool Functions::in_function_body() const { return in_function_; } + +bool Functions::in_block() const { return in_block_; } + +spv_result_t Functions::RegisterFunction(uint32_t id, uint32_t ret_type_id, + uint32_t function_control, + uint32_t function_type_id) { + assert(in_function_ == false && + "Function instructions can not be declared in a function"); + in_function_ = true; + id_.emplace_back(id); + type_id_.emplace_back(function_type_id); + declaration_type_.emplace_back(FunctionDecl::kFunctionDeclUnknown); + block_ids_.emplace_back(); + variable_ids_.emplace_back(); + parameter_ids_.emplace_back(); + + // TODO(umar): validate function type and type_id + (void)ret_type_id; + (void)function_control; + + return SPV_SUCCESS; +} + +spv_result_t Functions::RegisterFunctionParameter(uint32_t id, + uint32_t type_id) { + assert(in_function_ == true && + "Function parameter instructions cannot be declared outside of a function"); + if (in_block()) { + return module_.diag(SPV_ERROR_INVALID_LAYOUT) + << "Function parameters cannot be called in blocks"; + } + if (block_ids_.back().size() != 0) { + return module_.diag(SPV_ERROR_INVALID_LAYOUT) + << "Function parameters must only appear immediatly after the " + "function definition"; + } + // TODO(umar): Validate function parameter type order and count + // TODO(umar): Use these variables to validate parameter type + (void)id; + (void)type_id; + return SPV_SUCCESS; +} + +spv_result_t Functions::RegisterSetFunctionDeclType(FunctionDecl type) { + assert(in_function_ == true && "Function can not be declared inside of another function"); + if (declaration_type_.size() <= 1 || type == *(end(declaration_type_) - 2) || + type == FunctionDecl::kFunctionDeclDeclaration) { + declaration_type_.back() = type; + } else if (type == FunctionDecl::kFunctionDeclDeclaration) { + return module_.diag(SPV_ERROR_INVALID_LAYOUT) + << "Function declartions must appear before function definitions"; + } else { + declaration_type_.back() = type; + } + return SPV_SUCCESS; +} + +spv_result_t Functions::RegisterBlock(uint32_t id) { + assert(in_function_ == true && "Labels can only exsist in functions"); + if (module_.getLayoutStage() == + ModuleLayoutSection::kLayoutFunctionDeclarations) { + return module_.diag(SPV_ERROR_INVALID_LAYOUT) + << "Function declartions must appear before function definitions"; + } + if (declaration_type_.back() != FunctionDecl::kFunctionDeclDefinition) { + // NOTE: This should not happen. We should know that this function is a + // definition at this point. + return module_.diag(SPV_ERROR_INTERNAL) + << "Function declaration type should have already been defined"; + } + + block_ids_.back().push_back(id); + in_block_ = true; + return SPV_SUCCESS; +} + +spv_result_t Functions::RegisterFunctionEnd() { + assert(in_function_ == true && + "Function end can only be called in functions"); + if (in_block()) { + return module_.diag(SPV_ERROR_INVALID_LAYOUT) + << "Function end cannot be called in blocks"; + } + in_function_ = false; + return SPV_SUCCESS; +} + +spv_result_t Functions::RegisterBlockEnd() { + assert(in_block_ == true && + "Branch instruction can only be called in a block"); + in_block_ = false; + return SPV_SUCCESS; +} + +size_t Functions::get_block_count() { + assert(in_function_ == true && + "Branch instruction can only be called in a block"); + return block_ids_.back().size(); +} } diff --git a/source/validate_types.h b/source/validate_types.h index e57c58e65..d11badd32 100644 --- a/source/validate_types.h +++ b/source/validate_types.h @@ -38,12 +38,116 @@ namespace libspirv { -// This enum represents the sections of a SPIRV module. The MODULE section -// contains instructions who's scope spans the entire module. The FUNCTION -// section includes SPIRV function and function definitions -enum class ModuleLayoutSection { - kModule, // < Module scope instructions are executed - kFunction, // < Function scope instructions are executed +// This enum represents the sections of a SPIRV module. See section 2.4 +// of the SPIRV spec for additional details of the order. The enumerant values +// are in the same order as the vector returned by GetModuleOrder +enum ModuleLayoutSection { + kLayoutCapabilities, // < Section 2.4 #1 + kLayoutExtensions, // < Section 2.4 #2 + kLayoutExtInstImport, // < Section 2.4 #3 + kLayoutMemoryModel, // < Section 2.4 #4 + kLayoutEntryPoint, // < Section 2.4 #5 + kLayoutExecutionMode, // < Section 2.4 #6 + kLayoutDebug1, // < Section 2.4 #7 > 1 + kLayoutDebug2, // < Section 2.4 #7 > 2 + kLayoutAnnotations, // < Section 2.4 #8 + kLayoutTypes, // < Section 2.4 #9 + kLayoutFunctionDeclarations, // < Section 2.4 #10 + kLayoutFunctionDefinitions // < Section 2.4 #11 +}; + +enum class FunctionDecl { + kFunctionDeclUnknown, // < Unknown function declaration + kFunctionDeclDeclaration, // < Function declaration + kFunctionDeclDefinition // < Function definition +}; + +class ValidationState_t; + +// This class manages all function declaration and definitions in a module. It +// handles the state and id information while parsing a function in the SPIR-V +// binary. +// +// NOTE: This class is designed to be a Structure of Arrays. Therefore each +// member variable is a vector whose elements represent the values for the +// corresponding function in a SPIR-V module. Variables that are not vector +// types are used to manage the state while parsing the function. +class Functions { + public: + Functions(ValidationState_t& module); + + // 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, + uint32_t function_control, + uint32_t function_type_id); + + // Registers a function parameter in the current function + spv_result_t RegisterFunctionParameter(uint32_t id, uint32_t type_id); + + // Register a function end instruction + spv_result_t RegisterFunctionEnd(); + + // Sets the declaration type of the current function + spv_result_t RegisterSetFunctionDeclType(FunctionDecl type); + + // Registers a block in the current function. Subsequent block instructions + // will target this block + // @param id The ID of the label of the block + spv_result_t RegisterBlock(uint32_t id); + + // Registers a variable in the current block + spv_result_t RegisterBlockVariable(uint32_t type_id, uint32_t id, + SpvStorageClass storage, uint32_t init_id); + + spv_result_t RegisterBlockLoopMerge(uint32_t merge_id, uint32_t continue_id, + SpvLoopControlMask control); + + spv_result_t RegisterBlockSelectionMerge(uint32_t merge_id, + SpvSelectionControlMask control); + + // Registers the end of the block + spv_result_t RegisterBlockEnd(); + + // Returns the number of blocks in the current function being parsed + size_t get_block_count(); + + // Retuns true if called after a function instruction but before the + // function end instruction + bool in_function_body() const; + + // Returns true if called after a label instruction but before a branch + // instruction + bool in_block() const; + + libspirv::DiagnosticStream diag(spv_result_t error_code) const; + + private: + // Parent module + ValidationState_t& module_; + + // Funciton IDs in a module + std::vector id_; + + // OpTypeFunction IDs of each of the id_ functions + std::vector type_id_; + + // The type of declaration of each function + std::vector declaration_type_; + + // TODO(umar): Probably needs better abstractions + // The beginning of the block of functions + std::vector> block_ids_; + + // The variable IDs of the functions + std::vector> variable_ids_; + + // The function parameter ids of the functions + std::vector> parameter_ids_; + + // NOTE: See correspoding getter functions + bool in_function_; + bool in_block_; }; class ValidationState_t { @@ -94,6 +198,17 @@ class ValidationState_t { libspirv::DiagnosticStream diag(spv_result_t error_code) const; + // Returns the function states + Functions& get_functions(); + + // Retuns true if the called after a function instruction but before the + // function end instruction + bool in_function_body() const; + + // Returns true if called after a label instruction but before a branch + // instruction + bool in_block() const; + private: spv_diagnostic* diagnostic_; // Tracks the number of instructions evaluated by the validator @@ -110,12 +225,12 @@ class ValidationState_t { std::map operand_names_; - // The stage which is being processed by the validation. Partially based on - // Section 2.4. Logical Layout of a Module - uint32_t module_layout_order_stage_; - // The section of the code being processed ModuleLayoutSection current_layout_stage_; + + Functions module_functions_; + + std::vector module_capabilities_; }; } diff --git a/test/Validate.Layout.cpp b/test/Validate.Layout.cpp index e902dd30e..39f05d1c4 100644 --- a/test/Validate.Layout.cpp +++ b/test/Validate.Layout.cpp @@ -27,6 +27,7 @@ // Validation tests for Logical Layout #include "gmock/gmock.h" +#include "source/diagnostic.h" #include "UnitSPIRV.h" #include "ValidateFixtures.h" @@ -36,6 +37,7 @@ #include using std::function; +using std::ostream; using std::ostream_iterator; using std::pair; using std::stringstream; @@ -46,30 +48,37 @@ using std::vector; using ::testing::HasSubstr; -using pred_type = function; +using libspirv::spvResultToString; + +using pred_type = function; using ValidateLayout = spvtest::ValidateBase>, SPV_VALIDATE_LAYOUT_BIT>; - namespace { // returns true if order is equal to VAL -template -bool Equals(int order) { - return order == VAL; +template +spv_result_t Equals(int order) { + return order == VAL ? SPV_SUCCESS : RET; } // returns true if order is between MIN and MAX(inclusive) -template +template struct Range { - bool operator()(int order) { return order >= MIN && order <= MAX; } + Range(bool inverse = false) : inverse_(inverse) {} + spv_result_t operator()(int order) { + return (inverse_ ^ (order >= MIN && order <= MAX)) ? SPV_SUCCESS : RET; + } + + private: + bool inverse_; }; template -bool RangeSet(int order) { - for (bool val : {T()(order)...}) - if (!val) return val; - return false; +spv_result_t InvalidSet(int order) { + for (spv_result_t val : {T(true)(order)...}) + if (val != SPV_SUCCESS) return val; + return SPV_SUCCESS; } // SPIRV source used to test the logical layout @@ -92,41 +101,42 @@ const vector& getInstructions() { "OpMemberDecorate %struct 1 RowMajor", "%dgrp = OpDecorationGroup", "OpGroupDecorate %dgrp %mat33 %mat44", - "%intt = OpTypeInt 32 1", - "%floatt = OpTypeFloat 32", - "%voidt = OpTypeVoid", - "%boolt = OpTypeBool", - "%vec4 = OpTypeVector %intt 4", - "%vec3 = OpTypeVector %intt 3", - "%mat33 = OpTypeMatrix %vec3 3", - "%mat44 = OpTypeMatrix %vec4 4", - "%struct = OpTypeStruct %intt %mat33", - "%vfunct = OpTypeFunction %voidt", - "%viifunct = OpTypeFunction %voidt %intt %intt", - "%one = OpConstant %intt 1", + "%intt = OpTypeInt 32 1", + "%floatt = OpTypeFloat 32", + "%voidt = OpTypeVoid", + "%boolt = OpTypeBool", + "%vec4 = OpTypeVector %intt 4", + "%vec3 = OpTypeVector %intt 3", + "%mat33 = OpTypeMatrix %vec3 3", + "%mat44 = OpTypeMatrix %vec4 4", + "%struct = OpTypeStruct %intt %mat33", + "%vfunct = OpTypeFunction %voidt", + "%viifunct = OpTypeFunction %voidt %intt %intt", + "%one = OpConstant %intt 1", // TODO(umar): OpConstant fails because the type is not defined // TODO(umar): OpGroupMemberDecorate "OpLine %str 3 4", - "%func = OpFunction %voidt None %vfunct", + "%func = OpFunction %voidt None %vfunct", "OpFunctionEnd", - "%func2 = OpFunction %voidt None %viifunct", - "%funcp1 = OpFunctionParameter %intt", - "%funcp2 = OpFunctionParameter %intt", - "%fLabel = OpLabel", - " OpNop", - "OpReturn", + "%func2 = OpFunction %voidt None %viifunct", + "%funcp1 = OpFunctionParameter %intt", + "%funcp2 = OpFunctionParameter %intt", + "%fLabel = OpLabel", + " OpNop", + " OpReturn", "OpFunctionEnd" }; return instructions; } -pred_type All = Range<0, 1000>(); +static const int kRangeEnd = 1000; +pred_type All = Range<0, kRangeEnd>(); INSTANTIATE_TEST_CASE_P(InstructionsOrder, ValidateLayout, ::testing::Combine(::testing::Range((int)0, (int)getInstructions().size()), // | Instruction | Line(s) valid | Lines to compile - ::testing::Values( make_tuple( string("OpCapability") , Equals<0> , All) + ::testing::Values( make_tuple(string("OpCapability") , Equals<0> , All) , make_tuple(string("OpExtension") , Equals<1> , All) , make_tuple(string("OpExtInstImport") , Equals<2> , All) , make_tuple(string("OpMemoryModel") , Equals<3> , All) @@ -149,9 +159,11 @@ INSTANTIATE_TEST_CASE_P(InstructionsOrder, , make_tuple(string("OpTypeVector %intt 4") , Range<16, 28>() , All) , make_tuple(string("OpTypeMatrix %vec4 4") , Range<16, 28>() , All) , make_tuple(string("OpTypeStruct") , Range<16, 28>() , All) - , make_tuple(string("%vfunct = OpTypeFunction"), Range<16, 28>() , All) - , make_tuple(string("OpConstant") , Range<19, 28>() , static_cast(Range<19, 100>())) - //, make_tuple(string("OpLabel") , RangeSet, Range<35, 36>, > , All) + , make_tuple(string("%vfunct = OpTypeFunction"), Range<16, 28>() , All) + , make_tuple(string("OpConstant") , Range<19, 28>() , static_cast(Range<19, kRangeEnd>())) + , make_tuple(string("OpLabel") , Equals<34> , All) + , make_tuple(string("OpNop") , Equals<35> , All) + , make_tuple(string("OpReturn") , Equals<36> , All) ))); // clang-format on @@ -188,7 +200,7 @@ TEST_P(ValidateLayout, Layout) { tie(instruction, pred, test_pred) = testCase; // Skip test which break the code generation - if (!test_pred(order)) return; + if (test_pred(order)) return; vector code = GenerateCode(instruction, order); @@ -197,16 +209,18 @@ TEST_P(ValidateLayout, Layout) { // printf("code: \n%s\n", ss.str().c_str()); CompileSuccessfully(ss.str()); - if (pred(order)) { - ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()) - << "Order: " << order << "\nInstruction: " << instruction; - } else { - ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions()) - << "Order: " << order << "\nInstruction: " << instruction; - } + spv_result_t result; + // clang-format off + ASSERT_EQ(pred(order), result = ValidateInstructions()) + << "Actual: " << spvResultToString(result) + << "\nExpected: " << spvResultToString(pred(order)) + << "\nOrder: " << order + << "\nInstruction: " << instruction + << "\nCode: \n" << ss.str(); + // clang-format on } -TEST_F(ValidateLayout, DISABLED_MemoryModelMissing) { +TEST_F(ValidateLayout, MemoryModelMissing) { string str = R"( OpCapability Matrix OpExtension "TestExtension" @@ -219,6 +233,114 @@ TEST_F(ValidateLayout, DISABLED_MemoryModelMissing) { ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions()); } -// TODO(umar): Test optional instructions -// TODO(umar): Test logical layout of functions +TEST_F(ValidateLayout, VariableFunctionStorageGood) { + char str[] = R"( + OpMemoryModel Logical GLSL450 + OpDecorate %var Restrict +%intt = OpTypeInt 32 1 +%voidt = OpTypeVoid +%vfunct = OpTypeFunction %voidt +%ptrt = OpTypePointer Function %intt +%func = OpFunction %voidt None %vfunct +%funcl = OpLabel +%var = OpVariable %ptrt Function + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(str); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateLayout, VariableFunctionStorageBad) { + char str[] = R"( + OpMemoryModel Logical GLSL450 + OpDecorate %var Restrict +%intt = OpTypeInt 32 1 +%voidt = OpTypeVoid +%vfunct = OpTypeFunction %voidt +%ptrt = OpTypePointer Function %intt +%var = OpVariable %ptrt Function ; Invalid storage class for OpVariable +%func = OpFunction %voidt None %vfunct +%funcl = OpLabel + OpNop + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(str); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions()); +} + +TEST_F(ValidateLayout, FunctionDefinitionBeforeDeclarationBad) { + char str[] = R"( + OpMemoryModel Logical GLSL450 + OpDecorate %var Restrict +%intt = OpTypeInt 32 1 +%voidt = OpTypeVoid +%vfunct = OpTypeFunction %voidt +%vifunct = OpTypeFunction %voidt %intt +%ptrt = OpTypePointer Function %intt +%func = OpFunction %voidt None %vfunct +%funcl = OpLabel + OpNop + OpReturn + OpFunctionEnd +%func2 = OpFunction %voidt None %vifunct ; must appear before definition +%func2p = OpFunctionParameter %intt + OpFunctionEnd +)"; + + CompileSuccessfully(str); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions()); +} + +// TODO(umar): Passes but gives incorrect error message. Should be fixed after +// type checking +TEST_F(ValidateLayout, LabelBeforeFunctionParameterBad) { + char str[] = R"( + OpMemoryModel Logical GLSL450 + OpDecorate %var Restrict +%intt = OpTypeInt 32 1 +%voidt = OpTypeVoid +%vfunct = OpTypeFunction %voidt +%vifunct = OpTypeFunction %voidt %intt +%ptrt = OpTypePointer Function %intt +%func = OpFunction %voidt None %vifunct +%funcl = OpLabel ; Label appears before function parameter +%func2p = OpFunctionParameter %intt + OpNop + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(str); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions()); +} + +TEST_F(ValidateLayout, FuncParameterNotImmediatlyAfterFuncBad) { + char str[] = R"( + OpMemoryModel Logical GLSL450 + OpDecorate %var Restrict +%intt = OpTypeInt 32 1 +%voidt = OpTypeVoid +%vfunct = OpTypeFunction %voidt +%vifunct = OpTypeFunction %voidt %intt +%ptrt = OpTypePointer Function %intt +%func = OpFunction %voidt None %vifunct +%funcl = OpLabel + OpNop + OpBranch %next +%func2p = OpFunctionParameter %intt ;FunctionParameter appears in a function but not immediatly afterwards +%next = OpLabel + OpNop + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(str); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions()); +} + +// TODO(umar): Test optional instructions } diff --git a/test/Validate.SSA.cpp b/test/Validate.SSA.cpp index 5ca35df23..1eaf148ee 100644 --- a/test/Validate.SSA.cpp +++ b/test/Validate.SSA.cpp @@ -332,6 +332,7 @@ TEST_F(Validate, FunctionCallGood) { %5 = OpFunction %1 None %4 %6 = OpLabel %7 = OpFunctionCall %1 %9 %four %five + OpReturn OpFunctionEnd )"; CompileSuccessfully(str); @@ -351,6 +352,7 @@ TEST_F(Validate, ForwardFunctionCallGood) { %5 = OpFunction %1 None %4 %6 = OpLabel %7 = OpFunctionCall %1 %9 %four %five + OpReturn OpFunctionEnd %9 = OpFunction %1 None %8 %10 = OpFunctionParameter %2 @@ -770,6 +772,7 @@ TEST_P(Validate, ForwardGetKernelGood) { string str = kHeader + kBasicTypes + kKernelTypesAndConstants + R"( %main = OpFunction %voidt None %vfunct + %mainl = OpLabel )" + kKernelSetup + " %numsg = " + instruction + " %uintt" + ndrange_param + "%kfunc %firstp %psize %palign" diff --git a/test/ValidateFixtures.cpp b/test/ValidateFixtures.cpp index 3d38083a9..45735a21d 100644 --- a/test/ValidateFixtures.cpp +++ b/test/ValidateFixtures.cpp @@ -61,9 +61,10 @@ void ValidateBase::TearDown() { template void ValidateBase::CompileSuccessfully(std::string code) { spv_diagnostic diagnostic = nullptr; - EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(context_, code.c_str(), code.size(), + ASSERT_EQ(SPV_SUCCESS, spvTextToBinary(context_, code.c_str(), code.size(), &binary_, &diagnostic)) - << "SPIR-V could not be compiled into binary:" << code; + << "ERROR: " << diagnostic->error + << "\nSPIR-V could not be compiled into binary:\n" << code; } template @@ -77,12 +78,17 @@ std::string ValidateBase::getDiagnosticString() { return std::string(diagnostic_->error); } +template +spv_position_t ValidateBase::getErrorPosition() { + return diagnostic_->position; +} + template class spvtest::ValidateBase, SPV_VALIDATE_SSA_BIT | SPV_VALIDATE_LAYOUT_BIT>; template class spvtest::ValidateBase; template class spvtest::ValidateBase< - std::tuple, - std::function>>, + std::tuple, + std::function>>, SPV_VALIDATE_LAYOUT_BIT>; } diff --git a/test/ValidateFixtures.h b/test/ValidateFixtures.h index 21dc48340..c58b06889 100644 --- a/test/ValidateFixtures.h +++ b/test/ValidateFixtures.h @@ -52,6 +52,7 @@ class ValidateBase : public ::testing::Test, spv_result_t ValidateInstructions(); std::string getDiagnosticString(); + spv_position_t getErrorPosition(); spv_context context_; spv_binary binary_;