spirv-reader: infer function storage class on OpAccessChain without indices

Fixed: tint:941
Bug: tint:807
Change-Id: I37bec630c1b83dbb56d2f8937322a5b5be81f407
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56660
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/reader/spirv/function.cc

@@ -3786,6 +3786,21 @@ TypedExpression FunctionEmitter::MakeOperand(
   return parser_impl_.RectifyOperandSignedness(inst, std::move(expr));
 }
 
+TypedExpression FunctionEmitter::InferFunctionStorageClass(
+    TypedExpression expr) {
+  TypedExpression result(expr);
+  if (const auto* ref = expr.type->UnwrapAlias()->As<Reference>()) {
+    if (ref->storage_class == ast::StorageClass::kNone) {
+      expr.type = ty_.Reference(ref->type, ast::StorageClass::kFunction);
+    }
+  } else if (const auto* ptr = expr.type->UnwrapAlias()->As<Pointer>()) {
+    if (ptr->storage_class == ast::StorageClass::kNone) {
+      expr.type = ty_.Pointer(ptr->type, ast::StorageClass::kFunction);
+    }
+  }
+  return expr;
+}
+
 TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue(
     const spvtools::opt::Instruction& inst) {
   if (inst.result_id() == 0) {
@@ -4118,7 +4133,7 @@ TypedExpression FunctionEmitter::MakeAccessChain(
   // ever-deeper nested indexing expressions. Start off with an expression
   // for the base, and then bury that inside nested indexing expressions.
   if (!current_expr) {
-    current_expr = MakeOperand(inst, 0);
+    current_expr = InferFunctionStorageClass(MakeOperand(inst, 0));
     if (current_expr.type->Is<Pointer>()) {
       current_expr = Dereference(current_expr);
     }

src/reader/spirv/function.h

@@ -964,6 +964,13 @@ class FunctionEmitter {
   TypedExpression MakeOperand(const spvtools::opt::Instruction& inst,
                               uint32_t operand_index);
 
+  /// Copies a typed expression to the result, but when the type is a pointer
+  /// or reference type, ensures the storage class is not defaulted.  That is,
+  /// it changes a storage class of "none" to "function".
+  /// @param expr a typed expression
+  /// @results a copy of the expression, with possibly updated type
+  TypedExpression InferFunctionStorageClass(TypedExpression expr);
+
   /// Returns an expression for a SPIR-V OpAccessChain or OpInBoundsAccessChain
   /// instruction.
   /// @param inst the SPIR-V instruction

src/reader/spirv/function_memory_test.cc

@@ -903,6 +903,76 @@ TEST_F(SpvParserMemoryTest, EmitStatement_AccessChain_DereferenceBase) {
   EXPECT_EQ(got, expected) << got;
 }
 
+TEST_F(SpvParserMemoryTest,
+       EmitStatement_AccessChain_InferFunctionStorageClass) {
+  // An access chain can have no indices. When the base is a Function variable,
+  // the reference type has no explicit storage class in the AST representation.
+  // But the pointer type for the let declaration must have an explicit
+  // 'function' storage class. From crbug.com/tint/807
+  const std::string assembly = R"(
+OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+
+%uint = OpTypeInt 32 0
+%ptr_ty = OpTypePointer Function %uint
+
+  %void = OpTypeVoid
+%voidfn = OpTypeFunction %void
+  %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+     %1 = OpVariable %ptr_ty Function
+     %2 = OpAccessChain %ptr_ty %1
+          OpReturn
+          OpFunctionEnd
+)";
+  auto p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModule()) << assembly;
+  const auto got = p->program().to_str();
+  const std::string expected = R"(Module{
+  Function main_1 -> __void
+  ()
+  {
+    VariableDeclStatement{
+      Variable{
+        x_1
+        none
+        undefined
+        __u32
+      }
+    }
+    VariableDeclStatement{
+      VariableConst{
+        x_2
+        none
+        undefined
+        __ptr_function__u32
+        {
+          UnaryOp[not set]{
+            address-of
+            Identifier[not set]{x_1}
+          }
+        }
+      }
+    }
+    Return{}
+  }
+  Function main -> __void
+  StageDecoration{fragment}
+  ()
+  {
+    Call[not set]{
+      Identifier[not set]{main_1}
+      (
+      )
+    }
+  }
+}
+)";
+  EXPECT_EQ(got, expected) << got;
+}
+
 std::string OldStorageBufferPreamble() {
   return Preamble() + R"(
      OpName %myvar "myvar"