[spirv-reader] Support function calls, except returning void Functions returning void are blocked on https://crbug.com/tint/45 Change-Id: I15ec9cf0e267571bbfab921c678a59e25d0e3619 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25280 Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn index 67d7557..1c0b35a 100644 --- a/BUILD.gn +++ b/BUILD.gn
@@ -729,6 +729,7 @@ "src/reader/spirv/fail_stream_test.cc", "src/reader/spirv/function_arithmetic_test.cc", "src/reader/spirv/function_bit_test.cc", + "src/reader/spirv/function_call_test.cc", "src/reader/spirv/function_cfg_test.cc", "src/reader/spirv/function_composite_test.cc", "src/reader/spirv/function_conversion_test.cc",
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 23f3988..ed8c279 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt
@@ -344,6 +344,7 @@ reader/spirv/function_arithmetic_test.cc reader/spirv/function_bit_test.cc reader/spirv/function_cfg_test.cc + reader/spirv/function_call_test.cc reader/spirv/function_composite_test.cc reader/spirv/function_conversion_test.cc reader/spirv/function_decl_test.cc @@ -361,8 +362,8 @@ reader/spirv/parser_impl_import_test.cc reader/spirv/parser_impl_module_var_test.cc reader/spirv/parser_impl_named_types_test.cc - reader/spirv/parser_impl_user_name_test.cc reader/spirv/parser_impl_test.cc + reader/spirv/parser_impl_user_name_test.cc reader/spirv/parser_test.cc reader/spirv/spirv_tools_helpers_test.cc reader/spirv/spirv_tools_helpers_test.h
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 38e4e60..3137cc4 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc
@@ -569,6 +569,8 @@ if (ast_type != nullptr) { ast_params.emplace_back(parser_impl_.MakeVariable( param->result_id(), ast::StorageClass::kNone, ast_type)); + // The value is accessible by name. + identifier_values_.insert(param->result_id()); } else { // We've already logged an error and emitted a diagnostic. Do nothing // here. @@ -2599,8 +2601,7 @@ return EmitConstDefOrWriteToHoistedVar(inst, std::move(expr)); } case SpvOpFunctionCall: - // TODO(dneto): Fill this out. Make this pass, for existing tests - return success(); + return EmitFunctionCall(inst); default: break; } @@ -3350,6 +3351,30 @@ requested_type, std::move(result.expr))}; } +bool FunctionEmitter::EmitFunctionCall(const spvtools::opt::Instruction& inst) { + // We ignore function attributes such as Inline, DontInline, Pure, Const. + auto function = std::make_unique<ast::IdentifierExpression>( + namer_.Name(inst.GetSingleWordInOperand(0))); + + ast::ExpressionList params; + for (uint32_t iarg = 1; iarg < inst.NumInOperands(); ++iarg) { + params.emplace_back(MakeOperand(inst, iarg).expr); + } + TypedExpression expr{parser_impl_.ConvertType(inst.type_id()), + std::make_unique<ast::CallExpression>( + std::move(function), std::move(params))}; + + if (expr.type->IsVoid()) { + // TODO(dneto): Tint AST needs support for function call as a statement + // https://bugs.chromium.org/p/tint/issues/detail?id=45 + return Fail() << "missing support for function call as a statement: can't " + "generate code for function call returning void: " + << inst.PrettyPrint(); + } + + return EmitConstDefOrWriteToHoistedVar(inst, std::move(expr)); +} + } // namespace spirv } // namespace reader } // namespace tint
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index e3bde3a..7c9def4 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h
@@ -669,6 +669,11 @@ /// @returns an expression TypedExpression MakeAccessChain(const spvtools::opt::Instruction& inst); + /// Emits a function call. On failure, emits a diagnostic and returns false. + /// @param inst the SPIR-V function call instruction + /// @returns false if emission failed + bool EmitFunctionCall(const spvtools::opt::Instruction& inst); + /// Finds the header block for a structured construct that we can "break" /// out from, from deeply nested control flow, if such a block exists. /// If the construct is:
diff --git a/src/reader/spirv/function_call_test.cc b/src/reader/spirv/function_call_test.cc new file mode 100644 index 0000000..1f0b0f4 --- /dev/null +++ b/src/reader/spirv/function_call_test.cc
@@ -0,0 +1,258 @@ +// 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 <string> +#include <vector> + +#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::Eq; +using ::testing::HasSubstr; + +TEST_F(SpvParserTest, EmitStatement_VoidCallNoParams) { + auto p = parser(test::Assemble(R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + + %50 = OpFunction %void None %voidfn + %entry_50 = OpLabel + OpReturn + OpFunctionEnd + + %100 = OpFunction %void None %voidfn + %entry = OpLabel + %1 = OpFunctionCall %void %50 + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_FALSE(fe.EmitBody()); + EXPECT_THAT( + p->error(), + Eq("missing support for function call as a statement: can't generate " + "code for function call returning void: %1 = OpFunctionCall %2 %50")); +} + +TEST_F(SpvParserTest, EmitStatement_ScalarCallNoParams) { + auto p = parser(test::Assemble(R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %uint = OpTypeInt 32 0 + %uintfn = OpTypeFunction %uint + %val = OpConstant %uint 42 + + %50 = OpFunction %uint None %uintfn + %entry_50 = OpLabel + OpReturnValue %val + OpFunctionEnd + + %100 = OpFunction %void None %voidfn + %entry = OpLabel + %1 = OpFunctionCall %uint %50 + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + { + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()); + EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{ + Variable{ + x_1 + none + __u32 + { + Call{ + Identifier{x_50} + ( + ) + } + } + } +} +Return{})")) + << ToString(fe.ast_body()); + } + + { + FunctionEmitter fe(p, *spirv_function(50)); + EXPECT_TRUE(fe.EmitBody()); + EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(Return{ + { + ScalarConstructor{42} + } +})")) << ToString(fe.ast_body()); + } +} + +TEST_F(SpvParserTest, EmitStatement_ScalarCallNoParamsUsedTwice) { + auto p = parser(test::Assemble(R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %uint = OpTypeInt 32 0 + %uintfn = OpTypeFunction %uint + %val = OpConstant %uint 42 + %ptr_uint = OpTypePointer Function %uint + + %50 = OpFunction %uint None %uintfn + %entry_50 = OpLabel + OpReturnValue %val + OpFunctionEnd + + %100 = OpFunction %void None %voidfn + %entry = OpLabel + %10 = OpVariable %ptr_uint Function + %1 = OpFunctionCall %uint %50 + OpStore %10 %1 + OpStore %10 %1 + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + { + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{ + Variable{ + x_10 + function + __u32 + } +} +VariableDeclStatement{ + Variable{ + x_1 + none + __u32 + { + Call{ + Identifier{x_50} + ( + ) + } + } + } +} +Assignment{ + Identifier{x_10} + Identifier{x_1} +} +Assignment{ + Identifier{x_10} + Identifier{x_1} +} +Return{})")) + << ToString(fe.ast_body()); + } + { + FunctionEmitter fe(p, *spirv_function(50)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(Return{ + { + ScalarConstructor{42} + } +})")) << ToString(fe.ast_body()); + } +} + +TEST_F(SpvParserTest, EmitStatement_CallWithParams) { + auto p = parser(test::Assemble(R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %uint = OpTypeInt 32 0 + %uintfn_uint_uint = OpTypeFunction %uint %uint %uint + %val = OpConstant %uint 42 + %val2 = OpConstant %uint 84 + + %50 = OpFunction %uint None %uintfn_uint_uint + %51 = OpFunctionParameter %uint + %52 = OpFunctionParameter %uint + %entry_50 = OpLabel + %sum = OpIAdd %uint %51 %52 + OpReturnValue %sum + OpFunctionEnd + + %100 = OpFunction %void None %voidfn + %entry = OpLabel + %1 = OpFunctionCall %uint %50 %val %val2 + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error(); + EXPECT_TRUE(p->error().empty()); + const auto module_ast_str = p->module().to_str(); + EXPECT_THAT(module_ast_str, HasSubstr(R"(Module{ + Function x_50 -> __u32 + ( + Variable{ + x_51 + none + __u32 + } + Variable{ + x_52 + none + __u32 + } + ) + { + Return{ + { + Binary{ + Identifier{x_51} + add + Identifier{x_52} + } + } + } + } + Function x_100 -> __void + () + { + VariableDeclStatement{ + Variable{ + x_1 + none + __u32 + { + Call{ + Identifier{x_50} + ( + ScalarConstructor{42} + ScalarConstructor{84} + ) + } + } + } + } + Return{} + } +})")) << module_ast_str; +} + +} // namespace +} // namespace spirv +} // namespace reader +} // namespace tint
diff --git a/src/reader/spirv/parser_impl_function_decl_test.cc b/src/reader/spirv/parser_impl_function_decl_test.cc index 03ef876..7fc86d4 100644 --- a/src/reader/spirv/parser_impl_function_decl_test.cc +++ b/src/reader/spirv/parser_impl_function_decl_test.cc
@@ -85,47 +85,85 @@ } TEST_F(SpvParserTest, EmitFunctions_CalleePrecedesCaller) { - auto* p = parser( - test::Assemble(Names({"root", "branch", "leaf"}) + CommonTypes() + R"( + auto* p = parser(test::Assemble( + Names({"root", "branch", "leaf", "leaf_result", "branch_result"}) + + CommonTypes() + R"( + %uintfn = OpTypeFunction %uint + %uint_0 = OpConstant %uint 0 + %root = OpFunction %void None %voidfn %root_entry = OpLabel - %branch_result = OpFunctionCall %void %branch + %branch_result = OpFunctionCall %uint %branch OpReturn OpFunctionEnd - %branch = OpFunction %void None %voidfn + %branch = OpFunction %uint None %uintfn %branch_entry = OpLabel - %leaf_result = OpFunctionCall %void %leaf - OpReturn + %leaf_result = OpFunctionCall %uint %leaf + OpReturnValue %leaf_result OpFunctionEnd - %leaf = OpFunction %void None %voidfn + %leaf = OpFunction %uint None %uintfn %leaf_entry = OpLabel - OpReturn + OpReturnValue %uint_0 OpFunctionEnd )")); EXPECT_TRUE(p->BuildAndParseInternalModule()); EXPECT_TRUE(p->error().empty()); const auto module_ast = p->module().to_str(); - // TODO(dneto): This will need to be updated when function calls are - // supported. Otherwise, use more general matching instead of substring - // equality. EXPECT_THAT(module_ast, HasSubstr(R"( - Function leaf -> __void + Function leaf -> __u32 () { - Return{} + Return{ + { + ScalarConstructor{0} + } + } } - Function branch -> __void + Function branch -> __u32 () { - Return{} + VariableDeclStatement{ + Variable{ + leaf_result + none + __u32 + { + Call{ + Identifier{leaf} + ( + ) + } + } + } + } + Return{ + { + Identifier{leaf_result} + } + } } Function root -> __void () { + VariableDeclStatement{ + Variable{ + branch_result + none + __u32 + { + Call{ + Identifier{branch} + ( + ) + } + } + } + } Return{} - })")); + } +})")) << module_ast; } TEST_F(SpvParserTest, EmitFunctions_NonVoidResultType) { @@ -229,7 +267,8 @@ ) { Return{} - })")); + })")) + << module_ast; } } // namespace