From 753cdc75c464bf372623534dac8f384e8628bc72 Mon Sep 17 00:00:00 2001 From: David Neto Date: Tue, 7 Apr 2020 19:56:02 +0000 Subject: [PATCH] [spirv-reader] Test EmitFunctionDeclaration Tests FunctionEmitter directly. This is mostly refactoring to be able to selectively run parts of the parsing flow, and to access relevant internal data. Bug: tint:3 Change-Id: Ic2b166a2e9623a7e30e6769806088d12e78dcf45 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/18704 Reviewed-by: dan sinclair --- src/CMakeLists.txt | 1 + src/reader/spirv/function_decl_test.cc | 170 +++++++++++++++++++++ src/reader/spirv/parser_impl.cc | 16 +- src/reader/spirv/parser_impl.h | 13 ++ src/reader/spirv/parser_impl_test_helper.h | 9 ++ 5 files changed, 206 insertions(+), 3 deletions(-) create mode 100644 src/reader/spirv/function_decl_test.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f8c57cde01..456661e7c3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -328,6 +328,7 @@ if(${TINT_BUILD_SPV_READER}) list(APPEND TINT_TEST_SRCS reader/spirv/enum_converter_test.cc reader/spirv/fail_stream_test.cc + reader/spirv/function_decl_test.cc reader/spirv/namer_test.cc reader/spirv/parser_impl_convert_member_decoration_test.cc reader/spirv/parser_impl_convert_type_test.cc diff --git a/src/reader/spirv/function_decl_test.cc b/src/reader/spirv/function_decl_test.cc new file mode 100644 index 0000000000..2e2c35c6dc --- /dev/null +++ b/src/reader/spirv/function_decl_test.cc @@ -0,0 +1,170 @@ +// Copyright 2020 The Tint Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/reader/spirv/function.h" + +#include +#include + +#include "gmock/gmock.h" +#include "src/reader/spirv/parser_impl.h" +#include "src/reader/spirv/parser_impl_test_helper.h" +#include "src/reader/spirv/spirv_tools_helpers_test.h" + +namespace tint { +namespace reader { +namespace spirv { +namespace { + +using ::testing::HasSubstr; + +/// @returns a SPIR-V assembly segment which assigns debug names +/// to particular IDs. +std::string Names(std::vector ids) { + std::ostringstream outs; + for (auto& id : ids) { + outs << " OpName %" << id << " \"" << id << "\"\n"; + } + return outs.str(); +} + +std::string CommonTypes() { + return R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %float = OpTypeFloat 32 + %uint = OpTypeInt 32 0 + %int = OpTypeInt 32 1 + %float_0 = OpConstant %float 0.0 + )"; +} + +TEST_F(SpvParserTest, EmitFunctionDeclaration_VoidFunctionWithoutParams) { + auto p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + %entry = OpLabel + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitFunctionDeclaration()); + EXPECT_THAT(p->module().to_str(), HasSubstr(R"( + Function x_100 -> __void + () + { + })")); +} + +TEST_F(SpvParserTest, EmitFunctionDeclaration_NonVoidResultType) { + auto p = parser(test::Assemble(CommonTypes() + R"( + %fn_ret_float = OpTypeFunction %float + %100 = OpFunction %float None %fn_ret_float + %entry = OpLabel + OpReturnValue %float_0 + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitFunctionDeclaration()); + + EXPECT_THAT(p->module().to_str(), HasSubstr(R"( + Function x_100 -> __f32 + () + { + })")); +} + +TEST_F(SpvParserTest, EmitFunctionDeclaration_MixedParamTypes) { + auto p = parser(test::Assemble(Names({"a", "b", "c"}) + CommonTypes() + R"( + %fn_mixed_params = OpTypeFunction %float %uint %float %int + + %100 = OpFunction %void None %fn_mixed_params + %a = OpFunctionParameter %uint + %b = OpFunctionParameter %float + %c = OpFunctionParameter %int + %mixed_entry = OpLabel + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitFunctionDeclaration()); + + EXPECT_THAT(p->module().to_str(), HasSubstr(R"( + Function x_100 -> __void + ( + Variable{ + a + none + __u32 + } + Variable{ + b + none + __f32 + } + Variable{ + c + none + __i32 + } + ) + { + })")); +} + +TEST_F(SpvParserTest, EmitFunctionDeclaration_GenerateParamNames) { + auto p = parser(test::Assemble(CommonTypes() + R"( + %fn_mixed_params = OpTypeFunction %float %uint %float %int + + %100 = OpFunction %void None %fn_mixed_params + %14 = OpFunctionParameter %uint + %15 = OpFunctionParameter %float + %16 = OpFunctionParameter %int + %mixed_entry = OpLabel + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitFunctionDeclaration()); + + EXPECT_THAT(p->module().to_str(), HasSubstr(R"( + Function x_100 -> __void + ( + Variable{ + x_14 + none + __u32 + } + Variable{ + x_15 + none + __f32 + } + Variable{ + x_16 + none + __i32 + } + ) + { + })")); +} + +} // namespace +} // namespace spirv +} // namespace reader +} // namespace tint diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index ca89a05cc2..fc00865391 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -333,6 +333,19 @@ void ParserImpl::ResetInternalModule() { } bool ParserImpl::ParseInternalModule() { + if (!success_) { + return false; + } + if (!ParseInternalModuleExceptFunctions()) { + return false; + } + if (!EmitFunctions()) { + return false; + } + return success_; +} + +bool ParserImpl::ParseInternalModuleExceptFunctions() { if (!success_) { return false; } @@ -354,9 +367,6 @@ bool ParserImpl::ParseInternalModule() { if (!EmitModuleScopeVariables()) { return false; } - if (!EmitFunctions()) { - return false; - } return success_; } diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h index 46d9c7f2e6..dc42e75592 100644 --- a/src/reader/spirv/parser_impl.h +++ b/src/reader/spirv/parser_impl.h @@ -90,6 +90,13 @@ class ParserImpl : Reader { bool BuildAndParseInternalModule() { return BuildInternalModule() && ParseInternalModule(); } + /// Builds an internal representation of the SPIR-V binary, + /// and parses the module, except functions, into a Tint AST module. + /// Diagnostics are emitted to the error stream. + /// @returns true if it was successful. + bool BuildAndParseInternalModuleExceptFunctions() { + return BuildInternalModule() && ParseInternalModuleExceptFunctions(); + } /// @returns the set of SPIR-V IDs for imports of the "GLSL.std.450" /// extended instruction set. @@ -149,6 +156,12 @@ class ParserImpl : Reader { /// @returns true if the parser is still successful. bool ParseInternalModule(); + /// Walks the internal representation of the module, except for function + /// definitions, to populate the AST form of the module. + /// This is a no-op if the parser has already failed. + /// @returns true if the parser is still successful. + bool ParseInternalModuleExceptFunctions(); + /// Destroys the internal representation of the SPIR-V module. void ResetInternalModule(); diff --git a/src/reader/spirv/parser_impl_test_helper.h b/src/reader/spirv/parser_impl_test_helper.h index 66a48dfdc4..4f52e0985f 100644 --- a/src/reader/spirv/parser_impl_test_helper.h +++ b/src/reader/spirv/parser_impl_test_helper.h @@ -20,6 +20,7 @@ #include #include "gtest/gtest.h" +#include "source/opt/ir_context.h" #include "src/context.h" #include "src/reader/spirv/parser_impl.h" @@ -47,6 +48,14 @@ class SpvParserTest : public testing::Test { return impl_.get(); } + /// Gets the internal representation of the function with the given ID. + /// Assumes ParserImpl::BuildInternalRepresentation has been run and + /// succeeded. + /// @returns the internal representation of the function + spvtools::opt::Function* spirv_function(uint32_t id) { + return impl_->ir_context()->GetFunction(id); + } + private: std::unique_ptr impl_; Context ctx_;