From a6ef1ea29eecfb8a7ecdcc5a7070595440d6f188 Mon Sep 17 00:00:00 2001 From: Umar Arshad Date: Thu, 22 Sep 2016 10:23:08 -0400 Subject: [PATCH] Fix forward reference issues with forword pointer * Allows OpTypeForwardPointer to reference IDs not yet declared in the module * Allows OpTypeStruct to reference IDs not yet declared in the module Possible Issue: OpTypeStruct should only allow forward references if the ID is a pointer that is referenced by a forward pointer. Need Type support in Validator which is currently a work in progress. --- CHANGES | 3 +++ source/validate_id.cpp | 5 ++++- test/val/Validate.SSA.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index aa744814a..dbfb69cf2 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,9 @@ Revision history for SPIRV-Tools v2016.6-dev 2016-09-16 - Published the C++ interface for assembling, disassembling, validation, and optimization. + - Fixes issues: + #429: Validator: Allow OpTypeForwardPointer and OpTypeStruct to reference + undefined IDs v2016.5 2016-09-16 - Support SPV_KHR_shader_ballot in assembler, disassembler, parser. diff --git a/source/validate_id.cpp b/source/validate_id.cpp index 611d39fd4..bd7f2067b 100644 --- a/source/validate_id.cpp +++ b/source/validate_id.cpp @@ -2347,6 +2347,7 @@ function getCanBeForwardDeclaredFunction(SpvOp opcode) { case SpvOpSelectionMerge: case SpvOpDecorate: case SpvOpMemberDecorate: + case SpvOpTypeStruct: case SpvOpBranch: case SpvOpLoopMerge: out = [](unsigned) { return true; }; @@ -2383,7 +2384,9 @@ function getCanBeForwardDeclaredFunction(SpvOp opcode) { // The Invoke parameter. out = [](unsigned index) { return index == 2; }; break; - + case SpvOpTypeForwardPointer: + out = [](unsigned index) { return index == 0; }; + break; default: out = [](unsigned) { return false; }; break; diff --git a/test/val/Validate.SSA.cpp b/test/val/Validate.SSA.cpp index d37b6461e..06f56911b 100644 --- a/test/val/Validate.SSA.cpp +++ b/test/val/Validate.SSA.cpp @@ -1338,5 +1338,43 @@ TEST_F(ValidateSSA, UseFunctionParameterFromOtherFunctionBad) { MatchesRegex("ID .\\[first\\] used in function .\\[func2\\] is used " "outside of it's defining function .\\[func\\]")); } + +TEST_F(ValidateSSA, TypeForwardPointerForwardReference) { + // See https://github.com/KhronosGroup/SPIRV-Tools/issues/429 + // + // ForwardPointers can references instructions that have not been defined + string str = R"( + OpCapability Kernel + OpCapability Addresses + OpMemoryModel Logical OpenCL + OpName %intptrt "intptrt" + OpTypeForwardPointer %intptrt UniformConstant + %uint = OpTypeInt 32 0 + %intptrt = OpTypePointer UniformConstant %uint +)"; + + CompileSuccessfully(str); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateSSA, TypeStructForwardReference) { + string str = R"( + OpCapability Kernel + OpCapability Addresses + OpMemoryModel Logical OpenCL + OpName %structptr "structptr" + OpTypeForwardPointer %structptr UniformConstant + %uint = OpTypeInt 32 0 + %structt1 = OpTypeStruct %structptr %uint + %structt2 = OpTypeStruct %uint %structptr + %structt3 = OpTypeStruct %uint %uint %structptr + %structt4 = OpTypeStruct %uint %uint %uint %structptr + %structptr = OpTypePointer UniformConstant %structt1 +)"; + + CompileSuccessfully(str); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + // TODO(umar): OpGroupMemberDecorate } // namespace