From: Ehsan Nasiri Date: Wed, 1 Feb 2017 20:37:39 +0000 (-0500) Subject: Validator support for Variable Pointer extension. X-Git-Tag: upstream/2018.6~913 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=23af06c3a308a4e388412809788add9a8510fe80;p=platform%2Fupstream%2FSPIRV-Tools.git Validator support for Variable Pointer extension. If the variable_pointer extension is used: * OpLoad's pointer argument may be the result of any of the following: * OpSelect * OpPhi * OpFunctionCall * OpPtrAccessChain * OpCopyObject * OpLoad * OpConstantNull * Return value of a function may be a pointer. * It is valid to use a pointer as the return value of a function. * OpStore should allow a variable pointer argument. --- diff --git a/source/opcode.cpp b/source/opcode.cpp index 9f51b95..e3922f4 100644 --- a/source/opcode.cpp +++ b/source/opcode.cpp @@ -230,6 +230,26 @@ int32_t spvOpcodeIsComposite(const SpvOp opcode) { } } +bool spvOpcodeReturnsLogicalVariablePointer(const SpvOp opcode) { + switch (opcode) { + case SpvOpVariable: + case SpvOpAccessChain: + case SpvOpInBoundsAccessChain: + case SpvOpFunctionParameter: + case SpvOpImageTexelPointer: + case SpvOpCopyObject: + case SpvOpSelect: + case SpvOpPhi: + case SpvOpFunctionCall: + case SpvOpPtrAccessChain: + case SpvOpLoad: + case SpvOpConstantNull: + return true; + default: + return false; + } +} + int32_t spvOpcodeReturnsLogicalPointer(const SpvOp opcode) { switch (opcode) { case SpvOpVariable: diff --git a/source/opcode.h b/source/opcode.h index 9742cbc..a54a96d 100644 --- a/source/opcode.h +++ b/source/opcode.h @@ -79,6 +79,10 @@ int32_t spvOpcodeIsComposite(const SpvOp opcode); // addressing model. Returns zero if false, non-zero otherwise. int32_t spvOpcodeReturnsLogicalPointer(const SpvOp opcode); +// Returns whether the given opcode could result in a pointer or a variable +// pointer when using the logical addressing model. +bool spvOpcodeReturnsLogicalVariablePointer(const SpvOp opcode); + // Determines if the given opcode generates a type. Returns zero if false, // non-zero otherwise. int32_t spvOpcodeGeneratesType(SpvOp opcode); diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp index 12e864d..86e2d98 100644 --- a/source/val/validation_state.cpp +++ b/source/val/validation_state.cpp @@ -300,6 +300,14 @@ void ValidationState_t::RegisterCapability(SpvCapability cap) { features_.declare_int16_type = true; features_.declare_float16_type = true; features_.free_fp_rounding_mode = true; + break; + case SpvCapabilityVariablePointers: + features_.variable_pointers = true; + features_.variable_pointers_uniform_buffer_block = true; + break; + case SpvCapabilityVariablePointersUniformBufferBlock: + features_.variable_pointers_uniform_buffer_block = true; + break; default: break; } diff --git a/source/val/validation_state.h b/source/val/validation_state.h index a61fbab..d130c0f 100644 --- a/source/val/validation_state.h +++ b/source/val/validation_state.h @@ -62,6 +62,12 @@ class ValidationState_t { bool free_fp_rounding_mode = false; // Allow the FPRoundingMode decoration // and its vaules to be used without // requiring any capability + + // Allow functionalities enabled by VariablePointers capability. + bool variable_pointers = false; + // Allow functionalities enabled by VariablePointersUniformBufferBlock + // capability. + bool variable_pointers_uniform_buffer_block = false; }; ValidationState_t(const spv_const_context context, diff --git a/source/validate_id.cpp b/source/validate_id.cpp index 039c9eb..f150e56 100644 --- a/source/validate_id.cpp +++ b/source/validate_id.cpp @@ -1116,10 +1116,17 @@ bool idUsage::isValid(const spv_instruction_t* inst, << inst->words[resultTypeIndex] << "' is not defind."; return false; } + const bool uses_variable_pointer = + module_.features().variable_pointers || + module_.features().variable_pointers_uniform_buffer_block; auto pointerIndex = 3; auto pointer = module_.FindDef(inst->words[pointerIndex]); - if (!pointer || (addressingModel == SpvAddressingModelLogical && - !spvOpcodeReturnsLogicalPointer(pointer->opcode()))) { + if (!pointer || + (addressingModel == SpvAddressingModelLogical && + ((!uses_variable_pointer && + !spvOpcodeReturnsLogicalPointer(pointer->opcode())) || + (uses_variable_pointer && + !spvOpcodeReturnsLogicalVariablePointer(pointer->opcode()))))) { DIAG(pointerIndex) << "OpLoad Pointer '" << inst->words[pointerIndex] << "' is not a logical pointer."; return false; @@ -1145,10 +1152,17 @@ bool idUsage::isValid(const spv_instruction_t* inst, template <> bool idUsage::isValid(const spv_instruction_t* inst, const spv_opcode_desc) { - auto pointerIndex = 1; + const bool uses_variable_pointer = + module_.features().variable_pointers || + module_.features().variable_pointers_uniform_buffer_block; + const auto pointerIndex = 1; auto pointer = module_.FindDef(inst->words[pointerIndex]); - if (!pointer || (addressingModel == SpvAddressingModelLogical && - !spvOpcodeReturnsLogicalPointer(pointer->opcode()))) { + if (!pointer || + (addressingModel == SpvAddressingModelLogical && + ((!uses_variable_pointer && + !spvOpcodeReturnsLogicalPointer(pointer->opcode())) || + (uses_variable_pointer && + !spvOpcodeReturnsLogicalVariablePointer(pointer->opcode()))))) { DIAG(pointerIndex) << "OpStore Pointer '" << inst->words[pointerIndex] << "' is not a logical pointer."; return false; @@ -2404,8 +2418,11 @@ bool idUsage::isValid(const spv_instruction_t* inst, << "' is missing or void."; return false; } + const bool uses_variable_pointer = + module_.features().variable_pointers || + module_.features().variable_pointers_uniform_buffer_block; if (addressingModel == SpvAddressingModelLogical && - SpvOpTypePointer == valueType->opcode()) { + SpvOpTypePointer == valueType->opcode() && !uses_variable_pointer) { DIAG(valueIndex) << "OpReturnValue value's type '" << value->type_id() << "' is a pointer, which is invalid in the Logical addressing model."; diff --git a/source/validate_instruction.cpp b/source/validate_instruction.cpp index 49d2113..aafb7b0 100644 --- a/source/validate_instruction.cpp +++ b/source/validate_instruction.cpp @@ -395,9 +395,10 @@ spv_result_t InstructionPass(ValidationState_t& _, const SpvOp opcode = static_cast(inst->opcode); if (opcode == SpvOpExtension) CheckIfKnownExtension(_, inst); - if (opcode == SpvOpCapability) + if (opcode == SpvOpCapability) { _.RegisterCapability( static_cast(inst->words[inst->operands[0].offset])); + } if (opcode == SpvOpMemoryModel) { _.set_addressing_model( static_cast(inst->words[inst->operands[0].offset])); diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp index 754420a..ba8cf9f 100644 --- a/test/val/val_id_test.cpp +++ b/test/val/val_id_test.cpp @@ -1805,6 +1805,163 @@ TEST_F(ValidateIdWithMessage, OpLoadGood) { CompileSuccessfully(spirv.c_str()); EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); } + +// TODO: Add tests that exercise VariablePointersUniformBufferBlock instead of +// VariablePointers. +void createVariablePointerSpirvProgram(std::ostringstream* spirv, + std::string result_strategy, + bool use_varptr_cap, + bool add_helper_function) { + *spirv << "OpCapability Shader "; + if (use_varptr_cap) { + *spirv << "OpCapability VariablePointers "; + *spirv << "OpExtension \"SPV_KHR_variable_pointers\" "; + } + *spirv << R"( + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %main "main" + %void = OpTypeVoid + %voidf = OpTypeFunction %void + %bool = OpTypeBool + %i32 = OpTypeInt 32 1 + %f32 = OpTypeFloat 32 + %f32ptr = OpTypePointer Uniform %f32 + %i = OpConstant %i32 1 + %zero = OpConstant %i32 0 + %float_1 = OpConstant %f32 1.0 + %ptr1 = OpVariable %f32ptr Uniform + %ptr2 = OpVariable %f32ptr Uniform + )"; + if (add_helper_function) { + *spirv << R"( + ; //////////////////////////////////////////////////////////// + ;;;; Function that returns a pointer + ; //////////////////////////////////////////////////////////// + %selector_func_type = OpTypeFunction %f32ptr %bool %f32ptr %f32ptr + %choose_input_func = OpFunction %f32ptr None %selector_func_type + %is_neg_param = OpFunctionParameter %bool + %first_ptr_param = OpFunctionParameter %f32ptr + %second_ptr_param = OpFunctionParameter %f32ptr + %selector_func_begin = OpLabel + %result_ptr = OpSelect %f32ptr %is_neg_param %first_ptr_param %second_ptr_param + OpReturnValue %result_ptr + OpFunctionEnd + )"; + } + *spirv << R"( + %main = OpFunction %void None %voidf + %label = OpLabel + )"; + *spirv << result_strategy; + *spirv << R"( + OpReturn + OpFunctionEnd + )"; +} + +// With the VariablePointer Capability, OpLoad should allow loading a +// VaiablePointer. In this test the variable pointer is obtained by an OpSelect +TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpSelectGood) { + std::string result_strategy = R"( + %isneg = OpSLessThan %bool %i %zero + %varptr = OpSelect %f32ptr %isneg %ptr1 %ptr2 + %result = OpLoad %f32 %varptr + )"; + + std::ostringstream spirv; + createVariablePointerSpirvProgram(&spirv, result_strategy, + true /* Add VariablePointers Capability? */, + false /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +// Without the VariablePointers Capability, OpLoad will not allow loading +// through a variable pointer. +TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpSelectBad) { + std::string result_strategy = R"( + %isneg = OpSLessThan %bool %i %zero + %varptr = OpSelect %f32ptr %isneg %ptr1 %ptr2 + %result = OpLoad %f32 %varptr + )"; + + std::ostringstream spirv; + createVariablePointerSpirvProgram(&spirv, result_strategy, + false /* Add VariablePointers Capability?*/, + false /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr("is not a logical pointer.")); +} + +// With the VariablePointer Capability, OpLoad should allow loading a +// VaiablePointer. In this test the variable pointer is obtained by an OpPhi +TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpPhiGood) { + std::string result_strategy = R"( + %is_neg = OpSLessThan %bool %i %zero + OpSelectionMerge %end_label None + OpBranchConditional %is_neg %take_ptr_1 %take_ptr_2 + %take_ptr_1 = OpLabel + OpBranch %end_label + %take_ptr_2 = OpLabel + OpBranch %end_label + %end_label = OpLabel + %varptr = OpPhi %f32ptr %ptr1 %take_ptr_1 %ptr2 %take_ptr_2 + %result = OpLoad %f32 %varptr + )"; + + std::ostringstream spirv; + createVariablePointerSpirvProgram(&spirv, result_strategy, + true /* Add VariablePointers Capability?*/, + false /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +// Without the VariablePointers Capability, OpLoad will not allow loading +// through a variable pointer. +TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpPhiBad) { + std::string result_strategy = R"( + %is_neg = OpSLessThan %bool %i %zero + OpSelectionMerge %end_label None + OpBranchConditional %is_neg %take_ptr_1 %take_ptr_2 + %take_ptr_1 = OpLabel + OpBranch %end_label + %take_ptr_2 = OpLabel + OpBranch %end_label + %end_label = OpLabel + %varptr = OpPhi %f32ptr %ptr1 %take_ptr_1 %ptr2 %take_ptr_2 + %result = OpLoad %f32 %varptr + )"; + + std::ostringstream spirv; + createVariablePointerSpirvProgram(&spirv, result_strategy, + false /* Add VariablePointers Capability?*/, + false /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr("is not a logical pointer")); +} + +// With the VariablePointer Capability, OpLoad should allow loading through a +// VaiablePointer. In this test the variable pointer is obtained from an +// OpFunctionCall (return value from a function) +TEST_F(ValidateIdWithMessage, OpLoadVarPtrOpFunctionCallGood) { + std::ostringstream spirv; + std::string result_strategy = R"( + %isneg = OpSLessThan %bool %i %zero + %varptr = OpFunctionCall %f32ptr %choose_input_func %isneg %ptr1 %ptr2 + %result = OpLoad %f32 %varptr + )"; + + createVariablePointerSpirvProgram(&spirv, + result_strategy, + true /* Add VariablePointers Capability?*/, + true /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + TEST_F(ValidateIdWithMessage, OpLoadResultTypeBad) { string spirv = kGLSL450MemoryModel + R"( %1 = OpTypeVoid @@ -1927,6 +2084,41 @@ TEST_F(ValidateIdWithMessage, OpStoreLogicalPointerBad) { HasSubstr("OpStore Pointer '10' is not a logical pointer.")); } +// Without the VariablePointer Capability, OpStore should may not store +// through a variable pointer. +TEST_F(ValidateIdWithMessage, OpStoreVarPtrBad) { + std::string result_strategy = R"( + %isneg = OpSLessThan %bool %i %zero + %varptr = OpSelect %f32ptr %isneg %ptr1 %ptr2 + OpStore %varptr %float_1 + )"; + + std::ostringstream spirv; + createVariablePointerSpirvProgram( + &spirv, result_strategy, false /* Add VariablePointers Capability? */, + false /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), HasSubstr("is not a logical pointer.")); +} + +// With the VariablePointer Capability, OpStore should allow storing through a +// variable pointer. +TEST_F(ValidateIdWithMessage, OpStoreVarPtrGood) { + std::string result_strategy = R"( + %isneg = OpSLessThan %bool %i %zero + %varptr = OpSelect %f32ptr %isneg %ptr1 %ptr2 + OpStore %varptr %float_1 + )"; + + std::ostringstream spirv; + createVariablePointerSpirvProgram(&spirv, result_strategy, + true /* Add VariablePointers Capability? */, + false /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + TEST_F(ValidateIdWithMessage, OpStoreObjectGood) { string spirv = kGLSL450MemoryModel + R"( %1 = OpTypeVoid @@ -3614,6 +3806,33 @@ TEST_F(ValidateIdWithMessage, OpReturnValueIsVariableInLogical) { "which is invalid in the Logical addressing model.")); } +// With the VariablePointer Capability, the return value of a function is +// allowed to be a pointer. +TEST_F(ValidateIdWithMessage, OpReturnValueVarPtrGood) { + std::ostringstream spirv; + createVariablePointerSpirvProgram(&spirv, + "" /* Instructions to add to "main" */, + true /* Add VariablePointers Capability?*/, + true /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +// Without the VariablePointer Capability, the return value of a function is +// *not* allowed to be a pointer. +TEST_F(ValidateIdWithMessage, OpReturnValueVarPtrBad) { + std::ostringstream spirv; + createVariablePointerSpirvProgram(&spirv, + "" /* Instructions to add to "main" */, + false /* Add VariablePointers Capability?*/, + true /* Use Helper Function? */); + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("OpReturnValue value's type '7' is a pointer, " + "which is invalid in the Logical addressing model.")); +} + // TODO: enable when this bug is fixed: // https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15404 TEST_F(ValidateIdWithMessage, DISABLED_OpReturnValueIsFunction) {