diff --git a/source/val/validate_adjacency.cpp b/source/val/validate_adjacency.cpp index ae2eff3f8..47bfe46e4 100644 --- a/source/val/validate_adjacency.cpp +++ b/source/val/validate_adjacency.cpp @@ -28,28 +28,34 @@ namespace spvtools { namespace val { enum { + // Status right after meeting OpFunction. IN_NEW_FUNCTION, + // Status right after meeting the entry block. IN_ENTRY_BLOCK, + // Status right after meeting non-entry blocks. PHI_VALID, - PHI_INVALID, + // Status right after meeting non-OpVariable instructions in the entry block + // or non-OpPhi instructions in non-entry blocks, except OpLine. + PHI_AND_VAR_INVALID, }; spv_result_t ValidateAdjacency(ValidationState_t& _) { const auto& instructions = _.ordered_instructions(); - int phi_check = PHI_INVALID; + int adjacency_status = PHI_AND_VAR_INVALID; for (size_t i = 0; i < instructions.size(); ++i) { const auto& inst = instructions[i]; switch (inst.opcode()) { case SpvOpFunction: case SpvOpFunctionParameter: - phi_check = IN_NEW_FUNCTION; + adjacency_status = IN_NEW_FUNCTION; break; case SpvOpLabel: - phi_check = phi_check == IN_NEW_FUNCTION ? IN_ENTRY_BLOCK : PHI_VALID; + adjacency_status = + adjacency_status == IN_NEW_FUNCTION ? IN_ENTRY_BLOCK : PHI_VALID; break; case SpvOpPhi: - if (phi_check != PHI_VALID) { + if (adjacency_status != PHI_VALID) { return _.diag(SPV_ERROR_INVALID_DATA, &inst) << "OpPhi must appear within a non-entry block before all " << "non-OpPhi instructions " @@ -59,7 +65,7 @@ spv_result_t ValidateAdjacency(ValidationState_t& _) { case SpvOpLine: break; case SpvOpLoopMerge: - phi_check = PHI_INVALID; + adjacency_status = PHI_AND_VAR_INVALID; if (i != (instructions.size() - 1)) { switch (instructions[i + 1].opcode()) { case SpvOpBranch: @@ -75,7 +81,7 @@ spv_result_t ValidateAdjacency(ValidationState_t& _) { } break; case SpvOpSelectionMerge: - phi_check = PHI_INVALID; + adjacency_status = PHI_AND_VAR_INVALID; if (i != (instructions.size() - 1)) { switch (instructions[i + 1].opcode()) { case SpvOpBranchConditional: @@ -90,8 +96,16 @@ spv_result_t ValidateAdjacency(ValidationState_t& _) { } } break; + case SpvOpVariable: + if (inst.GetOperandAs(2) == SpvStorageClassFunction && + adjacency_status != IN_ENTRY_BLOCK) { + return _.diag(SPV_ERROR_INVALID_DATA, &inst) + << "All OpVariable instructions in a function must be the " + "first instructions in the first block."; + } + break; default: - phi_check = PHI_INVALID; + adjacency_status = PHI_AND_VAR_INVALID; break; } } diff --git a/test/opt/local_ssa_elim_test.cpp b/test/opt/local_ssa_elim_test.cpp index 92a95f14b..cd371575e 100644 --- a/test/opt/local_ssa_elim_test.cpp +++ b/test/opt/local_ssa_elim_test.cpp @@ -1662,6 +1662,7 @@ TEST_F(LocalSSAElimTest, CompositeExtractProblem) { %_ptr_Function__struct_11 = OpTypePointer Function %_struct_11 %2 = OpFunction %void None %4 %33 = OpLabel + %66 = OpVariable %_ptr_Function__arr__struct_11_uint_3 Function %34 = OpLoad %_arr_v4float_uint_3 %16 %35 = OpLoad %_arr_v4float_uint_3 %17 %36 = OpLoad %_arr_v4float_uint_3 %18 @@ -1694,7 +1695,6 @@ TEST_F(LocalSSAElimTest, CompositeExtractProblem) { %63 = OpCompositeExtract %v2float %40 2 %64 = OpCompositeConstruct %_struct_11 %57 %58 %59 %60 %61 %62 %63 %65 = OpCompositeConstruct %_arr__struct_11_uint_3 %48 %56 %64 - %66 = OpVariable %_ptr_Function__arr__struct_11_uint_3 Function %67 = OpLoad %uint %20 ; CHECK OpStore {{%\d+}} [[store_source:%\d+]] diff --git a/test/val/val_adjacency_test.cpp b/test/val/val_adjacency_test.cpp index 5d3121383..10002ef34 100644 --- a/test/val/val_adjacency_test.cpp +++ b/test/val/val_adjacency_test.cpp @@ -120,6 +120,7 @@ std::string GenerateShaderCode( %zero = OpConstant %int 0 %int_1 = OpConstant %int 1 %func = OpTypeFunction %void +%func_int = OpTypePointer Function %int %main = OpFunction %void None %func %main_entry = OpLabel )"; @@ -313,6 +314,68 @@ OpNop "non-OpPhi instructions")); } +TEST_F(ValidateAdjacency, OpVariableInFunctionGood) { + const std::string body = R"( +OpLine %string 1 1 +%var = OpVariable %func_int Function +OpLine %string 2 1 +OpNop +OpLine %string 3 1 +OpNop +)"; + + CompileSuccessfully(GenerateShaderCode(body)); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateAdjacency, OpVariableInFunctionMultipleGood) { + const std::string body = R"( +OpLine %string 1 1 +%1 = OpVariable %func_int Function +OpLine %string 2 1 +%2 = OpVariable %func_int Function +%3 = OpVariable %func_int Function +OpNop +OpLine %string 3 1 +OpNop +)"; + + CompileSuccessfully(GenerateShaderCode(body)); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateAdjacency, OpVariableInFunctionBad) { + const std::string body = R"( +%1 = OpUndef %int +%2 = OpVariable %func_int Function +)"; + + CompileSuccessfully(GenerateShaderCode(body)); + EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("All OpVariable instructions in a function must be the " + "first instructions")); +} + +TEST_F(ValidateAdjacency, OpVariableInFunctionMultipleBad) { + const std::string body = R"( +OpNop +%1 = OpVariable %func_int Function +OpLine %string 1 1 +%2 = OpVariable %func_int Function +OpNop +OpNop +OpLine %string 2 1 +%3 = OpVariable %func_int Function +)"; + + CompileSuccessfully(GenerateShaderCode(body)); + EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("All OpVariable instructions in a function must be the " + "first instructions")); +} + TEST_F(ValidateAdjacency, OpLoopMergePreceedsOpBranchSuccess) { const std::string body = R"( OpBranch %loop