From 28d0f4b905b1e262d4f052afe960f6ed6a9e5dd2 Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Tue, 21 Jul 2020 16:08:56 +0000
Subject: [PATCH] [spirv-reader] Support function call returning void

Bug: tint:3, tint:99
Change-Id: I6fe6d06d79115e6b3b0adb800f21c4704795dc15
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25360
Reviewed-by: dan sinclair <dsinclair@chromium.org>
---
 src/reader/spirv/function.cc           | 23 ++++++++++++----------
 src/reader/spirv/function_call_test.cc | 27 +++++++++++++++++++-------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index c3a6d9b088..66f5c1c29a 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -34,6 +34,7 @@
 #include "src/ast/bool_literal.h"
 #include "src/ast/break_statement.h"
 #include "src/ast/call_expression.h"
+#include "src/ast/call_statement.h"
 #include "src/ast/case_statement.h"
 #include "src/ast/cast_expression.h"
 #include "src/ast/continue_statement.h"
@@ -3365,19 +3366,21 @@ bool FunctionEmitter::EmitFunctionCall(const spvtools::opt::Instruction& inst) {
   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: "
+  auto call_expr = std::make_unique<ast::CallExpression>(std::move(function),
+                                                         std::move(params));
+  auto result_type = parser_impl_.ConvertType(inst.type_id());
+  if (!result_type) {
+    return Fail() << "internal error: no mapped type result of call: "
                   << inst.PrettyPrint();
   }
 
-  return EmitConstDefOrWriteToHoistedVar(inst, std::move(expr));
+  if (result_type->IsVoid()) {
+    return nullptr != AddStatement(std::make_unique<ast::CallStatement>(
+                          std::move(call_expr)));
+  }
+
+  return EmitConstDefOrWriteToHoistedVar(inst,
+                                         {result_type, std::move(call_expr)});
 }
 
 TypedExpression FunctionEmitter::MakeSimpleSelect(
diff --git a/src/reader/spirv/function_call_test.cc b/src/reader/spirv/function_call_test.cc
index 1f0b0f47c2..c38f136825 100644
--- a/src/reader/spirv/function_call_test.cc
+++ b/src/reader/spirv/function_call_test.cc
@@ -46,13 +46,26 @@ TEST_F(SpvParserTest, EmitStatement_VoidCallNoParams) {
      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"));
+  ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error();
+  const auto module_ast_str = p->module().to_str();
+  EXPECT_THAT(module_ast_str, Eq(R"(Module{
+  Function x_50 -> __void
+  ()
+  {
+    Return{}
+  }
+  Function x_100 -> __void
+  ()
+  {
+    Call{
+      Identifier{x_50}
+      (
+      )
+    }
+    Return{}
+  }
+}
+)")) << module_ast_str;
 }
 
 TEST_F(SpvParserTest, EmitStatement_ScalarCallNoParams) {