transform::Hlsl: Hoist array initializers out of expressions

Move these into a separate const variable declaration statement just above the before the use of the array initializer.
HLSL does not allow array initializers as part of a sub-expression

Fixed: tint:406
Change-Id: I98f93f2eca172865691831011c852de5e57c5ad6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41484
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: James Price <jrprice@google.com>

src/transform/hlsl.cc

@@ -1,4 +1,4 @@
-// Copyright 2020 The Tint Authors.
+// 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.
@@ -16,7 +16,11 @@
 
 #include <utility>
 
+#include "src/ast/variable_decl_statement.h"
 #include "src/program_builder.h"
+#include "src/semantic/expression.h"
+#include "src/semantic/statement.h"
+#include "src/type/array_type.h"
 
 namespace tint {
 namespace transform {
@@ -26,9 +30,76 @@ Hlsl::~Hlsl() = default;
 
 Transform::Output Hlsl::Run(const Program* in) {
   ProgramBuilder out;
-  CloneContext(&out, in).Clone();
+  CloneContext ctx(&out, in);
+  PromoteArrayInitializerToConstVar(ctx);
+  ctx.Clone();
   return Output{Program(std::move(out))};
 }
 
+void Hlsl::PromoteArrayInitializerToConstVar(CloneContext& ctx) const {
+  // Scan the AST nodes for array initializers which need to be promoted to
+  // their own constant declaration.
+
+  // Note: Correct handling of arrays-of-arrays is guaranteed due to the
+  // depth-first traversal of the ast::Node::Clone() methods:
+  //
+  // The inner-most array initializers are traversed first, and they are hoisted
+  // to const variables declared just above the statement of use. The outer
+  // array initializer will then be hoisted, inserting themselves between the
+  // inner array declaration and the statement of use. This pattern applies
+  // correctly to any nested depth.
+  //
+  // Depth-first traversal of the AST is guaranteed because AST nodes are fully
+  // immutable and require their children to be constructed first so their
+  // pointer can be passed to the parent's constructor.
+
+  for (auto* src_node : ctx.src->ASTNodes().Objects()) {
+    if (auto* src_init = src_node->As<ast::TypeConstructorExpression>()) {
+      if (auto* src_sem_expr = ctx.src->Sem().Get(src_init)) {
+        auto* src_sem_stmt = src_sem_expr->Stmt();
+        if (!src_sem_stmt) {
+          // Expression is outside of a statement. This usually means the
+          // expression is part of a global (module-scope) constant declaration.
+          // These must be constexpr, and so cannot contain the type of
+          // expressions that must be sanitized.
+          continue;
+        }
+        auto* src_stmt = src_sem_stmt->Declaration();
+
+        if (auto* src_var_decl = src_stmt->As<ast::VariableDeclStatement>()) {
+          if (src_var_decl->variable()->constructor() == src_init) {
+            // This statement is just a variable declaration with the array
+            // initializer as the constructor value. This is what we're
+            // attempting to transform to, and so ignore.
+            continue;
+          }
+        }
+
+        if (auto* src_array_ty = src_sem_expr->Type()->As<type::Array>()) {
+          // Create a new symbol for the constant
+          auto dst_symbol = ctx.dst->Symbols().New();
+          // Clone the array type
+          auto* dst_array_ty = ctx.Clone(src_array_ty);
+          // Clone the array initializer
+          auto* dst_init = ctx.Clone(src_init);
+          // Construct the constant that holds the array
+          auto* dst_var = ctx.dst->Const(
+              dst_symbol, ast::StorageClass::kFunction, dst_array_ty, dst_init);
+          // Construct the variable declaration statement
+          auto* dst_var_decl =
+              ctx.dst->create<ast::VariableDeclStatement>(dst_var);
+          // Construct the identifier for referencing the constant
+          auto* dst_ident = ctx.dst->Expr(dst_symbol);
+
+          // Insert the constant before the usage
+          ctx.InsertBefore(src_stmt, dst_var_decl);
+          // Replace the inlined array with a reference to the constant
+          ctx.Replace(src_init, dst_ident);
+        }
+      }
+    }
+  }
+}
+
 }  // namespace transform
 }  // namespace tint

src/transform/hlsl.h

