HLSL: work around FXC failures when dynamically indexing matrices

This fixes errors like "error X3500: array reference cannot be used as
an l-value; not natively addressable". Note that FXC treats matrices
like arrays. We still get this error for dynamically indexed arrays in
structs.

Also improved HLSL assign tests, and add missing ones for vector
indexing.

Manually removed 20 e2e skip hlsl SKIP files that are now passing with
this change.

Bug: tint:1333
Change-Id: If23881a667857a4d4ec6881e72666af0a666ef10
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/71982
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>

src/writer/hlsl/generator_impl.cc

@@ -344,6 +344,181 @@ bool GeneratorImpl::EmitDynamicVectorAssignment(
   return true;
 }
 
+bool GeneratorImpl::EmitDynamicMatrixVectorAssignment(
+    const ast::AssignmentStatement* stmt,
+    const sem::Matrix* mat) {
+  auto name = utils::GetOrCreate(
+      dynamic_matrix_vector_write_, mat, [&]() -> std::string {
+        std::string fn;
+        {
+          std::ostringstream ss;
+          if (!EmitType(ss, mat, tint::ast::StorageClass::kInvalid,
+                        ast::Access::kUndefined, "")) {
+            return "";
+          }
+          fn = UniqueIdentifier("set_vector_" + ss.str());
+        }
+        {
+          auto out = line(&helpers_);
+          out << "void " << fn << "(inout ";
+          if (!EmitTypeAndName(out, mat, ast::StorageClass::kInvalid,
+                               ast::Access::kUndefined, "mat")) {
+            return "";
+          }
+          out << ", int col, ";
+          if (!EmitTypeAndName(out, mat->ColumnType(),
+                               ast::StorageClass::kInvalid,
+                               ast::Access::kUndefined, "val")) {
+            return "";
+          }
+          out << ") {";
+        }
+        {
+          ScopedIndent si(&helpers_);
+          line(&helpers_) << "switch (col) {";
+          {
+            ScopedIndent si2(&helpers_);
+            for (uint32_t i = 0; i < mat->columns(); ++i) {
+              line(&helpers_)
+                  << "case " << i << ": mat[" << i << "] = val; break;";
+            }
+          }
+          line(&helpers_) << "}";
+        }
+        line(&helpers_) << "}";
+        line(&helpers_);
+        return fn;
+      });
+
+  if (name.empty()) {
+    return false;
+  }
+
+  auto* ast_access_expr = stmt->lhs->As<ast::IndexAccessorExpression>();
+
+  auto out = line();
+  out << name << "(";
+  if (!EmitExpression(out, ast_access_expr->object)) {
+    return false;
+  }
+  out << ", ";
+  if (!EmitExpression(out, ast_access_expr->index)) {
+    return false;
+  }
+  out << ", ";
+  if (!EmitExpression(out, stmt->rhs)) {
+    return false;
+  }
+  out << ");";
+
+  return true;
+}
+
+bool GeneratorImpl::EmitDynamicMatrixScalarAssignment(
+    const ast::AssignmentStatement* stmt,
+    const sem::Matrix* mat) {
+  auto* lhs_col_access = stmt->lhs->As<ast::IndexAccessorExpression>();
+  auto* lhs_row_access =
+      lhs_col_access->object->As<ast::IndexAccessorExpression>();
+
+  auto name = utils::GetOrCreate(
+      dynamic_matrix_scalar_write_, mat, [&]() -> std::string {
+        std::string fn;
+        {
+          std::ostringstream ss;
+          if (!EmitType(ss, mat, tint::ast::StorageClass::kInvalid,
+                        ast::Access::kUndefined, "")) {
+            return "";
+          }
+          fn = UniqueIdentifier("set_scalar_" + ss.str());
+        }
+        {
+          auto out = line(&helpers_);
+          out << "void " << fn << "(inout ";
+          if (!EmitTypeAndName(out, mat, ast::StorageClass::kInvalid,
+                               ast::Access::kUndefined, "mat")) {
+            return "";
+          }
+          out << ", int col, int row, ";
+          if (!EmitTypeAndName(out, mat->type(), ast::StorageClass::kInvalid,
+                               ast::Access::kUndefined, "val")) {
+            return "";
+          }
+          out << ") {";
+        }
+        {
+          ScopedIndent si(&helpers_);
+          line(&helpers_) << "switch (col) {";
+          {
+            ScopedIndent si2(&helpers_);
+            auto* vec =
+                TypeOf(lhs_row_access->object)->UnwrapRef()->As<sem::Vector>();
+            for (uint32_t i = 0; i < mat->columns(); ++i) {
+              line(&helpers_) << "case " << i << ":";
+              {
+                auto vec_name = "mat[" + std::to_string(i) + "]";
+                ScopedIndent si3(&helpers_);
+                {
+                  auto out = line(&helpers_);
+                  switch (mat->rows()) {
+                    case 2:
+                      out << vec_name
+                          << " = (row.xx == int2(0, 1)) ? val.xx : " << vec_name
+                          << ";";
+                      break;
+                    case 3:
+                      out << vec_name
+                          << " = (row.xxx == int3(0, 1, 2)) ? val.xxx : "
+                          << vec_name << ";";
+                      break;
+                    case 4:
+                      out << vec_name
+                          << " = (row.xxxx == int4(0, 1, 2, 3)) ? val.xxxx : "
+                          << vec_name << ";";
+                      break;
+                    default:
+                      TINT_UNREACHABLE(Writer, builder_.Diagnostics())
+                          << "invalid vector size " << vec->Width();
+                      break;
+                  }
+                }
+                line(&helpers_) << "break;";
+              }
+            }
+          }
+          line(&helpers_) << "}";
+        }
+        line(&helpers_) << "}";
+        line(&helpers_);
+        return fn;
+      });
+
+  if (name.empty()) {
+    return false;
+  }
+
+  auto out = line();
+  out << name << "(";
+  if (!EmitExpression(out, lhs_row_access->object)) {
+    return false;
+  }
+  out << ", ";
+  if (!EmitExpression(out, lhs_col_access->index)) {
+    return false;
+  }
+  out << ", ";
+  if (!EmitExpression(out, lhs_row_access->index)) {
+    return false;
+  }
+  out << ", ";
+  if (!EmitExpression(out, stmt->rhs)) {
+    return false;
+  }
+  out << ");";
+
+  return true;
+}
+
 bool GeneratorImpl::EmitIndexAccessor(
     std::ostream& out,
     const ast::IndexAccessorExpression* expr) {
@@ -387,9 +562,34 @@ bool GeneratorImpl::EmitBitcast(std::ostream& out,
 }
 
 bool GeneratorImpl::EmitAssign(const ast::AssignmentStatement* stmt) {
-  if (auto* idx = stmt->lhs->As<ast::IndexAccessorExpression>()) {
-    if (auto* vec = TypeOf(idx->object)->UnwrapRef()->As<sem::Vector>()) {
-      auto* rhs_sem = builder_.Sem().Get(idx->index);
+  if (auto* lhs_access = stmt->lhs->As<ast::IndexAccessorExpression>()) {
+    // BUG(crbug.com/tint/1333): work around assignment of scalar to matrices
+    // with at least one dynamic index
+    if (auto* lhs_sub_access =
+            lhs_access->object->As<ast::IndexAccessorExpression>()) {
+      if (auto* mat =
+              TypeOf(lhs_sub_access->object)->UnwrapRef()->As<sem::Matrix>()) {
+        auto* rhs_col_idx_sem = builder_.Sem().Get(lhs_access->index);
+        auto* rhs_row_idx_sem = builder_.Sem().Get(lhs_sub_access->index);
+        if (!rhs_col_idx_sem->ConstantValue().IsValid() ||
+            !rhs_row_idx_sem->ConstantValue().IsValid()) {
+          return EmitDynamicMatrixScalarAssignment(stmt, mat);
+        }
+      }
+    }
+    // BUG(crbug.com/tint/1333): work around assignment of vector to matrices
+    // with dynamic indices
+    const auto* lhs_access_type = TypeOf(lhs_access->object)->UnwrapRef();
+    if (auto* mat = lhs_access_type->As<sem::Matrix>()) {
+      auto* lhs_index_sem = builder_.Sem().Get(lhs_access->index);
+      if (!lhs_index_sem->ConstantValue().IsValid()) {
+        return EmitDynamicMatrixVectorAssignment(stmt, mat);
+      }
+    }
+    // BUG(crbug.com/tint/534): work around assignment to vectors with dynamic
+    // indices
+    if (auto* vec = lhs_access_type->As<sem::Vector>()) {
+      auto* rhs_sem = builder_.Sem().Get(lhs_access->index);
       if (!rhs_sem->ConstantValue().IsValid()) {
         return EmitDynamicVectorAssignment(stmt, vec);
       }

src/writer/hlsl/generator_impl.h

@@ -423,6 +423,26 @@ class GeneratorImpl : public TextGenerator {
   /// @returns true on success
   bool EmitDynamicVectorAssignment(const ast::AssignmentStatement* stmt,
                                    const sem::Vector* vec);
+  /// Emits call to a helper matrix assignment function for the input assignment
+  /// statement and matrix type. This is used to work around FXC issues where
+  /// assignment of a vector to a matrix with a dynamic index causes compilation
+  /// failures.
+  /// @param stmt assignment statement that corresponds to a matrix assignment
+  /// via an accessor expression
+  /// @param mat the matrix type being assigned to
+  /// @returns true on success
+  bool EmitDynamicMatrixVectorAssignment(const ast::AssignmentStatement* stmt,
+                                         const sem::Matrix* mat);
+  /// Emits call to a helper matrix assignment function for the input assignment
+  /// statement and matrix type. This is used to work around FXC issues where
+  /// assignment of a scalar to a matrix with at least one dynamic index causes
+  /// compilation failures.
+  /// @param stmt assignment statement that corresponds to a matrix assignment
+  /// via an accessor expression
+  /// @param mat the matrix type being assigned to
+  /// @returns true on success
+  bool EmitDynamicMatrixScalarAssignment(const ast::AssignmentStatement* stmt,
+                                         const sem::Matrix* mat);
 
   /// Handles generating a builtin method name
   /// @param intrinsic the semantic info for the intrinsic
@@ -491,6 +511,10 @@ class GeneratorImpl : public TextGenerator {
   std::unordered_map<const sem::Intrinsic*, std::string> intrinsics_;
   std::unordered_map<const sem::Struct*, std::string> structure_builders_;
   std::unordered_map<const sem::Vector*, std::string> dynamic_vector_write_;
+  std::unordered_map<const sem::Matrix*, std::string>
+      dynamic_matrix_vector_write_;
+  std::unordered_map<const sem::Matrix*, std::string>
+      dynamic_matrix_scalar_write_;
 };
 
 }  // namespace hlsl

src/writer/hlsl/generator_impl_assign_test.cc

@@ -22,17 +22,187 @@ namespace {
 using HlslGeneratorImplTest_Assign = TestHelper;
 
 TEST_F(HlslGeneratorImplTest_Assign, Emit_Assign) {
-  Global("lhs", ty.i32(), ast::StorageClass::kPrivate);
-  Global("rhs", ty.i32(), ast::StorageClass::kPrivate);
-  auto* assign = Assign("lhs", "rhs");
-  WrapInFunction(assign);
+  Func("fn", {}, ty.void_(),
+       {
+           Decl(Var("lhs", ty.i32())),
+           Decl(Var("rhs", ty.i32())),
+           Assign("lhs", "rhs"),
+       });
 
   GeneratorImpl& gen = Build();
 
-  gen.increment_indent();
+  ASSERT_TRUE(gen.Generate());
+  EXPECT_EQ(gen.result(),
+            R"(void fn() {
+  int lhs = 0;
+  int rhs = 0;
+  lhs = rhs;
+}
+)");
+}
 
-  ASSERT_TRUE(gen.EmitStatement(assign)) << gen.error();
-  EXPECT_EQ(gen.result(), "  lhs = rhs;\n");
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Vector_Assign_ConstantIndex) {
+  Func("fn", {}, ty.void_(),
+       {
+           Decl(Var("lhs", ty.vec3<f32>())),
+           Decl(Var("rhs", ty.f32())),
+           Decl(Const("index", ty.u32(), Expr(0u))),
+           Assign(IndexAccessor("lhs", "index"), "rhs"),
+       });
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.Generate());
+  EXPECT_EQ(gen.result(),
+            R"(void fn() {
+  float3 lhs = float3(0.0f, 0.0f, 0.0f);
+  float rhs = 0.0f;
+  const uint index = 0u;
+  lhs[index] = rhs;
+}
+)");
+}
+
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Vector_Assign_DynamicIndex) {
+  Func("fn", {}, ty.void_(),
+       {
+           Decl(Var("lhs", ty.vec3<f32>())),
+           Decl(Var("rhs", ty.f32())),
+           Decl(Var("index", ty.u32())),
+           Assign(IndexAccessor("lhs", "index"), "rhs"),
+       });
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.Generate());
+  EXPECT_EQ(gen.result(),
+            R"(void set_float3(inout float3 vec, int idx, float val) {
+  vec = (idx.xxx == int3(0, 1, 2)) ? val.xxx : vec;
+}
+
+void fn() {
+  float3 lhs = float3(0.0f, 0.0f, 0.0f);
+  float rhs = 0.0f;
+  uint index = 0u;
+  set_float3(lhs, index, rhs);
+}
+)");
+}
+
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Vector_ConstantIndex) {
+  Func("fn", {}, ty.void_(),
+       {
+           Decl(Var("lhs", ty.mat4x2<f32>())),
+           Decl(Var("rhs", ty.vec2<f32>())),
+           Decl(Const("index", ty.u32(), Expr(0u))),
+           Assign(IndexAccessor("lhs", "index"), "rhs"),
+       });
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.Generate());
+  EXPECT_EQ(gen.result(),
+            R"(void fn() {
+  float4x2 lhs = float4x2(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
+  float2 rhs = float2(0.0f, 0.0f);
+  const uint index = 0u;
+  lhs[index] = rhs;
+}
+)");
+}
+
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Vector_DynamicIndex) {
+  Func("fn", {}, ty.void_(),
+       {
+           Decl(Var("lhs", ty.mat4x2<f32>())),
+           Decl(Var("rhs", ty.vec2<f32>())),
+           Decl(Var("index", ty.u32())),
+           Assign(IndexAccessor("lhs", "index"), "rhs"),
+       });
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.Generate());
+  EXPECT_EQ(
+      gen.result(),
+      R"(void set_vector_float4x2(inout float4x2 mat, int col, float2 val) {
+  switch (col) {
+    case 0: mat[0] = val; break;
+    case 1: mat[1] = val; break;
+    case 2: mat[2] = val; break;
+    case 3: mat[3] = val; break;
+  }
+}
+
+void fn() {
+  float4x2 lhs = float4x2(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
+  float2 rhs = float2(0.0f, 0.0f);
+  uint index = 0u;
+  set_vector_float4x2(lhs, index, rhs);
+}
+)");
+}
+
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_ConstantIndex) {
+  Func("fn", {}, ty.void_(),
+       {
+           Decl(Var("lhs", ty.mat4x2<f32>())),
+           Decl(Var("rhs", ty.f32())),
+           Decl(Const("index", ty.u32(), Expr(0u))),
+           Assign(IndexAccessor(IndexAccessor("lhs", "index"), "index"), "rhs"),
+       });
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.Generate());
+  EXPECT_EQ(gen.result(),
+            R"(void fn() {
+  float4x2 lhs = float4x2(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
+  float rhs = 0.0f;
+  const uint index = 0u;
+  lhs[index][index] = rhs;
+}
+)");
+}
+
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_DynamicIndex) {
+  Func("fn", {}, ty.void_(),
+       {
+           Decl(Var("lhs", ty.mat4x2<f32>())),
+           Decl(Var("rhs", ty.f32())),
+           Decl(Var("index", ty.u32())),
+           Assign(IndexAccessor(IndexAccessor("lhs", "index"), "index"), "rhs"),
+       });
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.Generate());
+  EXPECT_EQ(
+      gen.result(),
+      R"(void set_scalar_float4x2(inout float4x2 mat, int col, int row, float val) {
+  switch (col) {
+    case 0:
+      mat[0] = (row.xx == int2(0, 1)) ? val.xx : mat[0];
+      break;
+    case 1:
+      mat[1] = (row.xx == int2(0, 1)) ? val.xx : mat[1];
+      break;
+    case 2:
+      mat[2] = (row.xx == int2(0, 1)) ? val.xx : mat[2];
+      break;
+    case 3:
+      mat[3] = (row.xx == int2(0, 1)) ? val.xx : mat[3];
+      break;
+  }
+}
+
+void fn() {
+  float4x2 lhs = float4x2(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
+  float rhs = 0.0f;
+  uint index = 0u;
+  set_scalar_float4x2(lhs, index, index, rhs);
+}
+)");
 }
 
 }  // namespace