diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp index 077363e40..06f9d989a 100644 --- a/source/val/validation_state.cpp +++ b/source/val/validation_state.cpp @@ -62,6 +62,10 @@ bool IsInstructionInLayoutSection(ModuleLayoutSection layout, SpvOp op) { default: break; } break; + case kLayoutDebug3: + // Only OpModuleProcessed is allowed here. + out = (op == SpvOpModuleProcessed); + break; case kLayoutAnnotations: switch (op) { case SpvOpDecorate: @@ -110,6 +114,7 @@ bool IsInstructionInLayoutSection(ModuleLayoutSection layout, SpvOp op) { case SpvOpString: case SpvOpName: case SpvOpMemberName: + case SpvOpModuleProcessed: case SpvOpDecorate: case SpvOpMemberDecorate: case SpvOpGroupDecorate: diff --git a/source/val/validation_state.h b/source/val/validation_state.h index 1fd1735a9..b33406563 100644 --- a/source/val/validation_state.h +++ b/source/val/validation_state.h @@ -46,6 +46,7 @@ enum ModuleLayoutSection { kLayoutExecutionMode, /// < Section 2.4 #6 kLayoutDebug1, /// < Section 2.4 #7 > 1 kLayoutDebug2, /// < Section 2.4 #7 > 2 + kLayoutDebug3, /// < Section 2.4 #7 > 3 kLayoutAnnotations, /// < Section 2.4 #8 kLayoutTypes, /// < Section 2.4 #9 kLayoutFunctionDeclarations, /// < Section 2.4 #10 diff --git a/source/validate_layout.cpp b/source/validate_layout.cpp index b8b551802..4718b912f 100644 --- a/source/validate_layout.cpp +++ b/source/validate_layout.cpp @@ -50,7 +50,7 @@ spv_result_t ModuleScopedInstructions(ValidationState_t& _, } break; case kLayoutFunctionDeclarations: - // All module sections have been processed. Recursivly call + // All module sections have been processed. Recursively call // ModuleLayoutPass to process the next section of the module return libspirv::ModuleLayoutPass(_, inst); default: @@ -190,6 +190,7 @@ spv_result_t ModuleLayoutPass(ValidationState_t& _, case kLayoutExecutionMode: case kLayoutDebug1: case kLayoutDebug2: + case kLayoutDebug3: case kLayoutAnnotations: case kLayoutTypes: if (auto error = ModuleScopedInstructions(_, inst, opcode)) return error; diff --git a/test/val/val_layout_test.cpp b/test/val/val_layout_test.cpp index 3a9900845..c6750e164 100644 --- a/test/val/val_layout_test.cpp +++ b/test/val/val_layout_test.cpp @@ -34,8 +34,9 @@ using std::tie; using std::tuple; using std::vector; -using ::testing::StrEq; +using ::testing::Eq; using ::testing::HasSubstr; +using ::testing::StrEq; using libspirv::spvResultToString; using pred_type = function; @@ -491,5 +492,114 @@ TEST_F(ValidateEntryPoint, NoEntryPointWithLinkageCapGood) { EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); } +TEST_F(ValidateLayout, ModuleProcessedInvalidIn10) { + char str[] = R"( + OpCapability Shader + OpCapability Linkage + OpMemoryModel Logical GLSL450 + OpName %void "void" + OpModuleProcessed "this is ok in 1.1 and later" + OpDecorate %void Volatile ; bogus, but makes the example short +%void = OpTypeVoid +)"; + + CompileSuccessfully(str, SPV_ENV_UNIVERSAL_1_1); + ASSERT_EQ(SPV_ERROR_INVALID_BINARY, ValidateInstructions(SPV_ENV_UNIVERSAL_1_0)); + // In a 1.0 environment the binary parse fails before we even get to + // validation. This occurs no matter where the OpModuleProcessed is placed. + EXPECT_THAT(getDiagnosticString(), HasSubstr("Invalid opcode: 330")); +} + +TEST_F(ValidateLayout, ModuleProcessedValidIn11) { + char str[] = R"( + OpCapability Shader + OpCapability Linkage + OpMemoryModel Logical GLSL450 + OpName %void "void" + OpModuleProcessed "this is ok in 1.1 and later" + OpDecorate %void Volatile ; bogus, but makes the example short +%void = OpTypeVoid +)"; + + CompileSuccessfully(str, SPV_ENV_UNIVERSAL_1_1); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1)); + EXPECT_THAT(getDiagnosticString(), Eq("")); +} + +TEST_F(ValidateLayout, ModuleProcessedBeforeLastNameIsTooEarly) { + char str[] = R"( + OpCapability Shader + OpCapability Linkage + OpMemoryModel Logical GLSL450 + OpModuleProcessed "this is too early" + OpName %void "void" +%void = OpTypeVoid +)"; + + CompileSuccessfully(str, SPV_ENV_UNIVERSAL_1_1); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1)); + // By the mechanics of the validator, we assume ModuleProcessed is in the + // right spot, but then that OpName is in the wrong spot. + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Name cannot appear in a function declaration")); +} + +TEST_F(ValidateLayout, ModuleProcessedInvalidAfterFirstAnnotation) { + char str[] = R"( + OpCapability Shader + OpCapability Linkage + OpMemoryModel Logical GLSL450 + OpDecorate %void Volatile ; this is bogus, but keeps the example short + OpModuleProcessed "this is too late" +%void = OpTypeVoid +)"; + + CompileSuccessfully(str, SPV_ENV_UNIVERSAL_1_1); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("ModuleProcessed cannot appear in a function declaration")); +} + +TEST_F(ValidateLayout, ModuleProcessedInvalidInFunctionBeforeLabel) { + char str[] = R"( + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %main "main" +%void = OpTypeVoid +%voidfn = OpTypeFunction %void +%main = OpFunction %void None %voidfn + OpModuleProcessed "this is too late, in function before label" +%entry = OpLabel + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(str, SPV_ENV_UNIVERSAL_1_1); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("ModuleProcessed cannot appear in a function declaration")); +} + +TEST_F(ValidateLayout, ModuleProcessedInvalidInBasicBlock) { + char str[] = R"( + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %main "main" +%void = OpTypeVoid +%voidfn = OpTypeFunction %void +%main = OpFunction %void None %voidfn +%entry = OpLabel + OpModuleProcessed "this is too late, in basic block" + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(str, SPV_ENV_UNIVERSAL_1_1); + ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("ModuleProcessed cannot appear in a function declaration")); +} + + // TODO(umar): Test optional instructions }