[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
