commit 8d133e2ca5a8f630c2559545e204b9855d33817b
author Ben Clayton <bclayton@google.com> Mon Jun 21 19:27:56 2021 +0000
committer Tint LUCI CQ <tint-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jun 21 19:27:56 2021 +0000
tree 4f48caae791c03eee6beb180eaa90dd6a8113700
parent a05669ae71887047383b05e6d0d9e07107adbdfb

writer/msl: Fix array stride calculations

We shouldn't be calculating array strides with the packed element size.

Bug: tint:180
Bug: tint:649
Bug: tint:898
Change-Id: Ifa23517480435610b20c1597c2fc2c2837f4280a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55257
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>

src/writer/msl/generator_impl.cc

diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index 9fea868..6e74487 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -1962,14 +1962,13 @@
   }
 
   if (auto* arr = ty->As<sem::Array>()) {
-    auto el_size_align = MslPackedTypeSizeAndAlign(arr->ElemType());
     if (!arr->IsStrideImplicit()) {
       TINT_ICE(diagnostics_) << "arrays with explicit strides should have "
                                 "removed with the PadArrayElements transform";
       return {};
     }
     auto num_els = std::max<uint32_t>(arr->Count(), 1);
-    return SizeAndAlign{el_size_align.size * num_els, el_size_align.align};
+    return SizeAndAlign{arr->Stride() * num_els, arr->Align()};
   }
 
   if (auto* str = ty->As<sem::Struct>()) {

src/writer/msl/generator_impl_type_test.cc

diff --git a/src/writer/msl/generator_impl_type_test.cc b/src/writer/msl/generator_impl_type_test.cc
index af58f47..e92e7b5 100644
--- a/src/writer/msl/generator_impl_type_test.cc
+++ b/src/writer/msl/generator_impl_type_test.cc
@@ -524,6 +524,47 @@
 #undef ALL_FIELDS
 }
 
+TEST_F(MslGeneratorImplTest, EmitType_Struct_Layout_ArrayVec3DefaultStride) {
+  // array: size(64), align(16)
+  auto* array = ty.array(ty.vec3<f32>(), 4);
+
+  auto* s =
+      Structure("S",
+                {
+                    Member("a", ty.i32()),
+                    Member("b", array),
+                    Member("c", ty.i32()),
+                },
+                ast::DecorationList{create<ast::StructBlockDecoration>()});
+
+  Global("G", ty.Of(s), ast::StorageClass::kStorage, ast::Access::kRead,
+         ast::DecorationList{
+             create<ast::BindingDecoration>(0),
+             create<ast::GroupDecoration>(0),
+         });
+
+  GeneratorImpl& gen = Build();
+
+  auto* sem_s = program->TypeOf(s)->As<sem::Struct>();
+  ASSERT_TRUE(gen.EmitStructType(sem_s)) << gen.error();
+
+  // ALL_FIELDS() calls the macro FIELD(ADDR, TYPE, NAME, SUFFIX)
+  // for each field of the structure s.
+#define ALL_FIELDS()                      \
+  FIELD(0x0000, int, a, /*NO SUFFIX*/)    \
+  FIELD(0x0004, int8_t, tint_pad_0, [12]) \
+  FIELD(0x0010, float3, b, [4])           \
+  FIELD(0x0050, int, c, /*NO SUFFIX*/)    \
+  FIELD(0x0054, int8_t, tint_pad_1, [12])
+
+  // Check that the generated string is as expected.
+#define FIELD(ADDR, TYPE, NAME, SUFFIX) \
+  "  /* " #ADDR " */ " #TYPE " " #NAME #SUFFIX ";\n"
+  auto* expect = "struct S {\n" ALL_FIELDS() "};\n";
+#undef FIELD
+  EXPECT_EQ(gen.result(), expect);
+}
+
 TEST_F(MslGeneratorImplTest, AttemptTintPadSymbolCollision) {
   auto* s = Structure(
       "S",