Add transform::VarForDynamicIndex

Use this as part of the Spirv sanitizer.
Cleans up buggy dynamic array indexing logic in the SPIR-V writer.

Fixed: tint:824
Change-Id: Ia408e49bd808bc8dbf3a1897eb47f9b33b71fdfb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51780
Commit-Queue: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/BUILD.gn b/src/BUILD.gn
index 36f3aad..e7735a2 100644
--- a/src/BUILD.gn
+++ b/src/BUILD.gn
@@ -523,6 +523,8 @@
     "transform/single_entry_point.h",
     "transform/transform.cc",
     "transform/transform.h",
+    "transform/var_for_dynamic_index.cc",
+    "transform/var_for_dynamic_index.h",
     "transform/vertex_pulling.cc",
     "transform/vertex_pulling.h",
     "typepair.h",
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index f928505..f4ff38d 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -287,6 +287,8 @@
   transform/single_entry_point.h
   transform/transform.cc
   transform/transform.h
+  transform/var_for_dynamic_index.cc
+  transform/var_for_dynamic_index.h
   transform/vertex_pulling.cc
   transform/vertex_pulling.h
   sem/bool_type.cc
@@ -810,6 +812,7 @@
       transform/renamer_test.cc
       transform/single_entry_point.cc
       transform/test_helper.h
+      transform/var_for_dynamic_index_test.cc
       transform/vertex_pulling_test.cc
     )
   endif()
diff --git a/src/transform/spirv.cc b/src/transform/spirv.cc
index 20bdf68..ffb8da8 100644
--- a/src/transform/spirv.cc
+++ b/src/transform/spirv.cc
@@ -28,6 +28,7 @@
 #include "src/sem/variable.h"
 #include "src/transform/external_texture_transform.h"
 #include "src/transform/manager.h"
+#include "src/transform/var_for_dynamic_index.h"
 
 TINT_INSTANTIATE_TYPEINFO(tint::transform::Spirv::Config);
 
