[glsl][ir] Fix emission of array elements

The code was incorrectly using the deepest element of the array, instead of the first non-array element. This Cl fixes the iteration to store the indexing away and calculate the first non-array element. Bug: 42251044 Change-Id: I33f0d21ea6d6f01897562c96b92be82a740b10bb Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/204834 Commit-Queue: dan sinclair <[email protected]> Reviewed-by: James Price <[email protected]>
google · Sep 3, 2024 · 6b1e61b · 6b1e61b
1 parent 49597cc
commit 6b1e61b
Show file tree

Hide file tree

Showing 18 changed files with 316 additions and 56 deletions.
diff --git a/src/tint/lang/glsl/writer/access_test.cc b/src/tint/lang/glsl/writer/access_test.cc
@@ -1510,7 +1510,7 @@ TEST_F(GlslWriterTest, AccessStoreStructNested) {
     EXPECT_EQ(output_.glsl, GlslHeader() + R"(
 struct Inner {
   mat3 s;
-  float t[5];
+  vec3 t[5];
 };
 
 struct Outer {

diff --git a/src/tint/lang/glsl/writer/printer/printer.cc b/src/tint/lang/glsl/writer/printer/printer.cc
@@ -587,26 +587,28 @@ class Printer : public tint::TextGenerator {
                        const core::type::Array* ary,
                        const std::string& name,
                        bool* name_printed) {
-        EmitType(out, ary->DeepestElement());
-        if (!name.empty()) {
-            out << " " << name;
-            if (name_printed) {
-                *name_printed = true;
-            }
-        }
-
+        std::stringstream args;
         const core::type::Type* ty = ary;
         while (auto* arr = ty->As<core::type::Array>()) {
             if (arr->Count()->Is<core::type::RuntimeArrayCount>()) {
-                out << "[]";
+                args << "[]";
             } else {
                 auto count = arr->ConstantCount();
                 TINT_ASSERT(count.has_value());
 
-                out << "[" << count.value() << "]";
+                args << "[" << count.value() << "]";
             }
             ty = arr->ElemType();
         }
+
+        EmitType(out, ty);
+        if (!name.empty()) {
+            out << " " << name;
+            if (name_printed) {
+                *name_printed = true;
+            }
+        }
+        out << args.str();
     }
 
     void EmitTextureType(StringStream& out, const core::type::Texture* t) {

diff --git a/src/tint/lang/glsl/writer/type_test.cc b/src/tint/lang/glsl/writer/type_test.cc
@@ -96,6 +96,31 @@ void main() {
 )");
 }
 
+TEST_F(GlslWriterTest, EmitType_StructArrayVec) {
+    auto* Inner =
+        ty.Struct(mod.symbols.New("Inner"), {
+                                                {mod.symbols.New("t"), ty.array<vec3<f32>, 5>()},
+                                            });
+    auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kCompute);
+    func->SetWorkgroupSize(1, 1, 1);
+    b.Append(func->Block(), [&] {
+        b.Var("a", ty.ptr(core::AddressSpace::kPrivate, Inner));
+        b.Return(func);
+    });
+
+    ASSERT_TRUE(Generate()) << err_ << output_.glsl;
+    EXPECT_EQ(output_.glsl, GlslHeader() + R"(
+struct Inner {
+  vec3 t[5];
+};
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void main() {
+  Inner a = Inner(vec3[5](vec3(0.0f), vec3(0.0f), vec3(0.0f), vec3(0.0f), vec3(0.0f)));
+}
+)");
+}
+
 TEST_F(GlslWriterTest, EmitType_Bool) {
     auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kCompute);
     func->SetWorkgroupSize(1, 1, 1);

diff --git a/test/tint/buffer/storage/static_index/write.wgsl.expected.ir.glsl b/test/tint/buffer/storage/static_index/write.wgsl.expected.ir.glsl
@@ -1,11 +1,94 @@
-SKIP: FAILED
+#version 310 es
 
-<dawn>/src/tint/lang/glsl/writer/printer/printer.cc:252 internal compiler error: Switch() matched no cases. Type: tint::core::ir::Store
-********************************************************************
-*  The tint shader compiler has encountered an unexpected error.   *
-*                                                                  *
-*  Please help us fix this issue by submitting a bug report at     *
-*  crbug.com/tint with the source program that triggered the bug.  *
-********************************************************************
+struct Inner {
+  int scalar_i32;
+  float scalar_f32;
+};
 
-tint executable returned error: signal: illegal instruction
+struct S {
+  float scalar_f32;
+  int scalar_i32;
+  uint scalar_u32;
+  vec2 vec2_f32;
+  ivec2 vec2_i32;
+  uvec2 vec2_u32;
+  vec3 vec3_f32;
+  ivec3 vec3_i32;
+  uvec3 vec3_u32;
+  vec4 vec4_f32;
+  ivec4 vec4_i32;
+  uvec4 vec4_u32;
+  mat2 mat2x2_f32;
+  mat2x3 mat2x3_f32;
+  mat2x4 mat2x4_f32;
+  mat3x2 mat3x2_f32;
+  mat3 mat3x3_f32;
+  mat3x4 mat3x4_f32;
+  mat4x2 mat4x2_f32;
+  mat4x3 mat4x3_f32;
+  mat4 mat4x4_f32;
+  vec3 arr2_vec3_f32[2];
+  Inner struct_inner;
+  Inner array_struct_inner[4];
+};
+
+S sb;
+void tint_store_and_preserve_padding_3(inout vec3 target[2], vec3 value_param[2]) {
+  {
+    uint v = 0u;
+    v = 0u;
+    while(true) {
+      uint v_1 = v;
+      if ((v_1 >= 2u)) {
+        break;
+      }
+      target[v_1] = value_param[v_1];
+      {
+        v = (v_1 + 1u);
+      }
+      continue;
+    }
+  }
+}
+void tint_store_and_preserve_padding_2(inout mat4x3 target, mat4x3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+  target[2u] = value_param[2u];
+  target[3u] = value_param[3u];
+}
+void tint_store_and_preserve_padding_1(inout mat3 target, mat3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+  target[2u] = value_param[2u];
+}
+void tint_store_and_preserve_padding(inout mat2x3 target, mat2x3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+}
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void main() {
+  sb.scalar_f32 = 0.0f;
+  sb.scalar_i32 = 0;
+  sb.scalar_u32 = 0u;
+  sb.vec2_f32 = vec2(0.0f);
+  sb.vec2_i32 = ivec2(0);
+  sb.vec2_u32 = uvec2(0u);
+  sb.vec3_f32 = vec3(0.0f);
+  sb.vec3_i32 = ivec3(0);
+  sb.vec3_u32 = uvec3(0u);
+  sb.vec4_f32 = vec4(0.0f);
+  sb.vec4_i32 = ivec4(0);
+  sb.vec4_u32 = uvec4(0u);
+  sb.mat2x2_f32 = mat2(vec2(0.0f), vec2(0.0f));
+  tint_store_and_preserve_padding(sb.mat2x3_f32, mat2x3(vec3(0.0f), vec3(0.0f)));
+  sb.mat2x4_f32 = mat2x4(vec4(0.0f), vec4(0.0f));
+  sb.mat3x2_f32 = mat3x2(vec2(0.0f), vec2(0.0f), vec2(0.0f));
+  tint_store_and_preserve_padding_1(sb.mat3x3_f32, mat3(vec3(0.0f), vec3(0.0f), vec3(0.0f)));
+  sb.mat3x4_f32 = mat3x4(vec4(0.0f), vec4(0.0f), vec4(0.0f));
+  sb.mat4x2_f32 = mat4x2(vec2(0.0f), vec2(0.0f), vec2(0.0f), vec2(0.0f));
+  tint_store_and_preserve_padding_2(sb.mat4x3_f32, mat4x3(vec3(0.0f), vec3(0.0f), vec3(0.0f), vec3(0.0f)));
+  sb.mat4x4_f32 = mat4(vec4(0.0f), vec4(0.0f), vec4(0.0f), vec4(0.0f));
+  tint_store_and_preserve_padding_3(sb.arr2_vec3_f32, vec3[2](vec3(0.0f), vec3(0.0f)));
+  sb.struct_inner = Inner(0, 0.0f);
+  sb.array_struct_inner = Inner[4](Inner(0, 0.0f), Inner(0, 0.0f), Inner(0, 0.0f), Inner(0, 0.0f));
+}
diff --git a/test/tint/buffer/storage/static_index/write_f16.wgsl.expected.ir.glsl b/test/tint/buffer/storage/static_index/write_f16.wgsl.expected.ir.glsl
@@ -1,11 +1,161 @@
-SKIP: FAILED
+#version 310 es
+#extension GL_AMD_gpu_shader_half_float: require
 
-<dawn>/src/tint/lang/glsl/writer/printer/printer.cc:252 internal compiler error: Switch() matched no cases. Type: tint::core::ir::Store
-********************************************************************
-*  The tint shader compiler has encountered an unexpected error.   *
-*                                                                  *
-*  Please help us fix this issue by submitting a bug report at     *
-*  crbug.com/tint with the source program that triggered the bug.  *
-********************************************************************
+struct Inner {
+  int scalar_i32;
+  float scalar_f32;
+  float16_t scalar_f16;
+};
 
-tint executable returned error: signal: illegal instruction
+struct S {
+  float scalar_f32;
+  int scalar_i32;
+  uint scalar_u32;
+  float16_t scalar_f16;
+  vec2 vec2_f32;
+  ivec2 vec2_i32;
+  uvec2 vec2_u32;
+  f16vec2 vec2_f16;
+  vec3 vec3_f32;
+  ivec3 vec3_i32;
+  uvec3 vec3_u32;
+  f16vec3 vec3_f16;
+  vec4 vec4_f32;
+  ivec4 vec4_i32;
+  uvec4 vec4_u32;
+  f16vec4 vec4_f16;
+  mat2 mat2x2_f32;
+  mat2x3 mat2x3_f32;
+  mat2x4 mat2x4_f32;
+  mat3x2 mat3x2_f32;
+  mat3 mat3x3_f32;
+  mat3x4 mat3x4_f32;
+  mat4x2 mat4x2_f32;
+  mat4x3 mat4x3_f32;
+  mat4 mat4x4_f32;
+  f16mat2 mat2x2_f16;
+  f16mat2x3 mat2x3_f16;
+  f16mat2x4 mat2x4_f16;
+  f16mat3x2 mat3x2_f16;
+  f16mat3 mat3x3_f16;
+  f16mat3x4 mat3x4_f16;
+  f16mat4x2 mat4x2_f16;
+  f16mat4x3 mat4x3_f16;
+  f16mat4 mat4x4_f16;
+  vec3 arr2_vec3_f32[2];
+  f16mat4x2 arr2_mat4x2_f16[2];
+  Inner struct_inner;
+  Inner array_struct_inner[4];
+};
+
+S sb;
+void tint_store_and_preserve_padding_7(inout Inner target, Inner value_param) {
+  target.scalar_i32 = value_param.scalar_i32;
+  target.scalar_f32 = value_param.scalar_f32;
+  target.scalar_f16 = value_param.scalar_f16;
+}
+void tint_store_and_preserve_padding_8(inout Inner target[4], Inner value_param[4]) {
+  {
+    uint v = 0u;
+    v = 0u;
+    while(true) {
+      uint v_1 = v;
+      if ((v_1 >= 4u)) {
+        break;
+      }
+      tint_store_and_preserve_padding_7(target[v_1], value_param[v_1]);
+      {
+        v = (v_1 + 1u);
+      }
+      continue;
+    }
+  }
+}
+void tint_store_and_preserve_padding_6(inout vec3 target[2], vec3 value_param[2]) {
+  {
+    uint v_2 = 0u;
+    v_2 = 0u;
+    while(true) {
+      uint v_3 = v_2;
+      if ((v_3 >= 2u)) {
+        break;
+      }
+      target[v_3] = value_param[v_3];
+      {
+        v_2 = (v_3 + 1u);
+      }
+      continue;
+    }
+  }
+}
+void tint_store_and_preserve_padding_5(inout f16mat4x3 target, f16mat4x3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+  target[2u] = value_param[2u];
+  target[3u] = value_param[3u];
+}
+void tint_store_and_preserve_padding_4(inout f16mat3 target, f16mat3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+  target[2u] = value_param[2u];
+}
+void tint_store_and_preserve_padding_3(inout f16mat2x3 target, f16mat2x3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+}
+void tint_store_and_preserve_padding_2(inout mat4x3 target, mat4x3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+  target[2u] = value_param[2u];
+  target[3u] = value_param[3u];
+}
+void tint_store_and_preserve_padding_1(inout mat3 target, mat3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+  target[2u] = value_param[2u];
+}
+void tint_store_and_preserve_padding(inout mat2x3 target, mat2x3 value_param) {
+  target[0u] = value_param[0u];
+  target[1u] = value_param[1u];
+}
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void main() {
+  sb.scalar_f32 = 0.0f;
+  sb.scalar_i32 = 0;
+  sb.scalar_u32 = 0u;
+  sb.scalar_f16 = 0.0hf;
+  sb.vec2_f32 = vec2(0.0f);
+  sb.vec2_i32 = ivec2(0);
+  sb.vec2_u32 = uvec2(0u);
+  sb.vec2_f16 = f16vec2(0.0hf);
+  sb.vec3_f32 = vec3(0.0f);
+  sb.vec3_i32 = ivec3(0);
+  sb.vec3_u32 = uvec3(0u);
+  sb.vec3_f16 = f16vec3(0.0hf);
+  sb.vec4_f32 = vec4(0.0f);
+  sb.vec4_i32 = ivec4(0);
+  sb.vec4_u32 = uvec4(0u);
+  sb.vec4_f16 = f16vec4(0.0hf);
+  sb.mat2x2_f32 = mat2(vec2(0.0f), vec2(0.0f));
+  tint_store_and_preserve_padding(sb.mat2x3_f32, mat2x3(vec3(0.0f), vec3(0.0f)));
+  sb.mat2x4_f32 = mat2x4(vec4(0.0f), vec4(0.0f));
+  sb.mat3x2_f32 = mat3x2(vec2(0.0f), vec2(0.0f), vec2(0.0f));
+  tint_store_and_preserve_padding_1(sb.mat3x3_f32, mat3(vec3(0.0f), vec3(0.0f), vec3(0.0f)));
+  sb.mat3x4_f32 = mat3x4(vec4(0.0f), vec4(0.0f), vec4(0.0f));
+  sb.mat4x2_f32 = mat4x2(vec2(0.0f), vec2(0.0f), vec2(0.0f), vec2(0.0f));
+  tint_store_and_preserve_padding_2(sb.mat4x3_f32, mat4x3(vec3(0.0f), vec3(0.0f), vec3(0.0f), vec3(0.0f)));
+  sb.mat4x4_f32 = mat4(vec4(0.0f), vec4(0.0f), vec4(0.0f), vec4(0.0f));
+  sb.mat2x2_f16 = f16mat2(f16vec2(0.0hf), f16vec2(0.0hf));
+  tint_store_and_preserve_padding_3(sb.mat2x3_f16, f16mat2x3(f16vec3(0.0hf), f16vec3(0.0hf)));
+  sb.mat2x4_f16 = f16mat2x4(f16vec4(0.0hf), f16vec4(0.0hf));
+  sb.mat3x2_f16 = f16mat3x2(f16vec2(0.0hf), f16vec2(0.0hf), f16vec2(0.0hf));
+  tint_store_and_preserve_padding_4(sb.mat3x3_f16, f16mat3(f16vec3(0.0hf), f16vec3(0.0hf), f16vec3(0.0hf)));
+  sb.mat3x4_f16 = f16mat3x4(f16vec4(0.0hf), f16vec4(0.0hf), f16vec4(0.0hf));
+  sb.mat4x2_f16 = f16mat4x2(f16vec2(0.0hf), f16vec2(0.0hf), f16vec2(0.0hf), f16vec2(0.0hf));
+  tint_store_and_preserve_padding_5(sb.mat4x3_f16, f16mat4x3(f16vec3(0.0hf), f16vec3(0.0hf), f16vec3(0.0hf), f16vec3(0.0hf)));
+  sb.mat4x4_f16 = f16mat4(f16vec4(0.0hf), f16vec4(0.0hf), f16vec4(0.0hf), f16vec4(0.0hf));
+  tint_store_and_preserve_padding_6(sb.arr2_vec3_f32, vec3[2](vec3(0.0f), vec3(0.0f)));
+  sb.arr2_mat4x2_f16 = f16mat4x2[2](f16mat4x2(f16vec2(0.0hf), f16vec2(0.0hf), f16vec2(0.0hf), f16vec2(0.0hf)), f16mat4x2(f16vec2(0.0hf), f16vec2(0.0hf), f16vec2(0.0hf), f16vec2(0.0hf)));
+  tint_store_and_preserve_padding_7(sb.struct_inner, Inner(0, 0.0f, 0.0hf));
+  tint_store_and_preserve_padding_8(sb.array_struct_inner, Inner[4](Inner(0, 0.0f, 0.0hf), Inner(0, 0.0f, 0.0hf), Inner(0, 0.0f, 0.0hf), Inner(0, 0.0f, 0.0hf)));
+}
diff --git a/test/tint/expressions/type_ctor/array/explicit/mat2x2/abstract.wgsl.expected.ir.glsl b/test/tint/expressions/type_ctor/array/explicit/mat2x2/abstract.wgsl.expected.ir.glsl
@@ -1,8 +1,8 @@
 #version 310 es
 
-float arr[2] = float[2](mat2(vec2(1.0f, 2.0f), vec2(3.0f, 4.0f)), mat2(vec2(5.0f, 6.0f), vec2(7.0f, 8.0f)));
+mat2 arr[2] = mat2[2](mat2(vec2(1.0f, 2.0f), vec2(3.0f, 4.0f)), mat2(vec2(5.0f, 6.0f), vec2(7.0f, 8.0f)));
 void f() {
-  float v[2] = arr;
+  mat2 v[2] = arr;
 }
 layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
 void main() {

diff --git a/test/tint/expressions/type_ctor/array/explicit/mat2x2/f32.wgsl.expected.ir.glsl b/test/tint/expressions/type_ctor/array/explicit/mat2x2/f32.wgsl.expected.ir.glsl
@@ -1,8 +1,8 @@
 #version 310 es
 
-float arr[2] = float[2](mat2(vec2(1.0f, 2.0f), vec2(3.0f, 4.0f)), mat2(vec2(5.0f, 6.0f), vec2(7.0f, 8.0f)));
+mat2 arr[2] = mat2[2](mat2(vec2(1.0f, 2.0f), vec2(3.0f, 4.0f)), mat2(vec2(5.0f, 6.0f), vec2(7.0f, 8.0f)));
 void f() {
-  float v[2] = arr;
+  mat2 v[2] = arr;
 }
 layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
 void main() {

diff --git a/test/tint/expressions/type_ctor/array/explicit/vec2/abstract.wgsl.expected.ir.glsl b/test/tint/expressions/type_ctor/array/explicit/vec2/abstract.wgsl.expected.ir.glsl
@@ -1,8 +1,8 @@
 #version 310 es
 
-float arr[2] = float[2](vec2(1.0f), vec2(2.0f));
+vec2 arr[2] = vec2[2](vec2(1.0f), vec2(2.0f));
 void f() {
-  float v[2] = arr;
+  vec2 v[2] = arr;
 }
 layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
 void main() {

diff --git a/test/tint/expressions/type_ctor/array/explicit/vec2/f32.wgsl.expected.ir.glsl b/test/tint/expressions/type_ctor/array/explicit/vec2/f32.wgsl.expected.ir.glsl
@@ -1,8 +1,8 @@
 #version 310 es
 
-float arr[2] = float[2](vec2(1.0f), vec2(2.0f));
+vec2 arr[2] = vec2[2](vec2(1.0f), vec2(2.0f));
 void f() {
-  float v[2] = arr;
+  vec2 v[2] = arr;
 }
 layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
 void main() {