[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>

BUILD.gn

@@ -729,6 +729,7 @@ source_set("tint_unittests_spv_reader_src") {
     "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",

src/CMakeLists.txt

@@ -344,6 +344,7 @@ if(${TINT_BUILD_SPV_READER})
     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 @@ if(${TINT_BUILD_SPV_READER})
     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

src/reader/spirv/function.cc

@@ -569,6 +569,8 @@ bool FunctionEmitter::EmitFunctionDeclaration() {
         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 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
       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 @@ TypedExpression FunctionEmitter::MakeNumericConversion(
                               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

src/reader/spirv/function.h

@@ -669,6 +669,11 @@ class FunctionEmitter {
   /// @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:

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

src/reader/spirv/parser_impl_function_decl_test.cc

@@ -85,47 +85,85 @@ TEST_F(SpvParserTest, EmitFunctions_VoidFunctionWithoutParams) {
 }
 
 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 @@ TEST_F(SpvParserTest, EmitFunctions_GenerateParamNames) {
   )
   {
     Return{}
-  })"));
+  })"))
+      << module_ast;
 }
 
 }  // namespace