@@ -40,6 +41,7 @@
 Output Spirv::Run(const Program* in, const DataMap& data) {
   Manager manager;
   manager.Add<ExternalTextureTransform>();
+  manager.Add<VarForDynamicIndex>();
   auto transformedInput = manager.Run(in, data);
 
   auto* cfg = data.Get<Config>();
diff --git a/src/transform/var_for_dynamic_index.cc b/src/transform/var_for_dynamic_index.cc
new file mode 100644
index 0000000..e9af1e7
--- /dev/null
+++ b/src/transform/var_for_dynamic_index.cc
@@ -0,0 +1,80 @@
+// Copyright 2021 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/transform/var_for_dynamic_index.h"
+
+#include <utility>
+
+#include "src/program_builder.h"
+#include "src/sem/array.h"
+#include "src/sem/block_statement.h"
+#include "src/sem/expression.h"
+#include "src/sem/statement.h"
+
+namespace tint {
+namespace transform {
+
+VarForDynamicIndex::VarForDynamicIndex() = default;
+
+VarForDynamicIndex::~VarForDynamicIndex() = default;
+
+Output VarForDynamicIndex::Run(const Program* in, const DataMap&) {
+  ProgramBuilder out;
+  CloneContext ctx(&out, in);
+
+  for (auto* node : in->ASTNodes().Objects()) {
+    if (auto* access_expr = node->As<ast::ArrayAccessorExpression>()) {
+      // Found an array accessor expression
+      auto* index_expr = access_expr->idx_expr();
+      auto* array_expr = access_expr->array();
+
+      if (index_expr->Is<ast::ScalarConstructorExpression>()) {
+        // Index expression is a literal value. As this isn't a dynamic index,
+        // we can ignore this.
+        continue;
+      }
+
+      auto* array = ctx.src->Sem().Get(array_expr);
+      if (!array->Type()->Is<sem::Array>()) {
+        // This transform currently only cares about arrays.
+        continue;
+      }
+
+      auto* stmt = array->Stmt();   // Statement that owns the expression
+      auto* block = stmt->Block();  // Block that owns the statement
+
+      // Construct a `var` declaration to hold the value in memory.
+      auto* ty = CreateASTTypeFor(&ctx, array->Type());
+      auto var_name = ctx.dst->Symbols().New("var_for_array");
+      auto* var_decl = ctx.dst->Decl(ctx.dst->Var(
+          var_name, ty, ast::StorageClass::kNone, ctx.Clone(array_expr)));
+
+      // Insert the `var` declaration before the statement that performs the
+      // indexing. Note that for indexing chains, AST node ordering guarantees
+      // that the inner-most index variable will be placed first in the block.
+      ctx.InsertBefore(block->Declaration()->statements(), stmt->Declaration(),
+                       var_decl);
+
+      // Replace the original index expression with the new `var`.
+      ctx.Replace(array_expr, ctx.dst->Expr(var_name));
+    }
+  }
+
+  ctx.Clone();
+
+  return Output(Program(std::move(out)));
+}
+
+}  // namespace transform
+}  // namespace tint
diff --git a/src/transform/var_for_dynamic_index.h b/src/transform/var_for_dynamic_index.h
new file mode 100644
index 0000000..7d928c6
--- /dev/null
+++ b/src/transform/var_for_dynamic_index.h
@@ -0,0 +1,48 @@
+// Copyright 2021 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.
+
+#ifndef SRC_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_
+#define SRC_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_
+
+#include <string>
+#include <unordered_map>
+
+#include "src/transform/transform.h"
+
+namespace tint {
+namespace transform {
+
+/// A transform that extracts array values that are dynamically indexed to a
+/// temporary `var` local before performing the index. This transform is used by
+/// the SPIR-V writer for dynamically indexing arrays, as there is no SPIR-V
+/// instruction that can dynamically index a non-pointer composite.
+class VarForDynamicIndex : public Transform {
+ public:
+  /// Constructor
+  VarForDynamicIndex();
+
+  /// Destructor
+  ~VarForDynamicIndex() override;
+
+  /// Runs the transform on `program`, returning the transformation result.
+  /// @param program the source program to transform
+  /// @param data optional extra transform-specific input data
+  /// @returns the transformation result
+  Output Run(const Program* program, const DataMap& data = {}) override;
+};
+
+}  // namespace transform
+}  // namespace tint
+
+#endif  // SRC_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_
diff --git a/src/transform/var_for_dynamic_index_test.cc b/src/transform/var_for_dynamic_index_test.cc
new file mode 100644
index 0000000..4626dea
--- /dev/null
+++ b/src/transform/var_for_dynamic_index_test.cc
@@ -0,0 +1,121 @@
+// Copyright 2021 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/transform/var_for_dynamic_index.h"
+
+#include "src/transform/test_helper.h"
+
+namespace tint {
+namespace transform {
+namespace {
+
+using VarForDynamicIndexTest = TransformTest;
+
+TEST_F(VarForDynamicIndexTest, EmptyModule) {
+  auto* src = "";
+  auto* expect = "";
+
+  auto got = Run<VarForDynamicIndex>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(VarForDynamicIndexTest, ArrayIndexDynamic) {
+  auto* src = R"(
+fn f() {
+  var i : i32;
+  let p : array<i32, 4> = array<i32, 4>(1, 2, 3, 4);
+  let x : i32 = p[i];
+}
+)";
+
+  auto* expect = R"(
+fn f() {
+  var i : i32;
+  let p : array<i32, 4> = array<i32, 4>(1, 2, 3, 4);
+  var var_for_array : array<i32, 4> = p;
+  let x : i32 = var_for_array[i];
+}
+)";
+
+  auto got = Run<VarForDynamicIndex>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(VarForDynamicIndexTest, ArrayIndexDynamicChain) {
+  auto* src = R"(
+fn f() {
+  var i : i32;
+  var j : i32;
+  let p : array<array<i32, 2>, 2> = array<array<i32, 2>, 2>(array<i32, 2>(1, 2), array<i32, 2>(3, 4));
+  let x : i32 = p[i][j];
+}
+)";
+
+  // TODO(bclayton): Optimize this case:
+  // This output is not as efficient as it could be.
+  // We only actually need to hoist the inner-most array to a `var`
+  // (`var_for_array`), as later indexing operations will be working with
+  // references, not values.
+
+  auto* expect = R"(
+fn f() {
+  var i : i32;
+  var j : i32;
+  let p : array<array<i32, 2>, 2> = array<array<i32, 2>, 2>(array<i32, 2>(1, 2), array<i32, 2>(3, 4));
+  var var_for_array : array<array<i32, 2>, 2> = p;
+  var var_for_array_1 : array<i32, 2> = var_for_array[i];
+  let x : i32 = var_for_array_1[j];
+}
+)";
+
+  auto got = Run<VarForDynamicIndex>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(VarForDynamicIndexTest, ArrayIndexLiteral) {
+  auto* src = R"(
+fn f() {
+  let p : array<i32, 4> = array<i32, 4>(1, 2, 3, 4);
+  let x : i32 = p[1];
+}
+)";
+
+  auto* expect = src;
+
+  auto got = Run<VarForDynamicIndex>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(VarForDynamicIndexTest, ArrayIndexLiteralChain) {
+  auto* src = R"(
+fn f() {
+  let p : array<array<i32, 2>, 2> = array<array<i32, 2>, 2>(array<i32, 2>(1, 2), array<i32, 2>(3, 4));
+  let x : i32 = p[0][1];
+}
+)";
+
+  auto* expect = src;
+
+  auto got = Run<VarForDynamicIndex>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+}  // namespace
+}  // namespace transform
+}  // namespace tint
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index c59b4e9..f267f06 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -1073,53 +1073,17 @@
   }
   info.source_type = TypeOf(source);
 
