From ad27ee8c10382caf2ee18d84c6466a6c25ad5893 Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Thu, 28 Jan 2021 21:22:39 +0000
Subject: [PATCH] spirv-reader: Handle OpInBoundsAccessChain for storage class
 translation

Fixed: tint:457
Change-Id: I25410a1e59cbc78f76da65c28312a8ddd6b3319b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39220
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
---
 src/reader/spirv/function.cc             |  1 +
 src/reader/spirv/function_memory_test.cc | 49 ++++++++++++++++++++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 596f90e704..44abc7b897 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -3653,6 +3653,7 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() {
               // Keep the default decision based on the result type.
               break;
             case SpvOpAccessChain:
+            case SpvOpInBoundsAccessChain:
             case SpvOpCopyObject:
               // Inherit from the first operand. We need this so we can pick up
               // a remapped storage buffer.
diff --git a/src/reader/spirv/function_memory_test.cc b/src/reader/spirv/function_memory_test.cc
index d28cdba51f..379286cf77 100644
--- a/src/reader/spirv/function_memory_test.cc
+++ b/src/reader/spirv/function_memory_test.cc
@@ -848,8 +848,8 @@ TEST_F(SpvParserTest, RemapStorageBuffer_ThroughAccessChain_NonCascaded) {
   ASSERT_TRUE(p->BuildAndParseInternalModule()) << assembly << p->error();
   FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
   EXPECT_TRUE(fe.EmitBody()) << p->error();
-  EXPECT_THAT(ToString(p->builder().Symbols(), fe.ast_body()),
-              HasSubstr(R"(Assignment{
+  const auto got = ToString(p->builder().Symbols(), fe.ast_body());
+  EXPECT_THAT(got, HasSubstr(R"(Assignment{
   MemberAccessor[not set]{
     Identifier[not set]{myvar}
     Identifier[not set]{field0}
@@ -865,7 +865,50 @@ Assignment{
     ScalarConstructor[not set]{1}
   }
   ScalarConstructor[not set]{0}
-})")) << ToString(p->builder().Symbols(), fe.ast_body())
+})")) << got
+      << p->error();
+}
+
+TEST_F(SpvParserTest,
+       RemapStorageBuffer_ThroughAccessChain_NonCascaded_InBoundsAccessChain) {
+  // Like the previous test, but using OpInBoundsAccessChain.
+  const auto assembly = OldStorageBufferPreamble() + R"(
+  %100 = OpFunction %void None %voidfn
+  %entry = OpLabel
+
+  ; the scalar element
+  %1 = OpInBoundsAccessChain %ptr_uint %myvar %uint_0
+  OpStore %1 %uint_0
+
+  ; element in the runtime array
+  %2 = OpInBoundsAccessChain %ptr_uint %myvar %uint_1 %uint_1
+  OpStore %2 %uint_0
+
+  OpReturn
+  OpFunctionEnd
+)";
+  auto p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModule()) << assembly << p->error();
+  FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
+  EXPECT_TRUE(fe.EmitBody()) << p->error();
+  const auto got = ToString(p->builder().Symbols(), fe.ast_body());
+  EXPECT_THAT(got, HasSubstr(R"(Assignment{
+  MemberAccessor[not set]{
+    Identifier[not set]{myvar}
+    Identifier[not set]{field0}
+  }
+  ScalarConstructor[not set]{0}
+}
+Assignment{
+  ArrayAccessor[not set]{
+    MemberAccessor[not set]{
+      Identifier[not set]{myvar}
+      Identifier[not set]{field1}
+    }
+    ScalarConstructor[not set]{1}
+  }
+  ScalarConstructor[not set]{0}
+})")) << got
       << p->error();
 }