@@ -18,6 +18,10 @@
 #include "src/transform/transform.h"
 
 namespace tint {
+
+// Forward declarations
+class CloneContext;
+
 namespace transform {
 
 /// Hlsl is a transform used to sanitize a Program for use with the Hlsl writer.
@@ -36,6 +40,12 @@ class Hlsl : public Transform {
   /// @param program the source program to transform
   /// @returns the transformation result
   Output Run(const Program* program) override;
+
+ private:
+  /// Hoists the array initializer to a constant variable, declared just before
+  /// the array usage statement.
+  /// See crbug.com/tint/406 for more details
+  void PromoteArrayInitializerToConstVar(CloneContext& ctx) const;
 };
 
 }  // namespace transform

src/transform/hlsl_test.cc

@@ -0,0 +1,154 @@
+// 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/hlsl.h"
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "src/transform/test_helper.h"
+
+namespace tint {
+namespace transform {
+namespace {
+
+using HlslTest = TransformTest;
+
+TEST_F(HlslTest, PromoteArrayInitializerToConstVar_Basic) {
+  auto* src = R"(
+[[stage(vertex)]]
+fn main() -> void {
+  var f0 : f32 = 1.0;
+  var f1 : f32 = 2.0;
+  var f2 : f32 = 3.0;
+  var f3 : f32 = 4.0;
+  var i : i32 = array<f32, 4>(f0, f1, f2, f3)[2];
+}
+)";
+
+  auto* expect = R"(
+[[stage(vertex)]]
+fn main() -> void {
+  var f0 : f32 = 1.0;
+  var f1 : f32 = 2.0;
+  var f2 : f32 = 3.0;
+  var f3 : f32 = 4.0;
+  const tint_symbol_1 : array<f32, 4> = array<f32, 4>(f0, f1, f2, f3);
+  var i : i32 = tint_symbol_1[2];
+}
+)";
+
+  auto got = Transform<Hlsl>(src);
+
+  EXPECT_EQ(expect, got);
+}
+
+TEST_F(HlslTest, PromoteArrayInitializerToConstVar_ArrayInArray) {
+  auto* src = R"(
+[[stage(vertex)]]
+fn main() -> void {
+  var i : i32 = array<array<f32, 2>, 2>(array<f32, 2>(1.0, 2.0), array<f32, 2>(3.0, 4.0))[0][1];
+}
+)";
+
+  auto* expect = R"(
+[[stage(vertex)]]
+fn main() -> void {
+  const tint_symbol_1 : array<f32, 2> = array<f32, 2>(1.0, 2.0);
+  const tint_symbol_2 : array<f32, 2> = array<f32, 2>(3.0, 4.0);
+  const tint_symbol_3 : array<array<f32, 2>, 2> = array<array<f32, 2>, 2>(tint_symbol_1, tint_symbol_2);
+  var i : i32 = tint_symbol_3[0][1];
+}
+)";
+
+  auto got = Transform<Hlsl>(src);
+
+  EXPECT_EQ(expect, got);
+}
+
+TEST_F(HlslTest, PromoteArrayInitializerToConstVar_NoChangeOnArrayVarDecl) {
+  auto* src = R"(
+[[stage(vertex)]]
+fn main() -> void {
+  var local_arr : array<f32, 4> = array<f32, 4>(0.0, 1.0, 2.0, 3.0);
+}
+
+const module_arr : array<f32, 4> = array<f32, 4>(0.0, 1.0, 2.0, 3.0);
+)";
+
+  auto* expect = src;
+
+  auto got = Transform<Hlsl>(src);
+
+  EXPECT_EQ(expect, got);
+}
+
+TEST_F(HlslTest, PromoteArrayInitializerToConstVar_Bug406) {
+  // See crbug.com/tint/406
+  auto* src = R"(
+[[block]]
+struct Uniforms {
+  [[offset(0)]]
+  transform : mat2x2<f32>;
+};
+
+[[group(0), binding(0)]] var<uniform> ubo : Uniforms;
+
+[[builtin(vertex_index)]] var<in> vertex_index : u32;
+
+[[builtin(position)]] var<out> position : vec4<f32>;
+
+[[stage(vertex)]]
+fn main() -> void {
+  const transform : mat2x2<f32> = ubo.transform;
+  var coord : array<vec2<f32>, 3> = array<vec2<f32>, 3>(
+      vec2<f32>(-1.0,  1.0),
+      vec2<f32>( 1.0,  1.0),
+      vec2<f32>(-1.0, -1.0)
+  )[vertex_index];
+  position = vec4<f32>(transform * coord, 0.0, 1.0);
+}
+)";
+
+  auto* expect = R"(
+[[block]]
+struct Uniforms {
+  [[offset(0)]]
+  transform : mat2x2<f32>;
+};
+
+[[group(0), binding(0)]] var<uniform> ubo : Uniforms;
+
+[[builtin(vertex_index)]] var<in> vertex_index : u32;
+
+[[builtin(position)]] var<out> position : vec4<f32>;
+
+[[stage(vertex)]]
+fn main() -> void {
+  const transform : mat2x2<f32> = ubo.transform;
+  const tint_symbol_1 : array<vec2<f32>, 3> = array<vec2<f32>, 3>(vec2<f32>(-1.0, 1.0), vec2<f32>(1.0, 1.0), vec2<f32>(-1.0, -1.0));
+  var coord : array<vec2<f32>, 3> = tint_symbol_1[vertex_index];
+  position = vec4<f32>((transform * coord), 0.0, 1.0);
+}
+)";
+
+  auto got = Transform<Hlsl>(src);
+
+  EXPECT_EQ(expect, got);
+}
+
+}  // namespace
+}  // namespace transform
+}  // namespace tint