-  // If our initial access is into a non-pointer array, and either has a
-  // non-scalar element type or the accessor uses a non-literal index, then we
-  // need to load that array into a variable in order to access chain into it.
-
-  // TODO(bclayton): The requirement for scalar element types is because of
-  // arrays-of-arrays - this logic only considers whether the root index is
-  // compile-time-constant, and not whether there are any dynamic, inner-array
-  // indexing being performed. Instead of trying to do complex hoisting in this
-  // writer, move this hoisting into the transform::Spirv sanitizer.
-
-  bool needs_load = false;  // Was the expression hoist to a temporary variable?
   if (auto* access = accessors[0]->As<ast::ArrayAccessorExpression>()) {
     auto* array = TypeOf(access->array())->As<sem::Array>();
-    bool trivial_indexing =
-        array && array->ElemType()->is_scalar() &&
-        access->idx_expr()->Is<ast::ScalarConstructorExpression>();
-    if (array && !trivial_indexing) {
-      // Wrap the source type in a reference to function storage.
-      auto* ref =
-          builder_.create<sem::Reference>(array, ast::StorageClass::kFunction);
-      auto result_type_id = GenerateTypeIfNeeded(ref);
-      if (result_type_id == 0) {
-        return 0;
-      }
-
-      auto ary_result = result_op();
-
-      auto init = GenerateConstantNullIfNeeded(array);
-
-      // If we're access chaining into an array then we must be in a function
-      push_function_var(
-          {Operand::Int(result_type_id), ary_result,
-           Operand::Int(ConvertStorageClass(ast::StorageClass::kFunction)),
-           Operand::Int(init)});
-
-      if (!push_function_inst(spv::Op::OpStore,
-                              {ary_result, Operand::Int(info.source_id)})) {
-        return false;
-      }
-
-      info.source_id = ary_result.to_i();
-      info.source_type = ref;
-      needs_load = true;
+    bool literal_index =
+        array && access->idx_expr()->Is<ast::ScalarConstructorExpression>();
+    if (array && !literal_index) {
+      TINT_ICE(builder_.Diagnostics())
+          << "Dynamic index on array value should have been promoted to "
+             "storage with the VarForDynamicIndex transform";
     }
   }
 
-  std::vector<uint32_t> access_chain_indices;
   for (auto* accessor : accessors) {
     if (auto* array = accessor->As<ast::ArrayAccessorExpression>()) {
       if (!GenerateArrayAccessor(array, &info)) {
@@ -1138,10 +1102,6 @@
 
   if (!info.access_chain_indices.empty()) {
     auto* type = TypeOf(expr);
-    if (needs_load) {
-      type =
-          builder_.create<sem::Reference>(type, ast::StorageClass::kFunction);
-    }
     auto result_type_id = GenerateTypeIfNeeded(type);
     if (result_type_id == 0) {
       return 0;
@@ -1160,11 +1120,6 @@
       return false;
     }
     info.source_id = result_id;
-
-    // Load from the access chain result if required.
-    if (needs_load) {
-      info.source_id = GenerateLoadIfNeeded(type, result_id);
-    }
   }
 
   return info.source_id;
diff --git a/src/writer/spirv/builder_accessor_expression_test.cc b/src/writer/spirv/builder_accessor_expression_test.cc
index 6e36a06..7e1cb1d 100644
--- a/src/writer/spirv/builder_accessor_expression_test.cc
+++ b/src/writer/spirv/builder_accessor_expression_test.cc
@@ -762,37 +762,32 @@
   auto* expr = IndexAccessor("pos", 1u);
   WrapInFunction(var, expr);
 
-  spirv::Builder& b = Build();
+  spirv::Builder& b = SanitizeAndBuild();
 
-  b.push_function(Function{});
-  ASSERT_TRUE(b.GenerateFunctionVariable(var)) << b.error();
-  EXPECT_EQ(b.GenerateAccessorExpression(expr), 19u) << b.error();
+  ASSERT_TRUE(b.Build());
 
-  EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeFloat 32
-%2 = OpTypeVector %3 2
-%4 = OpTypeInt 32 0
-%5 = OpConstant %4 3
-%1 = OpTypeArray %2 %5
-%6 = OpConstant %3 0
-%7 = OpConstant %3 0.5
-%8 = OpConstantComposite %2 %6 %7
-%9 = OpConstant %3 -0.5
-%10 = OpConstantComposite %2 %9 %9
-%11 = OpConstantComposite %2 %7 %9
-%12 = OpConstantComposite %1 %8 %10 %11
-%13 = OpTypePointer Function %1
-%15 = OpConstantNull %1
-%16 = OpConstant %4 1
-%17 = OpTypePointer Function %2
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid
+%1 = OpTypeFunction %2
+%7 = OpTypeFloat 32
+%6 = OpTypeVector %7 2
+%8 = OpTypeInt 32 0
+%9 = OpConstant %8 3
+%5 = OpTypeArray %6 %9
+%10 = OpConstant %7 0
+%11 = OpConstant %7 0.5
+%12 = OpConstantComposite %6 %10 %11
+%13 = OpConstant %7 -0.5
+%14 = OpConstantComposite %6 %13 %13
+%15 = OpConstantComposite %6 %11 %13
+%16 = OpConstantComposite %5 %12 %14 %15
+%17 = OpConstant %8 1
 )");
-  EXPECT_EQ(DumpInstructions(b.functions()[0].variables()),
-            R"(%14 = OpVariable %13 Function %15
-)");
+  EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), R"()");
   EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
-            R"(OpStore %14 %12
-%18 = OpAccessChain %17 %14 %16
-%19 = OpLoad %2 %18
+            R"(%18 = OpCompositeExtract %6 %16 1
 )");
