From 2c60b4c2cc703cc140a30fcafead4db5773851c6 Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Wed, 8 Apr 2020 17:18:07 +0000
Subject: [PATCH] [spirv-reader] use Function storage class on function vars

Change-Id: I22c76ea8bce7d050411b9dd0e283abe1dae8bc6c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/19101
Reviewed-by: dan sinclair <dsinclair@google.com>
---
 src/reader/spirv/function.cc          |  7 ++---
 src/reader/spirv/function_var_test.cc | 44 +++++++++++++--------------
 src/reader/spirv/parser_impl.h        |  2 +-
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 6cb34f0373..7e7a5b4e05 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -144,11 +144,8 @@ bool FunctionEmitter::EmitFunctionVariables() {
     if (failed()) {
       return false;
     }
-    // Use StorageClass::kNone because function variables should not explicitly mention
-    // their storage class.
-    auto var =
-        parser_impl_.MakeVariable(inst.result_id(),
-                                  ast::StorageClass::kNone, var_store_type);
+    auto var = parser_impl_.MakeVariable(
+        inst.result_id(), ast::StorageClass::kFunction, var_store_type);
     if (inst.NumInOperands() > 1) {
       // SPIR-V initializers are always constants.
       // (OpenCL also allows the ID of an OpVariable, but we don't handle that
diff --git a/src/reader/spirv/function_var_test.cc b/src/reader/spirv/function_var_test.cc
index 8dd8318584..f80aa4f5da 100644
--- a/src/reader/spirv/function_var_test.cc
+++ b/src/reader/spirv/function_var_test.cc
@@ -87,21 +87,21 @@ TEST_F(SpvParserTest, EmitFunctionVariables_AnonymousVars) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     x_1
-    none
+    function
     __u32
   }
 }
 VariableDeclStatement{
   Variable{
     x_2
-    none
+    function
     __u32
   }
 }
 VariableDeclStatement{
   Variable{
     x_3
-    none
+    function
     __u32
   }
 }
@@ -125,21 +125,21 @@ TEST_F(SpvParserTest, EmitFunctionVariables_NamedVars) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     a
-    none
+    function
     __u32
   }
 }
 VariableDeclStatement{
   Variable{
     b
-    none
+    function
     __u32
   }
 }
 VariableDeclStatement{
   Variable{
     c
-    none
+    function
     __u32
   }
 }
@@ -163,21 +163,21 @@ TEST_F(SpvParserTest, EmitFunctionVariables_MixedTypes) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     a
-    none
+    function
     __u32
   }
 }
 VariableDeclStatement{
   Variable{
     b
-    none
+    function
     __i32
   }
 }
 VariableDeclStatement{
   Variable{
     c
-    none
+    function
     __f32
   }
 }
@@ -204,7 +204,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ScalarInitializers) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     a
-    none
+    function
     __bool
     {
       ScalarConstructor{true}
@@ -214,7 +214,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ScalarInitializers) {
 VariableDeclStatement{
   Variable{
     b
-    none
+    function
     __bool
     {
       ScalarConstructor{false}
@@ -224,7 +224,7 @@ VariableDeclStatement{
 VariableDeclStatement{
   Variable{
     c
-    none
+    function
     __i32
     {
       ScalarConstructor{-1}
@@ -234,7 +234,7 @@ VariableDeclStatement{
 VariableDeclStatement{
   Variable{
     d
-    none
+    function
     __u32
     {
       ScalarConstructor{1}
@@ -244,7 +244,7 @@ VariableDeclStatement{
 VariableDeclStatement{
   Variable{
     e
-    none
+    function
     __f32
     {
       ScalarConstructor{1.500000}
@@ -278,7 +278,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ScalarNullInitializers) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     a
-    none
+    function
     __bool
     {
       ScalarConstructor{false}
@@ -288,7 +288,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ScalarNullInitializers) {
 VariableDeclStatement{
   Variable{
     b
-    none
+    function
     __i32
     {
       ScalarConstructor{0}
@@ -298,7 +298,7 @@ VariableDeclStatement{
 VariableDeclStatement{
   Variable{
     c
-    none
+    function
     __u32
     {
       ScalarConstructor{0}
@@ -308,7 +308,7 @@ VariableDeclStatement{
 VariableDeclStatement{
   Variable{
     d
-    none
+    function
     __f32
     {
       ScalarConstructor{0.000000}
@@ -337,7 +337,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_VectorInitializer) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     x_200
-    none
+    function
     __vec_2__f32
     {
       TypeConstructor{
@@ -375,7 +375,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_MatrixInitializer) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     x_200
-    none
+    function
     __mat_2_3__f32
     {
       TypeConstructor{
@@ -421,7 +421,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ArrayInitializer) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     x_200
-    none
+    function
     __array__u32_2
     {
       TypeConstructor{
@@ -455,7 +455,7 @@ TEST_F(SpvParserTest, EmitFunctionVariables_StructInitializer) {
   EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
   Variable{
     x_200
-    none
+    function
     __struct_S
     {
       TypeConstructor{
diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h
index 8edc327467..da73bcf824 100644
--- a/src/reader/spirv/parser_impl.h
+++ b/src/reader/spirv/parser_impl.h
@@ -214,7 +214,7 @@ class ParserImpl : Reader {
   /// Creates an AST Variable node for a SPIR-V ID, including any attached
   /// decorations.
   /// @param id the SPIR-V result ID
-  /// @param sc the storage class, which can be ast::StorageClass::kNone
+  /// @param sc the storage class, which cannot be ast::StorageClass::kNone
   /// @param type the type
   /// @returns a new Variable node, or null in the error case
   std::unique_ptr<ast::Variable> MakeVariable(uint32_t id,