From 33d34650e83c5cbcddd294d91520bc82b667e136 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Wed, 25 Mar 2020 19:43:20 +0000 Subject: [PATCH] [spirv-writer] Emit function signatures. This CL updates the SPIR-V writer to emit the OpTypeFunction and OpFunction instructions. Bug: tint:5 Change-Id: I85ead161ca37304a977213257a825ff268d29f2d Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/17741 Reviewed-by: David Neto --- src/CMakeLists.txt | 1 + src/ast/function.cc | 13 ++++ src/ast/function.h | 3 + src/ast/function_test.cc | 23 ++++++ src/reader/spirv/namer_test.cc | 2 +- src/reader/spirv/parser_impl.cc | 2 +- src/writer/spirv/builder.cc | 60 +++++++++++++++ src/writer/spirv/builder.h | 10 ++- src/writer/spirv/builder_entry_point_test.cc | 2 +- src/writer/spirv/builder_function_test.cc | 80 ++++++++++++++++++++ src/writer/spirv/spv_dump.h | 1 + 11 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 src/writer/spirv/builder_function_test.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 70979a2fff..10605fda7e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -408,6 +408,7 @@ if(${TINT_BUILD_SPV_WRITER}) writer/spirv/builder_test.cc writer/spirv/builder_type_test.cc writer/spirv/builder_entry_point_test.cc + writer/spirv/builder_function_test.cc writer/spirv/instruction_test.cc writer/spirv/operand_test.cc writer/spirv/spv_dump.cc diff --git a/src/ast/function.cc b/src/ast/function.cc index 4d13d139cc..4aa7c524c4 100644 --- a/src/ast/function.cc +++ b/src/ast/function.cc @@ -14,6 +14,8 @@ #include "src/ast/function.h" +#include + namespace tint { namespace ast { @@ -83,5 +85,16 @@ void Function::to_str(std::ostream& out, size_t indent) const { out << "}" << std::endl; } +std::string Function::type_name() const { + std::ostringstream out; + + out << "__func" + return_type_->type_name(); + for (const auto& param : params_) { + out << param->type()->type_name(); + } + + return out.str(); +} + } // namespace ast } // namespace tint diff --git a/src/ast/function.h b/src/ast/function.h index 691f059b1e..71afe7c459 100644 --- a/src/ast/function.h +++ b/src/ast/function.h @@ -94,6 +94,9 @@ class Function : public Node { /// @param indent number of spaces to indent the node when writing void to_str(std::ostream& out, size_t indent) const override; + /// @returns the type name for this function + std::string type_name() const; + private: Function(const Function&) = delete; diff --git a/src/ast/function_test.cc b/src/ast/function_test.cc index d2bd9aacf7..e4506a54c5 100644 --- a/src/ast/function_test.cc +++ b/src/ast/function_test.cc @@ -16,6 +16,7 @@ #include "gtest/gtest.h" #include "src/ast/nop_statement.h" +#include "src/ast/type/f32_type.h" #include "src/ast/type/i32_type.h" #include "src/ast/type/void_type.h" #include "src/ast/variable.h" @@ -202,5 +203,27 @@ TEST_F(FunctionTest, ToStr_WithParams) { )"); } +TEST_F(FunctionTest, TypeName) { + type::VoidType void_type; + + Function f("func", {}, &void_type); + EXPECT_EQ(f.type_name(), "__func__void"); +} + +TEST_F(FunctionTest, TypeName_WithParams) { + type::VoidType void_type; + type::I32Type i32; + type::F32Type f32; + + std::vector> params; + params.push_back( + std::make_unique("var1", StorageClass::kNone, &i32)); + params.push_back( + std::make_unique("var2", StorageClass::kNone, &f32)); + + Function f("func", std::move(params), &void_type); + EXPECT_EQ(f.type_name(), "__func__void__i32__f32"); +} + } // namespace ast } // namespace tint diff --git a/src/reader/spirv/namer_test.cc b/src/reader/spirv/namer_test.cc index 75c46c1201..4f94907976 100644 --- a/src/reader/spirv/namer_test.cc +++ b/src/reader/spirv/namer_test.cc @@ -200,7 +200,7 @@ TEST_F(SpvNamerTest, SuggestSanitizedMemberName_TakeSanitizedSuggestion) { TEST_F( SpvNamerTest, - SuggestSanitizedMemberName_TakeSuggestionWhenNoConflictAfterSuggestionForLowerMember) { + SuggestSanitizedMemberName_TakeSuggestionWhenNoConflictAfterSuggestionForLowerMember) { // NOLINT Namer namer(fail_stream_); EXPECT_TRUE(namer.SuggestSanitizedMemberName(1, 7, "mother")); EXPECT_THAT(namer.GetMemberName(1, 2), Eq("")); diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 000cd64e22..97ce5f2fc9 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -238,7 +238,7 @@ void ParserImpl::ResetInternalModule() { bool ParserImpl::ParseInternalModule() { if (!success_) { return false; - }; + } if (!RegisterExtendedInstructionImports()) { return false; } diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 6a9063dfc0..7d511ee368 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -14,6 +14,8 @@ #include "src/writer/spirv/builder.h" +#include + #include "spirv/unified1/spirv.h" #include "src/ast/struct.h" #include "src/ast/struct_member.h" @@ -74,6 +76,12 @@ bool Builder::Build(const ast::Module& m) { {Operand::Int(SpvAddressingModelLogical), Operand::Int(SpvMemoryModelVulkanKHR)}); + for (const auto& func : m.functions()) { + if (!GenerateFunction(func.get())) { + return false; + } + } + for (const auto& ep : m.entry_points()) { if (!GenerateEntryPoint(ep.get())) { return false; @@ -142,6 +150,58 @@ bool Builder::GenerateEntryPoint(ast::EntryPoint* ep) { return true; } +bool Builder::GenerateFunction(ast::Function* func) { + uint32_t func_type_id = GenerateFunctionTypeIfNeeded(func); + if (func_type_id == 0) { + return false; + } + + auto func_op = result_op(); + auto func_id = func_op.to_i(); + + push_debug(spv::Op::OpName, + {Operand::Int(func_id), Operand::String(func->name())}); + + auto ret_id = GenerateTypeIfNeeded(func->return_type()); + if (ret_id == 0) { + return false; + } + + // TODO(dsinclair): Handle parameters + push_inst(spv::Op::OpFunction, {Operand::Int(ret_id), func_op, + Operand::Int(SpvFunctionControlMaskNone), + Operand::Int(func_type_id)}); + push_inst(spv::Op::OpLabel, {result_op()}); + + // TODO(dsinclair): Function body ... + + push_inst(spv::Op::OpFunctionEnd, {}); + + func_name_to_id_[func->name()] = func_id; + return true; +} + +uint32_t Builder::GenerateFunctionTypeIfNeeded(ast::Function* func) { + auto val = type_name_to_id_.find(func->type_name()); + if (val != type_name_to_id_.end()) { + return val->second; + } + + auto func_op = result_op(); + auto func_type_id = func_op.to_i(); + + auto ret_id = GenerateTypeIfNeeded(func->return_type()); + if (ret_id == 0) { + return 0; + } + + // TODO(dsinclair): Handle parameters + push_type(spv::Op::OpTypeFunction, {func_op, Operand::Int(ret_id)}); + + type_name_to_id_[func->type_name()] = func_type_id; + return func_type_id; +} + void Builder::GenerateImport(ast::Import* imp) { auto result = result_op(); auto id = result.to_i(); diff --git a/src/writer/spirv/builder.h b/src/writer/spirv/builder.h index 31efbde760..fe56501f96 100644 --- a/src/writer/spirv/builder.h +++ b/src/writer/spirv/builder.h @@ -110,7 +110,7 @@ class Builder { instructions_.push_back(Instruction{op, operands}); } /// @returns the instruction list - const std::vector& inst() const { return instructions_; } + const std::vector& instructions() const { return instructions_; } /// Adds an instruction to the annotations /// @param op the op to set /// @param operands the operands for the instruction @@ -124,6 +124,14 @@ class Builder { /// @param ep the entry point /// @returns true if the instruction was generated, false otherwise bool GenerateEntryPoint(ast::EntryPoint* ep); + /// Generates the instructions for a function + /// @param func the function to generate + /// @returns true if the instructions were generated + bool GenerateFunction(ast::Function* func); + /// Generates a function type if not already created + /// @param func the function to generate for + /// @returns the ID to use for the function type. Returns 0 on failure. + uint32_t GenerateFunctionTypeIfNeeded(ast::Function* func); /// Generates an import instruction /// @param imp the import void GenerateImport(ast::Import* imp); diff --git a/src/writer/spirv/builder_entry_point_test.cc b/src/writer/spirv/builder_entry_point_test.cc index 953fb4765e..44213b42fe 100644 --- a/src/writer/spirv/builder_entry_point_test.cc +++ b/src/writer/spirv/builder_entry_point_test.cc @@ -85,7 +85,7 @@ TEST_P(EntryPointStageTest, Emit) { ASSERT_EQ(preamble.size(), 1); EXPECT_EQ(preamble[0].opcode(), spv::Op::OpEntryPoint); - ASSERT_TRUE(preamble[0].operands().size() >= 3); + ASSERT_GE(preamble[0].operands().size(), 3); EXPECT_EQ(preamble[0].operands()[0].to_i(), params.model); } INSTANTIATE_TEST_SUITE_P( diff --git a/src/writer/spirv/builder_function_test.cc b/src/writer/spirv/builder_function_test.cc new file mode 100644 index 0000000000..891675da09 --- /dev/null +++ b/src/writer/spirv/builder_function_test.cc @@ -0,0 +1,80 @@ +// 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 + +#include "gtest/gtest.h" +#include "spirv/unified1/spirv.h" +#include "spirv/unified1/spirv.hpp11" +#include "src/ast/function.h" +#include "src/ast/type/void_type.h" +#include "src/writer/spirv/builder.h" +#include "src/writer/spirv/spv_dump.h" + +namespace tint { +namespace writer { +namespace spirv { + +using BuilderTest = testing::Test; + +TEST_F(BuilderTest, Function_Empty) { + ast::type::VoidType void_type; + ast::Function func("a_func", {}, &void_type); + + Builder b; + ASSERT_TRUE(b.GenerateFunction(&func)); + + EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %3 "a_func" +)"); + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid +%1 = OpTypeFunction %2 +)"); + EXPECT_EQ(DumpInstructions(b.instructions()), R"(%3 = OpFunction %2 None %1 +%4 = OpLabel +OpFunctionEnd +)"); +} + +TEST_F(BuilderTest, DISABLED_Function_WithParams) {} + +TEST_F(BuilderTest, DISABLED_Function_WithBody) {} + +TEST_F(BuilderTest, FunctionType) { + ast::type::VoidType void_type; + ast::Function func("a_func", {}, &void_type); + + Builder b; + ASSERT_TRUE(b.GenerateFunction(&func)); + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid +%1 = OpTypeFunction %2 +)"); +} + +TEST_F(BuilderTest, FunctionType_DeDuplicate) { + ast::type::VoidType void_type; + ast::Function func1("a_func", {}, &void_type); + ast::Function func2("b_func", {}, &void_type); + + Builder b; + ASSERT_TRUE(b.GenerateFunction(&func1)); + ASSERT_TRUE(b.GenerateFunction(&func2)); + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid +%1 = OpTypeFunction %2 +)"); +} + + +} // namespace spirv +} // namespace writer +} // namespace tint diff --git a/src/writer/spirv/spv_dump.h b/src/writer/spirv/spv_dump.h index 34a023ec38..c9079c78af 100644 --- a/src/writer/spirv/spv_dump.h +++ b/src/writer/spirv/spv_dump.h @@ -16,6 +16,7 @@ #define SRC_WRITER_SPIRV_SPV_DUMP_H_ #include +#include #include "src/writer/spirv/builder.h" #include "src/writer/spirv/instruction.h"