+
+  Validate(b);
 }
 
 TEST_F(BuilderTest, Accessor_Array_Of_Array_Of_f32) {
@@ -810,39 +805,34 @@
   auto* expr = IndexAccessor(IndexAccessor("pos", 2u), 1u);
   WrapInFunction(var, expr);
 
-  spirv::Builder& b = Build();
+  spirv::Builder& b = SanitizeAndBuild();
 
-  b.push_function(Function{});
-  ASSERT_TRUE(b.GenerateFunctionVariable(var)) << b.error();
-  EXPECT_EQ(b.GenerateAccessorExpression(expr), 21u) << b.error();
+  ASSERT_TRUE(b.Build());
 
-  EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeFloat 32
-%2 = OpTypeVector %3 2
-%4 = OpTypeInt 32 0
-%5 = OpConstant %4 3
-%1 = OpTypeArray %2 %5
-%6 = OpConstant %3 0
-%7 = OpConstant %3 0.5
-%8 = OpConstantComposite %2 %6 %7
-%9 = OpConstant %3 -0.5
-%10 = OpConstantComposite %2 %9 %9
-%11 = OpConstantComposite %2 %7 %9
-%12 = OpConstantComposite %1 %8 %10 %11
-%13 = OpTypePointer Function %1
-%15 = OpConstantNull %1
-%16 = OpConstant %4 2
-%17 = OpConstant %4 1
-%19 = OpTypePointer Function %3
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid
+%1 = OpTypeFunction %2
+%7 = OpTypeFloat 32
+%6 = OpTypeVector %7 2
+%8 = OpTypeInt 32 0
+%9 = OpConstant %8 3
+%5 = OpTypeArray %6 %9
+%10 = OpConstant %7 0
+%11 = OpConstant %7 0.5
+%12 = OpConstantComposite %6 %10 %11
+%13 = OpConstant %7 -0.5
+%14 = OpConstantComposite %6 %13 %13
+%15 = OpConstantComposite %6 %11 %13
+%16 = OpConstantComposite %5 %12 %14 %15
+%17 = OpConstant %8 2
+%19 = OpConstant %8 1
 )");
-  EXPECT_EQ(DumpInstructions(b.functions()[0].variables()),
-            R"(%14 = OpVariable %13 Function %15
-)");
+  EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), R"()");
   EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
-            R"(OpStore %14 %12
-%18 = OpCompositeExtract %3 %14 1
-%20 = OpAccessChain %19 %18 %16
-%21 = OpLoad %3 %20
+            R"(%18 = OpCompositeExtract %6 %16 2
+%20 = OpCompositeExtract %7 %18 1
 )");
+
+  Validate(b);
 }
 
 TEST_F(BuilderTest, Accessor_Const_Vec) {
@@ -919,27 +909,29 @@
   auto* expr = IndexAccessor("a", 2);
   WrapInFunction(var, expr);
 
-  spirv::Builder& b = Build();
+  spirv::Builder& b = SanitizeAndBuild();
 
-  b.push_function(Function{});
-  ASSERT_TRUE(b.GenerateGlobalVariable(var)) << b.error();
-  EXPECT_EQ(b.GenerateAccessorExpression(expr), 11u) << b.error();
+  ASSERT_TRUE(b.Build());
 
-  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
-%3 = OpTypeInt 32 0
-%4 = OpConstant %3 3
-%1 = OpTypeArray %2 %4
-%5 = OpConstant %2 0
-%6 = OpConstant %2 0.5
-%7 = OpConstant %2 1
-%8 = OpConstantComposite %1 %5 %6 %7
-%9 = OpTypeInt 32 1
-%10 = OpConstant %9 2
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid
+%1 = OpTypeFunction %2
+%6 = OpTypeFloat 32
+%7 = OpTypeInt 32 0
+%8 = OpConstant %7 3
+%5 = OpTypeArray %6 %8
+%9 = OpConstant %6 0
+%10 = OpConstant %6 0.5
+%11 = OpConstant %6 1
+%12 = OpConstantComposite %5 %9 %10 %11
+%13 = OpTypeInt 32 1
+%14 = OpConstant %13 2
 )");
   EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), "");
   EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
