tint/hlsl: fix assignment to matrix element

When calling the generated helper, the column and row arguments were
swapped.

Improved the unit tests to actually show this, rather than passing in a
single value for both column and row.

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

src/tint/writer/hlsl/generator_impl.cc

@@ -512,8 +512,8 @@ bool GeneratorImpl::EmitDynamicMatrixVectorAssignment(const ast::AssignmentState
 
 bool GeneratorImpl::EmitDynamicMatrixScalarAssignment(const ast::AssignmentStatement* stmt,
                                                       const type::Matrix* mat) {
-    auto* lhs_col_access = stmt->lhs->As<ast::IndexAccessorExpression>();
+    auto* lhs_row_access = stmt->lhs->As<ast::IndexAccessorExpression>();
-    auto* lhs_row_access = lhs_col_access->object->As<ast::IndexAccessorExpression>();
+    auto* lhs_col_access = lhs_row_access->object->As<ast::IndexAccessorExpression>();
 
     auto name = utils::GetOrCreate(dynamic_matrix_scalar_write_, mat, [&]() -> std::string {
         std::string fn;
@@ -544,7 +544,6 @@ bool GeneratorImpl::EmitDynamicMatrixScalarAssignment(const ast::AssignmentState
             line(&helpers_) << "switch (col) {";
             {
                 ScopedIndent si2(&helpers_);
-                auto* vec = TypeOf(lhs_row_access->object)->UnwrapRef()->As<type::Vector>();
                 for (uint32_t i = 0; i < mat->columns(); ++i) {
                     line(&helpers_) << "case " << i << ":";
                     {
@@ -568,10 +567,14 @@ bool GeneratorImpl::EmitDynamicMatrixScalarAssignment(const ast::AssignmentState
                                         << " = (row.xxxx == int4(0, 1, 2, 3)) ? val.xxxx : "
                                         << vec_name << ";";
                                     break;
-                                default:
+                                default: {
+                                    auto* vec = TypeOf(lhs_row_access->object)
+                                                    ->UnwrapRef()
+                                                    ->As<type::Vector>();
                                     TINT_UNREACHABLE(Writer, diagnostics_)
                                         << "invalid vector size " << vec->Width();
                                     break;
+                                }
                             }
                         }
                         line(&helpers_) << "break;";
@@ -591,7 +594,7 @@ bool GeneratorImpl::EmitDynamicMatrixScalarAssignment(const ast::AssignmentState
 
     auto out = line();
     out << name << "(";
-    if (!EmitExpression(out, lhs_row_access->object)) {
+    if (!EmitExpression(out, lhs_col_access->object)) {
         return false;
     }
     out << ", ";
@@ -655,9 +658,9 @@ bool GeneratorImpl::EmitAssign(const ast::AssignmentStatement* stmt) {
         // 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<type::Matrix>()) {
-                auto* rhs_col_idx_sem = builder_.Sem().Get(lhs_access->index);
+                auto* rhs_row_idx_sem = builder_.Sem().Get(lhs_access->index);
-                auto* rhs_row_idx_sem = builder_.Sem().Get(lhs_sub_access->index);
+                auto* rhs_col_idx_sem = builder_.Sem().Get(lhs_sub_access->index);
-                if (!rhs_col_idx_sem->ConstantValue() || !rhs_row_idx_sem->ConstantValue()) {
+                if (!rhs_row_idx_sem->ConstantValue() || !rhs_col_idx_sem->ConstantValue()) {
                     return EmitDynamicMatrixScalarAssignment(stmt, mat);
                 }
             }

src/tint/writer/hlsl/generator_impl_assign_test.cc

@@ -197,13 +197,16 @@ void fn() {
 )");
 }
 
-TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_LetIndex) {
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_LetIndices) {
+    auto* col = IndexAccessor("lhs", "col");
+    auto* el = IndexAccessor(col, "row");
     Func("fn", utils::Empty, ty.void_(),
          utils::Vector{
              Decl(Var("lhs", ty.mat4x2<f32>())),
              Decl(Var("rhs", ty.f32())),
-             Decl(Let("index", ty.u32(), Expr(0_u))),
+             Decl(Let("col", ty.u32(), Expr(0_u))),
-             Assign(IndexAccessor(IndexAccessor("lhs", "index"), "index"), "rhs"),
+             Decl(Let("row", ty.u32(), Expr(1_u))),
+             Assign(el, "rhs"),
          });
 
     GeneratorImpl& gen = Build();
@@ -230,19 +233,23 @@ TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_LetIndex) {
 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;
+  const uint col = 0u;
-  set_scalar_float4x2(lhs, index, index, rhs);
+  const uint row = 1u;
+  set_scalar_float4x2(lhs, col, row, rhs);
 }
 )");
 }
 
-TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_ConstIndex) {
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_ConstIndices) {
+    auto* col = IndexAccessor("lhs", "col");
+    auto* el = IndexAccessor(col, "row");
     Func("fn", utils::Empty, ty.void_(),
          utils::Vector{
              Decl(Var("lhs", ty.mat4x2<f32>())),
              Decl(Var("rhs", ty.f32())),
-             Decl(Const("index", ty.u32(), Expr(0_u))),
+             Decl(Const("col", ty.u32(), Expr(0_u))),
-             Assign(IndexAccessor(IndexAccessor("lhs", "index"), "index"), "rhs"),
+             Decl(Const("row", ty.u32(), Expr(1_u))),
+             Assign(el, "rhs"),
          });
 
     GeneratorImpl& gen = Build();
@@ -252,18 +259,21 @@ TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_ConstIndex) {
               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;
-  lhs[0u][0u] = rhs;
+  lhs[0u][1u] = rhs;
 }
 )");
 }
 
-TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_DynamicIndex) {
+TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_DynamicIndices) {
+    auto* col = IndexAccessor("lhs", "col");
+    auto* el = IndexAccessor(col, "row");
     Func("fn", utils::Empty, ty.void_(),
          utils::Vector{
              Decl(Var("lhs", ty.mat4x2<f32>())),
              Decl(Var("rhs", ty.f32())),
-             Decl(Var("index", ty.u32())),
+             Decl(Var("col", ty.u32())),
-             Assign(IndexAccessor(IndexAccessor("lhs", "index"), "index"), "rhs"),
+             Decl(Var("row", ty.u32())),
+             Assign(el, "rhs"),
          });
 
     GeneratorImpl& gen = Build();
@@ -290,8 +300,9 @@ TEST_F(HlslGeneratorImplTest_Assign, Emit_Matrix_Assign_Scalar_DynamicIndex) {
 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;
+  uint col = 0u;
-  set_scalar_float4x2(lhs, index, index, rhs);
+  uint row = 0u;
+  set_scalar_float4x2(lhs, col, row, rhs);
 }
 )");
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/local_assign_scalar_x.wgsl.expected.dxc.hlsl

@@ -16,6 +16,6 @@ cbuffer cbuffer_uniforms : register(b4, space1) {
 [numthreads(1, 1, 1)]
 void main() {
   float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
-  set_scalar_float2x4(m1, 0, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, 0, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/local_assign_scalar_x.wgsl.expected.fxc.hlsl

@@ -16,6 +16,6 @@ cbuffer cbuffer_uniforms : register(b4, space1) {
 [numthreads(1, 1, 1)]
 void main() {
   float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
-  set_scalar_float2x4(m1, 0, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, 0, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/local_assign_scalar_xy.wgsl.expected.dxc.hlsl

@@ -16,6 +16,6 @@ cbuffer cbuffer_uniforms : register(b4, space1) {
 [numthreads(1, 1, 1)]
 void main() {
   float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
-  set_scalar_float2x4(m1, uniforms[0].y, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, uniforms[0].y, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/local_assign_scalar_xy.wgsl.expected.fxc.hlsl

@@ -16,6 +16,6 @@ cbuffer cbuffer_uniforms : register(b4, space1) {
 [numthreads(1, 1, 1)]
 void main() {
   float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
-  set_scalar_float2x4(m1, uniforms[0].y, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, uniforms[0].y, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/local_assign_scalar_y.wgsl.expected.dxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, uniforms[0].y, 0, 1.0f);
+  set_scalar_float2x4(m1, 0, uniforms[0].y, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/local_assign_scalar_y.wgsl.expected.fxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, uniforms[0].y, 0, 1.0f);
+  set_scalar_float2x4(m1, 0, uniforms[0].y, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/module_assign_scalar_x.wgsl.expected.dxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, 0, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, 0, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/module_assign_scalar_x.wgsl.expected.fxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, 0, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, 0, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/module_assign_scalar_xy.wgsl.expected.dxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, uniforms[0].y, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, uniforms[0].y, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/module_assign_scalar_xy.wgsl.expected.fxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, uniforms[0].y, uniforms[0].x, 1.0f);
+  set_scalar_float2x4(m1, uniforms[0].x, uniforms[0].y, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/module_assign_scalar_y.wgsl.expected.dxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, uniforms[0].y, 0, 1.0f);
+  set_scalar_float2x4(m1, 0, uniforms[0].y, 1.0f);
   return;
 }

test/tint/bug/fxc/matrix_assignment_dynamic_index/module_assign_scalar_y.wgsl.expected.fxc.hlsl

@@ -16,6 +16,6 @@ static float2x4 m1 = float2x4(0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f);
 
 [numthreads(1, 1, 1)]
 void main() {
-  set_scalar_float2x4(m1, uniforms[0].y, 0, 1.0f);
+  set_scalar_float2x4(m1, 0, uniforms[0].y, 1.0f);
   return;
 }