-            R"(%11 = OpCompositeExtract %2 %8 2
+            R"(%15 = OpCompositeExtract %6 %12 2
 )");
+
+  Validate(b);
 }
 
 TEST_F(BuilderTest, Accessor_Array_NonPointer_Dynamic) {
@@ -955,38 +947,39 @@
 
   WrapInFunction(var, idx, expr);
 
-  spirv::Builder& b = Build();
+  spirv::Builder& b = SanitizeAndBuild();
 
-  b.push_function(Function{});
-  ASSERT_TRUE(b.GenerateGlobalVariable(var)) << b.error();
-  ASSERT_TRUE(b.GenerateFunctionVariable(idx)) << b.error();
-  EXPECT_EQ(b.GenerateAccessorExpression(expr), 19u) << b.error();
+  ASSERT_TRUE(b.Build());
 
-  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
-%3 = OpTypeInt 32 0
-%4 = OpConstant %3 3
-%1 = OpTypeArray %2 %4
-%5 = OpConstant %2 0
-%6 = OpConstant %2 0.5
-%7 = OpConstant %2 1
-%8 = OpConstantComposite %1 %5 %6 %7
-%11 = OpTypeInt 32 1
-%10 = OpTypePointer Function %11
-%12 = OpConstantNull %11
-%13 = OpTypePointer Function %1
-%15 = OpConstantNull %1
-%17 = OpTypePointer Function %2
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid
+%1 = OpTypeFunction %2
+%6 = OpTypeFloat 32
+%7 = OpTypeInt 32 0
+%8 = OpConstant %7 3
+%5 = OpTypeArray %6 %8
+%9 = OpConstant %6 0
+%10 = OpConstant %6 0.5
+%11 = OpConstant %6 1
+%12 = OpConstantComposite %5 %9 %10 %11
+%15 = OpTypeInt 32 1
+%14 = OpTypePointer Function %15
+%16 = OpConstantNull %15
+%18 = OpTypePointer Function %5
+%19 = OpConstantNull %5
+%21 = OpTypePointer Function %6
 )");
   EXPECT_EQ(DumpInstructions(b.functions()[0].variables()),
-            R"(%9 = OpVariable %10 Function %12
-%14 = OpVariable %13 Function %15
+            R"(%13 = OpVariable %14 Function %16
+%17 = OpVariable %18 Function %19
 )");
   EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
-            R"(OpStore %14 %8
-%16 = OpLoad %11 %9
-%18 = OpAccessChain %17 %14 %16
-%19 = OpLoad %2 %18
+            R"(OpStore %17 %12
+%20 = OpLoad %15 %13
+%22 = OpAccessChain %21 %17 %20
+%23 = OpLoad %6 %22
 )");
+
+  Validate(b);
 }
 
 }  // namespace
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 8ea99a1..02b1039 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -305,6 +305,7 @@
     "../src/transform/renamer_test.cc",
     "../src/transform/single_entry_point_test.cc",
     "../src/transform/transform_test.cc",
+    "../src/transform/var_for_dynamic_index_test.cc",
     "../src/transform/vertex_pulling_test.cc",
     "../src/utils/command_test.cc",
     "../src/utils/get_or_create_test.cc",
diff --git a/test/bug/tint/824.wgsl b/test/bug/tint/824.wgsl
new file mode 100644
index 0000000..ed907e3
--- /dev/null
+++ b/test/bug/tint/824.wgsl
@@ -0,0 +1,25 @@
+struct Output {
+    [[builtin(position)]] Position : vec4<f32>;
+    [[location(0)]] color : vec4<f32>;
+};
+[[stage(vertex)]] fn main(
+    [[builtin(vertex_index)]] VertexIndex : u32,
+    [[builtin(instance_index)]] InstanceIndex : u32) -> Output {
+    // TODO: remove workaround for Tint unary array access broke
+    let zv : array<vec2<f32>, 4> = array<vec2<f32>, 4>(
+        vec2<f32>(0.2, 0.2),
+        vec2<f32>(0.3, 0.3),
+        vec2<f32>(-0.1, -0.1),
+        vec2<f32>(1.1, 1.1));
+    let z : f32 = zv[InstanceIndex].x;
+    var output : Output;
+    output.Position = vec4<f32>(0.5, 0.5, z, 1.0);
+    let colors : array<vec4<f32>, 4> = array<vec4<f32>, 4>(
+        vec4<f32>(1.0, 0.0, 0.0, 1.0),
+        vec4<f32>(0.0, 1.0, 0.0, 1.0),
+        vec4<f32>(0.0, 0.0, 1.0, 1.0),
+        vec4<f32>(1.0, 1.0, 1.0, 1.0)
+    );
+    output.color = colors[InstanceIndex];
+    return output;
+}
diff --git a/test/bug/tint/824.wgsl.expected.hlsl b/test/bug/tint/824.wgsl.expected.hlsl
new file mode 100644
index 0000000..241f0e7
--- /dev/null
+++ b/test/bug/tint/824.wgsl.expected.hlsl
@@ -0,0 +1,26 @@
+struct Output {
+  float4 Position;
+  float4 color;
+};
+struct tint_symbol_1 {
+  uint VertexIndex : SV_VertexID;
+  uint InstanceIndex : SV_InstanceID;
+};
+struct tint_symbol_2 {
+  float4 color : TEXCOORD0;
+  float4 Position : SV_Position;
+};
+
+tint_symbol_2 main(tint_symbol_1 tint_symbol) {
+  const uint VertexIndex = tint_symbol.VertexIndex;
+  const uint InstanceIndex = tint_symbol.InstanceIndex;
+  const float2 zv[4] = {float2(0.200000003f, 0.200000003f), float2(0.300000012f, 0.300000012f), float2(-0.100000001f, -0.100000001f), float2(1.100000024f, 1.100000024f)};
+  const float z = zv[InstanceIndex].x;
+  Output output = {float4(0.0f, 0.0f, 0.0f, 0.0f), float4(0.0f, 0.0f, 0.0f, 0.0f)};
+  output.Position = float4(0.5f, 0.5f, z, 1.0f);
+  const float4 colors[4] = {float4(1.0f, 0.0f, 0.0f, 1.0f), float4(0.0f, 1.0f, 0.0f, 1.0f), float4(0.0f, 0.0f, 1.0f, 1.0f), float4(1.0f, 1.0f, 1.0f, 1.0f)};
+  output.color = colors[InstanceIndex];
+  const tint_symbol_2 tint_symbol_3 = {output.color, output.Position};
+  return tint_symbol_3;
+}
+
diff --git a/test/bug/tint/824.wgsl.expected.msl b/test/bug/tint/824.wgsl.expected.msl
new file mode 100644
index 0000000..9f1e787
--- /dev/null
+++ b/test/bug/tint/824.wgsl.expected.msl
@@ -0,0 +1,28 @@
+#include <metal_stdlib>
+
+using namespace metal;
+struct Output {
+  float4 Position;
+  float4 color;
+};
+struct tint_symbol_2 {
+  uint VertexIndex [[vertex_id]];
+  uint InstanceIndex [[instance_id]];
+};
+struct tint_symbol_3 {
+  float4 color [[user(locn0)]];
+  float4 Position [[position]];
+};
+
+vertex tint_symbol_3 tint_symbol(tint_symbol_2 tint_symbol_1 [[stage_in]]) {
+  uint const VertexIndex = tint_symbol_1.VertexIndex;
+  uint const InstanceIndex = tint_symbol_1.InstanceIndex;
+  float2 zv[4] const = {float2(0.200000003f, 0.200000003f), float2(0.300000012f, 0.300000012f), float2(-0.100000001f, -0.100000001f), float2(1.100000024f, 1.100000024f)};
+  float const z = zv[InstanceIndex].x;
+  Output output = {};
+  output.Position = float4(0.5f, 0.5f, z, 1.0f);
+  float4 colors[4] const = {float4(1.0f, 0.0f, 0.0f, 1.0f), float4(0.0f, 1.0f, 0.0f, 1.0f), float4(0.0f, 0.0f, 1.0f, 1.0f), float4(1.0f, 1.0f, 1.0f, 1.0f)};
+  output.color = colors[InstanceIndex];
+  return {output.color, output.Position};
+}
+
diff --git a/test/bug/tint/824.wgsl.expected.spvasm b/test/bug/tint/824.wgsl.expected.spvasm
new file mode 100644
index 0000000..bbdd3fd
--- /dev/null
+++ b/test/bug/tint/824.wgsl.expected.spvasm
@@ -0,0 +1,111 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google Tint Compiler; 0
+; Bound: 70
+; Schema: 0
+               OpCapability Shader
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Vertex %main "main" %tint_pointsize %tint_symbol_1 %tint_symbol_3 %tint_symbol_4
+               OpName %tint_pointsize "tint_pointsize"
+               OpName %tint_symbol "tint_symbol"
+               OpName %tint_symbol_1 "tint_symbol_1"
+               OpName %tint_symbol_3 "tint_symbol_3"
+               OpName %tint_symbol_4 "tint_symbol_4"
+               OpName %Output "Output"
+               OpMemberName %Output 0 "Position"
+               OpMemberName %Output 1 "color"
+               OpName %tint_symbol_5 "tint_symbol_5"
+               OpName %tint_symbol_2 "tint_symbol_2"
+               OpName %main "main"
+               OpName %var_for_array "var_for_array"
+               OpName %output "output"
+               OpName %var_for_array_1 "var_for_array_1"
+               OpDecorate %tint_pointsize BuiltIn PointSize
+               OpDecorate %tint_symbol BuiltIn VertexIndex
+               OpDecorate %tint_symbol_1 BuiltIn InstanceIndex
+               OpDecorate %tint_symbol_3 BuiltIn Position
+               OpDecorate %tint_symbol_4 Location 0
+               OpMemberDecorate %Output 0 Offset 0
+               OpMemberDecorate %Output 1 Offset 16
+               OpDecorate %_arr_v2float_uint_4 ArrayStride 8
+               OpDecorate %_arr_v4float_uint_4 ArrayStride 16
+      %float = OpTypeFloat 32
+%_ptr_Output_float = OpTypePointer Output %float
+          %4 = OpConstantNull %float
+%tint_pointsize = OpVariable %_ptr_Output_float Output %4
+       %uint = OpTypeInt 32 0
+%_ptr_Input_uint = OpTypePointer Input %uint
+%tint_symbol = OpVariable %_ptr_Input_uint Input
+%tint_symbol_1 = OpVariable %_ptr_Input_uint Input
+    %v4float = OpTypeVector %float 4
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+         %12 = OpConstantNull %v4float
+%tint_symbol_3 = OpVariable %_ptr_Output_v4float Output %12
+%tint_symbol_4 = OpVariable %_ptr_Output_v4float Output %12
+       %void = OpTypeVoid
+     %Output = OpTypeStruct %v4float %v4float
+         %14 = OpTypeFunction %void %Output
+         %22 = OpTypeFunction %void
+    %float_1 = OpConstant %float 1
+    %v2float = OpTypeVector %float 2
+     %uint_4 = OpConstant %uint 4
+%_arr_v2float_uint_4 = OpTypeArray %v2float %uint_4
+%float_0_200000003 = OpConstant %float 0.200000003
+         %30 = OpConstantComposite %v2float %float_0_200000003 %float_0_200000003
+%float_0_300000012 = OpConstant %float 0.300000012
+         %32 = OpConstantComposite %v2float %float_0_300000012 %float_0_300000012
+%float_n0_100000001 = OpConstant %float -0.100000001
+         %34 = OpConstantComposite %v2float %float_n0_100000001 %float_n0_100000001
+%float_1_10000002 = OpConstant %float 1.10000002
+         %36 = OpConstantComposite %v2float %float_1_10000002 %float_1_10000002
+         %37 = OpConstantComposite %_arr_v2float_uint_4 %30 %32 %34 %36
+%_ptr_Function__arr_v2float_uint_4 = OpTypePointer Function %_arr_v2float_uint_4
+         %40 = OpConstantNull %_arr_v2float_uint_4
+     %uint_0 = OpConstant %uint 0
+%_ptr_Function_float = OpTypePointer Function %float
+%_ptr_Function_Output = OpTypePointer Function %Output
+         %48 = OpConstantNull %Output
+%_ptr_Function_v4float = OpTypePointer Function %v4float
+  %float_0_5 = OpConstant %float 0.5
+%_arr_v4float_uint_4 = OpTypeArray %v4float %uint_4
+    %float_0 = OpConstant %float 0
+         %55 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
+         %56 = OpConstantComposite %v4float %float_0 %float_1 %float_0 %float_1
+         %57 = OpConstantComposite %v4float %float_0 %float_0 %float_1 %float_1
+         %58 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
+         %59 = OpConstantComposite %_arr_v4float_uint_4 %55 %56 %57 %58
+%_ptr_Function__arr_v4float_uint_4 = OpTypePointer Function %_arr_v4float_uint_4
+         %62 = OpConstantNull %_arr_v4float_uint_4
+     %uint_1 = OpConstant %uint 1
+%tint_symbol_5 = OpFunction %void None %14
+%tint_symbol_2 = OpFunctionParameter %Output
+         %19 = OpLabel
+         %20 = OpCompositeExtract %v4float %tint_symbol_2 0
+               OpStore %tint_symbol_3 %20
+         %21 = OpCompositeExtract %v4float %tint_symbol_2 1
+               OpStore %tint_symbol_4 %21
+               OpReturn
+               OpFunctionEnd
+       %main = OpFunction %void None %22
+         %24 = OpLabel
+%var_for_array = OpVariable %_ptr_Function__arr_v2float_uint_4 Function %40
+     %output = OpVariable %_ptr_Function_Output Function %48
+%var_for_array_1 = OpVariable %_ptr_Function__arr_v4float_uint_4 Function %62
+               OpStore %tint_pointsize %float_1
+               OpStore %var_for_array %37
+         %41 = OpLoad %uint %tint_symbol_1
+         %44 = OpAccessChain %_ptr_Function_float %var_for_array %41 %uint_0
+         %45 = OpLoad %float %44
+         %50 = OpAccessChain %_ptr_Function_v4float %output %uint_0
+         %52 = OpCompositeConstruct %v4float %float_0_5 %float_0_5 %45 %float_1
+               OpStore %50 %52
+               OpStore %var_for_array_1 %59
+         %64 = OpAccessChain %_ptr_Function_v4float %output %uint_1
+         %65 = OpLoad %uint %tint_symbol_1
+         %66 = OpAccessChain %_ptr_Function_v4float %var_for_array_1 %65
+         %67 = OpLoad %v4float %66
+               OpStore %64 %67
+         %69 = OpLoad %Output %output
+         %68 = OpFunctionCall %void %tint_symbol_5 %69
+               OpReturn
+               OpFunctionEnd
diff --git a/test/bug/tint/824.wgsl.expected.wgsl b/test/bug/tint/824.wgsl.expected.wgsl
new file mode 100644
index 0000000..6d051de
--- /dev/null
+++ b/test/bug/tint/824.wgsl.expected.wgsl
@@ -0,0 +1,17 @@
+struct Output {
+  [[builtin(position)]]
+  Position : vec4<f32>;
+  [[location(0)]]
+  color : vec4<f32>;
+};
+
+[[stage(vertex)]]
+fn main([[builtin(vertex_index)]] VertexIndex : u32, [[builtin(instance_index)]] InstanceIndex : u32) -> Output {
+  let zv : array<vec2<f32>, 4> = array<vec2<f32>, 4>(vec2<f32>(0.200000003, 0.200000003), vec2<f32>(0.300000012, 0.300000012), vec2<f32>(-0.100000001, -0.100000001), vec2<f32>(1.100000024, 1.100000024));
+  let z : f32 = zv[InstanceIndex].x;
+  var output : Output;
+  output.Position = vec4<f32>(0.5, 0.5, z, 1.0);
+  let colors : array<vec4<f32>, 4> = array<vec4<f32>, 4>(vec4<f32>(1.0, 0.0, 0.0, 1.0), vec4<f32>(0.0, 1.0, 0.0, 1.0), vec4<f32>(0.0, 0.0, 1.0, 1.0), vec4<f32>(1.0, 1.0, 1.0, 1.0));
+  output.color = colors[InstanceIndex];
+  return output;
+}
diff --git a/test/samples/triangle.wgsl.expected.spvasm b/test/samples/triangle.wgsl.expected.spvasm
index 8a7bf08..c6f874d 100644
--- a/test/samples/triangle.wgsl.expected.spvasm
+++ b/test/samples/triangle.wgsl.expected.spvasm
@@ -16,6 +16,7 @@
                OpName %tint_symbol_3 "tint_symbol_3"
                OpName %tint_symbol_1 "tint_symbol_1"
                OpName %vtx_main "vtx_main"
+               OpName %var_for_array "var_for_array"
                OpName %tint_symbol_6 "tint_symbol_6"
                OpName %tint_symbol_4 "tint_symbol_4"
                OpName %frag_main "frag_main"
@@ -52,7 +53,7 @@
          %29 = OpTypeFunction %void
     %float_1 = OpConstant %float 1
 %_ptr_Function__arr_v2float_uint_3 = OpTypePointer Function %_arr_v2float_uint_3
-         %36 = OpConstantNull %_arr_v2float_uint_3
+         %35 = OpConstantNull %_arr_v2float_uint_3
 %_ptr_Function_v2float = OpTypePointer Function %v2float
          %50 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
 %tint_symbol_3 = OpFunction %void None %24
@@ -63,16 +64,16 @@
                OpFunctionEnd
    %vtx_main = OpFunction %void None %29
          %31 = OpLabel
-         %35 = OpVariable %_ptr_Function__arr_v2float_uint_3 Function %36
+%var_for_array = OpVariable %_ptr_Function__arr_v2float_uint_3 Function %35
                OpStore %tint_pointsize %float_1
-               OpStore %35 %pos
+               OpStore %var_for_array %pos
          %37 = OpLoad %int %tint_symbol
-         %39 = OpAccessChain %_ptr_Function_v2float %35 %37
+         %39 = OpAccessChain %_ptr_Function_v2float %var_for_array %37
          %40 = OpLoad %v2float %39
          %41 = OpCompositeExtract %float %40 0
          %42 = OpCompositeExtract %float %40 1
          %43 = OpCompositeConstruct %v4float %41 %42 %float_0 %float_1
-         %33 = OpFunctionCall %void %tint_symbol_3 %43
+         %36 = OpFunctionCall %void %tint_symbol_3 %43
                OpReturn
                OpFunctionEnd
 %tint_symbol_6 = OpFunction %void None %24