Add transform/PadArrayElements

Replaces arrays with an explicit stride with an array to a structure holding the element padded with a `[[size]]` decoration.

Note that the HLSL writer is still not correctly emitting structure fields with a `[[size]]`, which will be fixed in a follow up change.

Bug: tint:182
Bug: tint:895
Fixed: tint:180
Fixed: tint:649
Change-Id: Ic135dfc89309ac805507e9f39392577c7f82d154
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54582
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>

src/BUILD.gn

@@ -559,6 +559,8 @@ libtint_source_set("libtint_core_all_src") {
     "transform/inline_pointer_lets.h",
     "transform/manager.cc",
     "transform/manager.h",
+    "transform/pad_array_elements.cc",
+    "transform/pad_array_elements.h",
     "transform/promote_initializers_to_const_var.cc",
     "transform/promote_initializers_to_const_var.h",
     "transform/renamer.cc",

src/CMakeLists.txt

@@ -284,6 +284,8 @@ set(TINT_LIB_SRCS
   transform/inline_pointer_lets.h
   transform/manager.cc
   transform/manager.h
+  transform/pad_array_elements.cc
+  transform/pad_array_elements.h
   transform/promote_initializers_to_const_var.cc
   transform/promote_initializers_to_const_var.h
   transform/renamer.cc
@@ -851,6 +853,7 @@ if(${TINT_BUILD_TESTS})
       transform/external_texture_transform_test.cc
       transform/first_index_offset_test.cc
       transform/inline_pointer_lets_test.cc
+      transform/pad_array_elements_test.cc
       transform/promote_initializers_to_const_var_test.cc
       transform/renamer_test.cc
       transform/simplify_test.cc

src/resolver/resolver.cc

@@ -2922,7 +2922,7 @@ sem::Array* Resolver::Array(const ast::Array* arr) {
   // records an element count of 0 for it.
   auto size = std::max<uint32_t>(arr->size(), 1) * stride;
   auto* sem = builder_->create<sem::Array>(el_ty, arr->size(), el_align, size,
-                                           stride, stride == implicit_stride);
+                                           stride, implicit_stride);
 
   if (!ValidateArray(sem, source)) {
     return nullptr;

src/sem/array.cc

@@ -28,13 +28,13 @@ Array::Array(const Type* element,
              uint32_t align,
              uint32_t size,
              uint32_t stride,
-             bool stride_implicit)
+             uint32_t implicit_stride)
     : element_(element),
       count_(count),
       align_(align),
       size_(size),
       stride_(stride),
-      stride_implicit_(stride_implicit) {
+      implicit_stride_(implicit_stride) {
   TINT_ASSERT(element_);
 }
 
@@ -44,14 +44,14 @@ std::string Array::type_name() const {
   type_name += "_align_" + std::to_string(align_);
   type_name += "_size_" + std::to_string(size_);
   type_name += "_stride_" + std::to_string(stride_);
-  // Note: stride_implicit is not part of the type_name string as this is a
+  // Note: implicit_stride is not part of the type_name string as this is
-  // property derived from the other fields.
+  // derived from the element type
   return type_name;
 }
 
 std::string Array::FriendlyName(const SymbolTable& symbols) const {
   std::ostringstream out;
-  if (!stride_implicit_) {
+  if (!IsStrideImplicit()) {
     out << "[[stride(" << stride_ << ")]] ";
   }
   out << "array<" << element_->FriendlyName(symbols);

src/sem/array.h

@@ -42,14 +42,15 @@ class Array : public Castable<Array, Type> {
   /// @param size the byte size of the array
   /// @param stride the number of bytes from the start of one element of the
   /// array to the start of the next element
-  /// @param stride_implicit is true if the value of `stride` matches the
+  /// @param implicit_stride the number of bytes from the start of one element
-  /// element's natural stride.
+  /// of the array to the start of the next element, if there was no [[stride]]
+  /// decoration applied.
   Array(Type const* element,
         uint32_t count,
         uint32_t align,
         uint32_t size,
         uint32_t stride,
-        bool stride_implicit);
+        uint32_t implicit_stride);
 
   /// @return the array element type
   Type const* ElemType() const { return element_; }
@@ -72,9 +73,14 @@ class Array : public Castable<Array, Type> {
   /// array to the start of the next element
   uint32_t Stride() const { return stride_; }
 
-  /// @returns true if the value returned by Stride() does matches the
+  /// @returns the number of bytes from the start of one element of the
-  /// element's natural stride
+  /// array to the start of the next element, if there was no [[stride]]
-  bool IsStrideImplicit() const { return stride_implicit_; }
+  /// decoration applied
+  uint32_t ImplicitStride() const { return implicit_stride_; }
+
+  /// @returns true if the value returned by Stride() matches the element's
+  /// natural stride
+  bool IsStrideImplicit() const { return stride_ == implicit_stride_; }
 
   /// @returns true if this array is runtime sized
   bool IsRuntimeSized() const { return count_ == 0; }
@@ -93,7 +99,7 @@ class Array : public Castable<Array, Type> {
   uint32_t const align_;
   uint32_t const size_;
   uint32_t const stride_;
-  bool const stride_implicit_;
+  uint32_t const implicit_stride_;
 };
 
 }  // namespace sem

src/sem/sem_array_test.cc

@@ -23,57 +23,59 @@ using ArrayTest = TestHelper;
 
 TEST_F(ArrayTest, CreateSizedArray) {
   U32 u32;
-  auto* arr = create<Array>(&u32, 2, 4, 8, 16, true);
+  auto* arr = create<Array>(&u32, 2, 4, 8, 32, 16);
   EXPECT_EQ(arr->ElemType(), &u32);
   EXPECT_EQ(arr->Count(), 2u);
   EXPECT_EQ(arr->Align(), 4u);
   EXPECT_EQ(arr->SizeInBytes(), 8u);
-  EXPECT_EQ(arr->Stride(), 16u);
+  EXPECT_EQ(arr->Stride(), 32u);
-  EXPECT_TRUE(arr->IsStrideImplicit());
+  EXPECT_EQ(arr->ImplicitStride(), 16u);
+  EXPECT_FALSE(arr->IsStrideImplicit());
   EXPECT_FALSE(arr->IsRuntimeSized());
 }
 
 TEST_F(ArrayTest, CreateRuntimeArray) {
   U32 u32;
-  auto* arr = create<Array>(&u32, 0, 4, 8, 16, true);
+  auto* arr = create<Array>(&u32, 0, 4, 8, 32, 32);
   EXPECT_EQ(arr->ElemType(), &u32);
   EXPECT_EQ(arr->Count(), 0u);
   EXPECT_EQ(arr->Align(), 4u);
   EXPECT_EQ(arr->SizeInBytes(), 8u);
-  EXPECT_EQ(arr->Stride(), 16u);
+  EXPECT_EQ(arr->Stride(), 32u);
+  EXPECT_EQ(arr->ImplicitStride(), 32u);
   EXPECT_TRUE(arr->IsStrideImplicit());
   EXPECT_TRUE(arr->IsRuntimeSized());
 }
 
 TEST_F(ArrayTest, TypeName) {
   I32 i32;
-  auto* arr = create<Array>(&i32, 2, 0, 4, 4, true);
+  auto* arr = create<Array>(&i32, 2, 0, 4, 4, 4);
   EXPECT_EQ(arr->type_name(), "__array__i32_count_2_align_0_size_4_stride_4");
 }
 
 TEST_F(ArrayTest, FriendlyNameRuntimeSized) {
-  auto* arr = create<Array>(create<I32>(), 0, 0, 4, 4, true);
+  auto* arr = create<Array>(create<I32>(), 0, 0, 4, 4, 4);
   EXPECT_EQ(arr->FriendlyName(Symbols()), "array<i32>");
 }
 
 TEST_F(ArrayTest, FriendlyNameStaticSized) {
-  auto* arr = create<Array>(create<I32>(), 5, 4, 20, 4, true);
+  auto* arr = create<Array>(create<I32>(), 5, 4, 20, 4, 4);
   EXPECT_EQ(arr->FriendlyName(Symbols()), "array<i32, 5>");
 }
 
 TEST_F(ArrayTest, FriendlyNameRuntimeSizedNonImplicitStride) {
-  auto* arr = create<Array>(create<I32>(), 0, 0, 4, 4, false);
+  auto* arr = create<Array>(create<I32>(), 0, 0, 4, 8, 4);
-  EXPECT_EQ(arr->FriendlyName(Symbols()), "[[stride(4)]] array<i32>");
+  EXPECT_EQ(arr->FriendlyName(Symbols()), "[[stride(8)]] array<i32>");
 }
 
 TEST_F(ArrayTest, FriendlyNameStaticSizedNonImplicitStride) {
-  auto* arr = create<Array>(create<I32>(), 5, 4, 20, 4, false);
+  auto* arr = create<Array>(create<I32>(), 5, 4, 20, 8, 4);
-  EXPECT_EQ(arr->FriendlyName(Symbols()), "[[stride(4)]] array<i32, 5>");
+  EXPECT_EQ(arr->FriendlyName(Symbols()), "[[stride(8)]] array<i32, 5>");
 }
 
 TEST_F(ArrayTest, TypeName_RuntimeArray) {
   I32 i32;
-  auto* arr = create<Array>(&i32, 2, 4, 8, 16, true);
+  auto* arr = create<Array>(&i32, 2, 4, 8, 16, 16);
   EXPECT_EQ(arr->type_name(), "__array__i32_count_2_align_4_size_8_stride_16");
 }
 

src/transform/hlsl.cc

@@ -23,6 +23,7 @@
 #include "src/transform/external_texture_transform.h"
 #include "src/transform/inline_pointer_lets.h"
 #include "src/transform/manager.h"
+#include "src/transform/pad_array_elements.h"
 #include "src/transform/promote_initializers_to_const_var.h"
 #include "src/transform/simplify.h"
 #include "src/transform/wrap_arrays_in_structs.h"
@@ -52,6 +53,7 @@ Output Hlsl::Run(const Program* in, const DataMap&) {
   manager.Add<ExternalTextureTransform>();
   manager.Add<PromoteInitializersToConstVar>();
   manager.Add<WrapArraysInStructs>();
+  manager.Add<PadArrayElements>();
   data.Add<CanonicalizeEntryPointIO::Config>(
       CanonicalizeEntryPointIO::BuiltinStyle::kStructMember);
   auto out = manager.Run(in, data);

src/transform/msl.cc

@@ -27,6 +27,7 @@
 #include "src/transform/canonicalize_entry_point_io.h"
 #include "src/transform/external_texture_transform.h"
 #include "src/transform/manager.h"
+#include "src/transform/pad_array_elements.h"
 #include "src/transform/promote_initializers_to_const_var.h"
 #include "src/transform/wrap_arrays_in_structs.h"
 
@@ -43,6 +44,7 @@ Output Msl::Run(const Program* in, const DataMap&) {
   manager.Add<ExternalTextureTransform>();
   manager.Add<PromoteInitializersToConstVar>();
   manager.Add<WrapArraysInStructs>();
+  manager.Add<PadArrayElements>();
   data.Add<CanonicalizeEntryPointIO::Config>(
       CanonicalizeEntryPointIO::BuiltinStyle::kParameter);
   auto out = manager.Run(in, data);

src/transform/pad_array_elements.cc

@@ -0,0 +1,157 @@
+// 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/pad_array_elements.h"
+
+#include <unordered_map>
+#include <utility>
+
+#include "src/program_builder.h"
+#include "src/sem/array.h"
+#include "src/sem/expression.h"
+#include "src/utils/get_or_create.h"
+
+namespace tint {
+namespace transform {
+namespace {
+
+using ArrayBuilder = std::function<ast::Array*()>;
+
+/// PadArray returns a function that constructs a new array in `ctx.dst` with
+/// the element type padded to account for the explicit stride. PadArray will
+/// recursively pad arrays-of-arrays. The new array element type will be added
+/// to module-scope type declarations of `ctx.dst`.
+/// @param ctx the CloneContext
+/// @param create_ast_type_for Transform::CreateASTTypeFor()
+/// @param padded_arrays a map of src array type to the new array name
+/// @param array the array type
+/// @return the new AST array
+template <typename CREATE_AST_TYPE_FOR>
+ArrayBuilder PadArray(
+    CloneContext& ctx,
+    CREATE_AST_TYPE_FOR&& create_ast_type_for,
+    std::unordered_map<const sem::Array*, ArrayBuilder>& padded_arrays,
+    const sem::Array* array) {
+  if (array->IsStrideImplicit()) {
+    // We don't want to wrap arrays that have an implicit stride
+    return nullptr;
+  }
+
+  return utils::GetOrCreate(padded_arrays, array, [&] {
+    // Generate a unique name for the array element type
+    auto name = ctx.dst->Symbols().New("tint_padded_array_element");
+
+    // Examine the element type. Is it also an array?
+    ast::Type* el_ty = nullptr;
+    if (auto* el_array = array->ElemType()->As<sem::Array>()) {
+      // Array of array - call PadArray() on the element type
+      if (auto p =
+              PadArray(ctx, create_ast_type_for, padded_arrays, el_array)) {
+        el_ty = p();
+      }
+    }
+
+    // If the element wasn't a padded array, just create the typical AST type
+    // for it
+    if (el_ty == nullptr) {
+      el_ty = create_ast_type_for(&ctx, array->ElemType());
+    }
+
+    // Structure() will create and append the ast::Struct to the
+    // global declarations of `ctx.dst`. As we haven't finished building the
+    // current module-scope statement or function, this will be placed
+    // immediately before the usage.
+    ctx.dst->Structure(
+        name,
+        {ctx.dst->Member("el", el_ty, {ctx.dst->MemberSize(array->Stride())})});
+
+    auto* dst = ctx.dst;
+    return [=] {
+      return dst->ty.array(dst->create<ast::TypeName>(name), array->Count());
+    };
+  });
+}
+
+}  // namespace
+
+PadArrayElements::PadArrayElements() = default;
+
+PadArrayElements::~PadArrayElements() = default;
+
+Output PadArrayElements::Run(const Program* in, const DataMap&) {
+  ProgramBuilder out;
+  CloneContext ctx(&out, in);
+
+  auto& sem = ctx.src->Sem();
+
+  std::unordered_map<const sem::Array*, ArrayBuilder> padded_arrays;
+  auto pad = [&](const sem::Array* array) {
+    return PadArray(ctx, CreateASTTypeFor, padded_arrays, array);
+  };
+
+  // Replace all array types with their corresponding padded array type
+  ctx.ReplaceAll([&](ast::Type* ast_type) -> ast::Type* {
+    auto* type = ctx.src->TypeOf(ast_type);
+    if (auto* array = type->UnwrapRef()->As<sem::Array>()) {
+      if (auto p = pad(array)) {
+        return p();
+      }
+    }
+    return nullptr;
+  });
+
+  // Fix up array accessors so `a[1]` becomes `a[1].el`
+  ctx.ReplaceAll(
+      [&](ast::ArrayAccessorExpression* accessor) -> ast::Expression* {
+        if (auto* array = tint::As<sem::Array>(
+                sem.Get(accessor->array())->Type()->UnwrapRef())) {
+          if (pad(array)) {
+            // Array element is wrapped in a structure. Emit a member accessor
+            // to get to the actual array element.
+            auto* idx = ctx.CloneWithoutTransform(accessor);
+            return ctx.dst->MemberAccessor(idx, "el");
+          }
+        }
+        return nullptr;
+      });
+
+  // Fix up array constructors so `A(1,2)` becomes
+  // `A(padded(1), padded(2))`
+  ctx.ReplaceAll([&](ast::TypeConstructorExpression* ctor) -> ast::Expression* {
+    if (auto* array =
+            tint::As<sem::Array>(sem.Get(ctor)->Type()->UnwrapRef())) {
+      if (auto p = pad(array)) {
+        auto* arr_ty = p();
+        auto el_typename = arr_ty->type()->As<ast::TypeName>()->name();
+
+        ast::ExpressionList args;
+        args.reserve(ctor->values().size());
+        for (auto* arg : ctor->values()) {
+          args.emplace_back(ctx.dst->Construct(
+              ctx.dst->create<ast::TypeName>(el_typename), ctx.Clone(arg)));
+        }
+
+        return ctx.dst->Construct(arr_ty, args);
+      }
+    }
+    return nullptr;
+  });
+
+  ctx.Clone();
+
+  return Output(Program(std::move(out)));
+}
+
+}  // namespace transform
+}  // namespace tint

src/transform/pad_array_elements.h

@@ -0,0 +1,51 @@
+// 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.
+
+#ifndef SRC_TRANSFORM_PAD_ARRAY_ELEMENTS_H_
+#define SRC_TRANSFORM_PAD_ARRAY_ELEMENTS_H_
+
+#include "src/transform/transform.h"
+
+namespace tint {
+namespace transform {
+
+/// PadArrayElements is a transform that replaces array types with an explicit
+/// stride that is larger than the implicit stride, with an array of a new
+/// structure type. This structure holds with a single field of the element
+/// type, decorated with a [[size]] decoration to pad the structure to the
+/// required array stride. The new array types have no explicit stride,
+/// structure size is equal to the desired stride.
+/// Array index expressions and constructors are also adjusted to deal with this
+/// structure element type.
+/// This transform helps with backends that cannot directly return arrays or use
+/// them as parameters.
+class PadArrayElements : public Transform {
+ public:
+  /// Constructor
+  PadArrayElements();
+
+  /// Destructor
+  ~PadArrayElements() override;
+
+  /// Runs the transform on `program`, returning the transformation result.
+  /// @param program the source program to transform
+  /// @param data optional extra transform-specific input data
+  /// @returns the transformation result
+  Output Run(const Program* program, const DataMap& data = {}) override;
+};
+
+}  // namespace transform
+}  // namespace tint
+
+#endif  // SRC_TRANSFORM_PAD_ARRAY_ELEMENTS_H_

src/transform/pad_array_elements_test.cc

@@ -0,0 +1,379 @@
+// 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/pad_array_elements.h"
+
+#include <utility>
+
+#include "src/transform/test_helper.h"
+
+namespace tint {
+namespace transform {
+namespace {
+
+using PadArrayElementsTest = TransformTest;
+
+TEST_F(PadArrayElementsTest, EmptyModule) {
+  auto* src = "";
+  auto* expect = "";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, ImplicitArrayStride) {
+  auto* src = R"(
+var<private> arr : array<i32, 4>;
+)";
+  auto* expect = src;
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, ArrayAsGlobal) {
+  auto* src = R"(
+var<private> arr : [[stride(8)]] array<i32, 4>;
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  [[size(8)]]
+  el : i32;
+};
+
+var<private> arr : array<tint_padded_array_element, 4>;
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, ArrayFunctionVar) {
+  auto* src = R"(
+fn f() {
+  var arr : [[stride(16)]] array<i32, 4>;
+  arr = [[stride(16)]] array<i32, 4>();
+  arr = [[stride(16)]] array<i32, 4>(1, 2, 3, 4);
+  let x = arr[3];
+}
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  [[size(16)]]
+  el : i32;
+};
+
+fn f() {
+  var arr : array<tint_padded_array_element, 4>;
+  arr = array<tint_padded_array_element, 4>();
+  arr = array<tint_padded_array_element, 4>(tint_padded_array_element(1), tint_padded_array_element(2), tint_padded_array_element(3), tint_padded_array_element(4));
+  let x = arr[3].el;
+}
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, ArrayAsParam) {
+  auto* src = R"(
+fn f(a : [[stride(12)]] array<i32, 4>) -> i32 {
+  return a[2];
+}
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  [[size(12)]]
+  el : i32;
+};
+
+fn f(a : array<tint_padded_array_element, 4>) -> i32 {
+  return a[2].el;
+}
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+// TODO(crbug.com/tint/781): Cannot parse the stride on the return array type.
+TEST_F(PadArrayElementsTest, DISABLED_ArrayAsReturn) {
+  auto* src = R"(
+fn f() -> [[stride(8)]] array<i32, 4> {
+  return array<i32, 4>(1, 2, 3, 4);
+}
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  el : i32;
+  [[size(4)]]
+  padding : u32;
+};
+
+fn f() -> array<tint_padded_array_element, 4> {
+  return array<tint_padded_array_element, 4>(tint_padded_array_element(1, 0u), tint_padded_array_element(2, 0u), tint_padded_array_element(3, 0u), tint_padded_array_element(4, 0u));
+}
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, ArrayAlias) {
+  auto* src = R"(
+type Array = [[stride(16)]] array<i32, 4>;
+
+fn f() {
+  var arr : Array;
+  arr = Array();
+  arr = Array(1, 2, 3, 4);
+  let vals : Array = Array(1, 2, 3, 4);
+  arr = vals;
+  let x = arr[3];
+}
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  [[size(16)]]
+  el : i32;
+};
+
+type Array = array<tint_padded_array_element, 4>;
+
+fn f() {
+  var arr : array<tint_padded_array_element, 4>;
+  arr = array<tint_padded_array_element, 4>();
+  arr = array<tint_padded_array_element, 4>(tint_padded_array_element(1), tint_padded_array_element(2), tint_padded_array_element(3), tint_padded_array_element(4));
+  let vals : array<tint_padded_array_element, 4> = array<tint_padded_array_element, 4>(tint_padded_array_element(1), tint_padded_array_element(2), tint_padded_array_element(3), tint_padded_array_element(4));
+  arr = vals;
+  let x = arr[3].el;
+}
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, ArraysInStruct) {
+  auto* src = R"(
+struct S {
+  a : [[stride(8)]] array<i32, 4>;
+  b : [[stride(8)]] array<i32, 8>;
+  c : [[stride(8)]] array<i32, 4>;
+  d : [[stride(12)]] array<i32, 8>;
+};
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  [[size(8)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_1 {
+  [[size(8)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_2 {
+  [[size(12)]]
+  el : i32;
+};
+
+struct S {
+  a : array<tint_padded_array_element, 4>;
+  b : array<tint_padded_array_element_1, 8>;
+  c : array<tint_padded_array_element, 4>;
+  d : array<tint_padded_array_element_2, 8>;
+};
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, ArraysOfArraysInStruct) {
+  auto* src = R"(
+struct S {
+  a : [[stride(512)]] array<i32, 4>;
+  b : [[stride(512)]] array<[[stride(32)]] array<i32, 4>, 4>;
+  c : [[stride(512)]] array<[[stride(64)]] array<[[stride(8)]] array<i32, 4>, 4>, 4>;
+};
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  [[size(512)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_2 {
+  [[size(32)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_1 {
+  [[size(512)]]
+  el : array<tint_padded_array_element_2, 4>;
+};
+
+struct tint_padded_array_element_5 {
+  [[size(8)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_4 {
+  [[size(64)]]
+  el : array<tint_padded_array_element_5, 4>;
+};
+
+struct tint_padded_array_element_3 {
+  [[size(512)]]
+  el : array<tint_padded_array_element_4, 4>;
+};
+
+struct S {
+  a : array<tint_padded_array_element, 4>;
+  b : array<tint_padded_array_element_1, 4>;
+  c : array<tint_padded_array_element_3, 4>;
+};
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, AccessArraysOfArraysInStruct) {
+  auto* src = R"(
+struct S {
+  a : [[stride(512)]] array<i32, 4>;
+  b : [[stride(512)]] array<[[stride(32)]] array<i32, 4>, 4>;
+  c : [[stride(512)]] array<[[stride(64)]] array<[[stride(8)]] array<i32, 4>, 4>, 4>;
+};
+
+fn f(s : S) -> i32 {
+  return s.a[2] + s.b[1][2] + s.c[3][1][2];
+}
+)";
+  auto* expect = R"(
+struct tint_padded_array_element {
+  [[size(512)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_2 {
+  [[size(32)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_1 {
+  [[size(512)]]
+  el : array<tint_padded_array_element_2, 4>;
+};
+
+struct tint_padded_array_element_5 {
+  [[size(8)]]
+  el : i32;
+};
+
+struct tint_padded_array_element_4 {
+  [[size(64)]]
+  el : array<tint_padded_array_element_5, 4>;
+};
+
+struct tint_padded_array_element_3 {
+  [[size(512)]]
+  el : array<tint_padded_array_element_4, 4>;
+};
+
+struct S {
+  a : array<tint_padded_array_element, 4>;
+  b : array<tint_padded_array_element_1, 4>;
+  c : array<tint_padded_array_element_3, 4>;
+};
+
+fn f(s : S) -> i32 {
+  return ((s.a[2].el + s.b[1].el[2].el) + s.c[3].el[1].el[2].el);
+}
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadArrayElementsTest, DeclarationOrder) {
+  auto* src = R"(
+type T0 = i32;
+
+type T1 = [[stride(8)]] array<i32, 1>;
+
+type T2 = i32;
+
+fn f1(a : [[stride(8)]] array<i32, 2>) {
+}
+
+type T3 = i32;
+
+fn f2() {
+  var v : [[stride(8)]] array<i32, 3>;
+}
+)";
+  auto* expect = R"(
+type T0 = i32;
+
+struct tint_padded_array_element {
+  [[size(8)]]
+  el : i32;
+};
+
+type T1 = array<tint_padded_array_element, 1>;
+
+type T2 = i32;
+
+struct tint_padded_array_element_1 {
+  [[size(8)]]
+  el : i32;
+};
+
+fn f1(a : array<tint_padded_array_element_1, 2>) {
+}
+
+type T3 = i32;
+
+struct tint_padded_array_element_2 {
+  [[size(8)]]
+  el : i32;
+};
+
+fn f2() {
+  var v : array<tint_padded_array_element_2, 3>;
+}
+)";
+
+  auto got = Run<PadArrayElements>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
+}  // namespace
+}  // namespace transform
+}  // namespace tint

src/transform/transform_test.cc

@@ -78,7 +78,7 @@ TEST_F(CreateASTTypeForTest, Vector) {
 
 TEST_F(CreateASTTypeForTest, ArrayImplicitStride) {
   auto* arr = create([](ProgramBuilder& b) {
-    return b.create<sem::Array>(b.create<sem::F32>(), 2, 4, 4, 32u, true);
+    return b.create<sem::Array>(b.create<sem::F32>(), 2, 4, 4, 32u, 32u);
   });
   ASSERT_TRUE(arr->Is<ast::Array>());
   ASSERT_TRUE(arr->As<ast::Array>()->type()->Is<ast::F32>());
@@ -88,7 +88,7 @@ TEST_F(CreateASTTypeForTest, ArrayImplicitStride) {
 
 TEST_F(CreateASTTypeForTest, ArrayNonImplicitStride) {
   auto* arr = create([](ProgramBuilder& b) {
-    return b.create<sem::Array>(b.create<sem::F32>(), 2, 4, 4, 32u, false);
+    return b.create<sem::Array>(b.create<sem::F32>(), 2, 4, 4, 64u, 32u);
   });
   ASSERT_TRUE(arr->Is<ast::Array>());
   ASSERT_TRUE(arr->As<ast::Array>()->type()->Is<ast::F32>());
@@ -100,7 +100,7 @@ TEST_F(CreateASTTypeForTest, ArrayNonImplicitStride) {
                 ->decorations()[0]
                 ->As<ast::StrideDecoration>()
                 ->stride(),
-            32u);
+            64u);
 }
 
 TEST_F(CreateASTTypeForTest, Struct) {

src/writer/msl/generator_impl.cc

@@ -2157,9 +2157,6 @@ bool GeneratorImpl::EmitPackedType(const sem::Type* type,
 }
 
 bool GeneratorImpl::EmitStructType(const sem::Struct* str) {
-  // TODO(dsinclair): Block decoration?
-  // if (str->impl()->decoration() != ast::Decoration::kNone) {
-  // }
   out_ << "struct " << program_->Symbols().NameFor(str->Declaration()->name())
        << " {" << std::endl;
 
@@ -2457,12 +2454,9 @@ GeneratorImpl::SizeAndAlign GeneratorImpl::MslPackedTypeSizeAndAlign(
 
   if (auto* arr = ty->As<sem::Array>()) {
     auto el_size_align = MslPackedTypeSizeAndAlign(arr->ElemType());
-    if (arr->Stride() != el_size_align.size) {
+    if (!arr->IsStrideImplicit()) {
-      // TODO(crbug.com/tint/649): transform::Msl needs to replace these arrays
+      TINT_ICE(diagnostics_) << "arrays with explicit strides should have "
-      // with a new array type that has the element type padded to the required
+                                "removed with the PadArrayElements transform";
-      // stride.
-      TINT_UNIMPLEMENTED(diagnostics_)
-          << "Arrays with custom strides not yet implemented";
       return {};
     }
     auto num_els = std::max<uint32_t>(arr->Count(), 1);

src/writer/msl/generator_impl_type_test.cc

@@ -14,6 +14,8 @@
 
 #include <array>
 
+#include "gmock/gmock.h"
+
 #include "src/ast/struct_block_decoration.h"
 #include "src/sem/depth_texture_type.h"
 #include "src/sem/multisampled_texture_type.h"
@@ -26,6 +28,8 @@ namespace writer {
 namespace msl {
 namespace {
 
+using ::testing::HasSubstr;
+
 #define CHECK_TYPE_SIZE_AND_ALIGN(TYPE, SIZE, ALIGN)    \
   static_assert(sizeof(TYPE) == SIZE, "Bad type size"); \
   static_assert(alignof(TYPE) == ALIGN, "Bad type alignment")
@@ -77,19 +81,6 @@ TEST_F(MslGeneratorImplTest, EmitType_ArrayOfArray) {
   EXPECT_EQ(gen.result(), "bool ary[5][4]");
 }
 
-// TODO(dsinclair): Is this possible? What order should it output in?
-TEST_F(MslGeneratorImplTest, DISABLED_EmitType_ArrayOfArrayOfRuntimeArray) {
-  auto* a = ty.array<bool, 4>();
-  auto* b = ty.array(a, 5);
-  auto* c = ty.array(b, 0);
-  Global("G", c, ast::StorageClass::kPrivate);
-
-  GeneratorImpl& gen = Build();
-
-  ASSERT_TRUE(gen.EmitType(program->TypeOf(c), "ary")) << gen.error();
-  EXPECT_EQ(gen.result(), "bool ary[5][4][1]");
-}
-
 TEST_F(MslGeneratorImplTest, EmitType_ArrayOfArrayOfArray) {
   auto* a = ty.array<bool, 4>();
   auto* b = ty.array(a, 5);
@@ -122,6 +113,31 @@ TEST_F(MslGeneratorImplTest, EmitType_RuntimeArray) {
   EXPECT_EQ(gen.result(), "bool ary[1]");
 }
 
+TEST_F(MslGeneratorImplTest, EmitType_ArrayWithStride) {
+  auto* s = Structure("s", {Member("arr", ty.array<f32, 4>(64))},
+                      {create<ast::StructBlockDecoration>()});
+  auto* ubo = Global("ubo", ty.Of(s), ast::StorageClass::kUniform,
+                     ast::DecorationList{
+                         create<ast::GroupDecoration>(1),
+                         create<ast::BindingDecoration>(1),
+                     });
+  WrapInFunction(MemberAccessor(ubo, "arr"));
+
+  GeneratorImpl& gen = SanitizeAndBuild();
+
+  ASSERT_TRUE(gen.Generate()) << gen.error();
+  EXPECT_THAT(gen.result(), HasSubstr(R"(struct tint_padded_array_element {
+  /* 0x0000 */ float el;
+  /* 0x0004 */ int8_t tint_pad_0[60];
+};)"));
+  EXPECT_THAT(gen.result(), HasSubstr(R"(struct tint_array_wrapper {
+  /* 0x0000 */ tint_padded_array_element arr[4];
+};)"));
+  EXPECT_THAT(gen.result(), HasSubstr(R"(struct s {
+  /* 0x0000 */ tint_array_wrapper arr;
+};)"));
+}
+
 TEST_F(MslGeneratorImplTest, EmitType_Bool) {
   auto* bool_ = create<sem::Bool>();
 
@@ -596,8 +612,6 @@ TEST_F(MslGeneratorImplTest, AttemptTintPadSymbolCollision) {
 )");
 }
 
-// TODO(crbug.com/tint/649): Add tests for array with explicit stride.
-
 // TODO(dsinclair): How to translate [[block]]
 TEST_F(MslGeneratorImplTest, DISABLED_EmitType_Struct_WithDecoration) {
   auto* s = Structure("S",

test/BUILD.gn

@@ -279,6 +279,7 @@ tint_unittests_source_set("tint_unittests_core_src") {
     "../src/transform/external_texture_transform_test.cc",
     "../src/transform/first_index_offset_test.cc",
     "../src/transform/inline_pointer_lets_test.cc",
+    "../src/transform/pad_array_elements_test.cc",
     "../src/transform/promote_initializers_to_const_var_test.cc",
     "../src/transform/renamer_test.cc",
     "../src/transform/simplify_test.cc",

test/array/assign_to_function_var.wgsl.expected.hlsl

@@ -3,15 +3,18 @@ void unused_entry_point() {
   return;
 }
 
+struct tint_padded_array_element {
+  int el;
+};
 struct tint_array_wrapper {
-  int arr[4];
+  tint_padded_array_element arr[4];
 };
 struct S {
   tint_array_wrapper arr;
 };
 
 tint_array_wrapper tint_symbol_2(RWByteAddressBuffer buffer, uint offset) {
-  const tint_array_wrapper tint_symbol_3 = {{asint(buffer.Load((offset + 0u))), asint(buffer.Load((offset + 16u))), asint(buffer.Load((offset + 32u))), asint(buffer.Load((offset + 48u)))}};
+  const tint_array_wrapper tint_symbol_3 = {{{asint(buffer.Load((offset + 0u)))}, {asint(buffer.Load((offset + 16u)))}, {asint(buffer.Load((offset + 32u)))}, {asint(buffer.Load((offset + 48u)))}}};
   return tint_symbol_3;
 }
 
@@ -21,12 +24,12 @@ ConstantBuffer<S> src_uniform : register(b0, space0);
 RWByteAddressBuffer src_storage : register(u1, space0);
 
 tint_array_wrapper ret_arr() {
-  const tint_array_wrapper tint_symbol_4 = {{0, 0, 0, 0}};
+  const tint_array_wrapper tint_symbol_4 = {{{0}, {0}, {0}, {0}}};
   return tint_symbol_4;
 }
 
 S ret_struct_arr() {
-  const S tint_symbol_5 = {{{0, 0, 0, 0}}};
+  const S tint_symbol_5 = {{{{0}, {0}, {0}, {0}}}};
   return tint_symbol_5;
 }
 
@@ -41,13 +44,13 @@ struct tint_array_wrapper_1 {
 };
 
 void foo(tint_array_wrapper src_param) {
-  tint_array_wrapper src_function = {{0, 0, 0, 0}};
+  tint_array_wrapper src_function = {{{0}, {0}, {0}, {0}}};
-  tint_array_wrapper tint_symbol = {{0, 0, 0, 0}};
+  tint_array_wrapper tint_symbol = {{{0}, {0}, {0}, {0}}};
-  const tint_array_wrapper tint_symbol_6 = {{1, 2, 3, 3}};
+  const tint_array_wrapper tint_symbol_6 = {{{1}, {2}, {3}, {3}}};
   tint_symbol = tint_symbol_6;
   tint_symbol = src_param;
   tint_symbol = ret_arr();
-  const tint_array_wrapper src_let = {{0, 0, 0, 0}};
+  const tint_array_wrapper src_let = {{{0}, {0}, {0}, {0}}};
   tint_symbol = src_let;
   tint_symbol = src_function;
   tint_symbol = src_private;

test/array/assign_to_private_var.wgsl.expected.hlsl

@@ -3,15 +3,18 @@ void unused_entry_point() {
   return;
 }
 
+struct tint_padded_array_element {
+  int el;
+};
 struct tint_array_wrapper {
-  int arr[4];
+  tint_padded_array_element arr[4];
 };
 struct S {
   tint_array_wrapper arr;
 };
 
 tint_array_wrapper tint_symbol_2(RWByteAddressBuffer buffer, uint offset) {
-  const tint_array_wrapper tint_symbol_3 = {{asint(buffer.Load((offset + 0u))), asint(buffer.Load((offset + 16u))), asint(buffer.Load((offset + 32u))), asint(buffer.Load((offset + 48u)))}};
+  const tint_array_wrapper tint_symbol_3 = {{{asint(buffer.Load((offset + 0u)))}, {asint(buffer.Load((offset + 16u)))}, {asint(buffer.Load((offset + 32u)))}, {asint(buffer.Load((offset + 48u)))}}};
   return tint_symbol_3;
 }
 
@@ -34,22 +37,22 @@ struct tint_array_wrapper_1 {
 static tint_array_wrapper_1 dst_nested;
 
 tint_array_wrapper ret_arr() {
-  const tint_array_wrapper tint_symbol_4 = {{0, 0, 0, 0}};
+  const tint_array_wrapper tint_symbol_4 = {{{0}, {0}, {0}, {0}}};
   return tint_symbol_4;
 }
 
 S ret_struct_arr() {
-  const S tint_symbol_5 = {{{0, 0, 0, 0}}};
+  const S tint_symbol_5 = {{{{0}, {0}, {0}, {0}}}};
   return tint_symbol_5;
 }
 
 void foo(tint_array_wrapper src_param) {
-  tint_array_wrapper src_function = {{0, 0, 0, 0}};
+  tint_array_wrapper src_function = {{{0}, {0}, {0}, {0}}};
-  const tint_array_wrapper tint_symbol_6 = {{1, 2, 3, 3}};
+  const tint_array_wrapper tint_symbol_6 = {{{1}, {2}, {3}, {3}}};
   tint_symbol = tint_symbol_6;
   tint_symbol = src_param;
   tint_symbol = ret_arr();
-  const tint_array_wrapper src_let = {{0, 0, 0, 0}};
+  const tint_array_wrapper src_let = {{{0}, {0}, {0}, {0}}};
   tint_symbol = src_let;
   tint_symbol = src_function;
   tint_symbol = src_private;

test/array/assign_to_storage_var.wgsl.expected.hlsl

@@ -3,23 +3,26 @@ void unused_entry_point() {
   return;
 }
 
+struct tint_padded_array_element {
+  int el;
+};
 struct tint_array_wrapper {
-  int arr[4];
+  tint_padded_array_element arr[4];
 };
 struct S {
   tint_array_wrapper arr;
 };
 
 tint_array_wrapper tint_symbol_2(RWByteAddressBuffer buffer, uint offset) {
-  const tint_array_wrapper tint_symbol_9 = {{asint(buffer.Load((offset + 0u))), asint(buffer.Load((offset + 16u))), asint(buffer.Load((offset + 32u))), asint(buffer.Load((offset + 48u)))}};
+  const tint_array_wrapper tint_symbol_9 = {{{asint(buffer.Load((offset + 0u)))}, {asint(buffer.Load((offset + 16u)))}, {asint(buffer.Load((offset + 32u)))}, {asint(buffer.Load((offset + 48u)))}}};
   return tint_symbol_9;
 }
 
 void tint_symbol_4(RWByteAddressBuffer buffer, uint offset, tint_array_wrapper value) {
-  buffer.Store((offset + 0u), asuint(value.arr[0u]));
+  buffer.Store((offset + 0u), asuint(value.arr[0u].el));
-  buffer.Store((offset + 16u), asuint(value.arr[1u]));
+  buffer.Store((offset + 16u), asuint(value.arr[1u].el));
-  buffer.Store((offset + 32u), asuint(value.arr[2u]));
+  buffer.Store((offset + 32u), asuint(value.arr[2u].el));
-  buffer.Store((offset + 48u), asuint(value.arr[3u]));
+  buffer.Store((offset + 48u), asuint(value.arr[3u].el));
 }
 
 struct tint_array_wrapper_3 {
@@ -58,22 +61,22 @@ RWByteAddressBuffer tint_symbol : register(u2, space0);
 RWByteAddressBuffer dst_nested : register(u3, space0);
 
 tint_array_wrapper ret_arr() {
-  const tint_array_wrapper tint_symbol_10 = {{0, 0, 0, 0}};
+  const tint_array_wrapper tint_symbol_10 = {{{0}, {0}, {0}, {0}}};
   return tint_symbol_10;
 }
 
 S ret_struct_arr() {
-  const S tint_symbol_11 = {{{0, 0, 0, 0}}};
+  const S tint_symbol_11 = {{{{0}, {0}, {0}, {0}}}};
   return tint_symbol_11;
 }
 
 void foo(tint_array_wrapper src_param) {
-  tint_array_wrapper src_function = {{0, 0, 0, 0}};
+  tint_array_wrapper src_function = {{{0}, {0}, {0}, {0}}};
-  const tint_array_wrapper tint_symbol_12 = {{1, 2, 3, 3}};
+  const tint_array_wrapper tint_symbol_12 = {{{1}, {2}, {3}, {3}}};
   tint_symbol_4(tint_symbol, 0u, tint_symbol_12);
   tint_symbol_4(tint_symbol, 0u, src_param);
   tint_symbol_4(tint_symbol, 0u, ret_arr());
-  const tint_array_wrapper src_let = {{0, 0, 0, 0}};
+  const tint_array_wrapper src_let = {{{0}, {0}, {0}, {0}}};
   tint_symbol_4(tint_symbol, 0u, src_let);
   tint_symbol_4(tint_symbol, 0u, src_function);
   tint_symbol_4(tint_symbol, 0u, src_private);

test/array/assign_to_workgroup_var.wgsl.expected.hlsl

@@ -3,15 +3,18 @@ void unused_entry_point() {
   return;
 }
 
+struct tint_padded_array_element {
+  int el;
+};
 struct tint_array_wrapper {
-  int arr[4];
+  tint_padded_array_element arr[4];
 };
 struct S {
   tint_array_wrapper arr;
 };
 
 tint_array_wrapper tint_symbol_2(RWByteAddressBuffer buffer, uint offset) {
-  const tint_array_wrapper tint_symbol_3 = {{asint(buffer.Load((offset + 0u))), asint(buffer.Load((offset + 16u))), asint(buffer.Load((offset + 32u))), asint(buffer.Load((offset + 48u)))}};
+  const tint_array_wrapper tint_symbol_3 = {{{asint(buffer.Load((offset + 0u)))}, {asint(buffer.Load((offset + 16u)))}, {asint(buffer.Load((offset + 32u)))}, {asint(buffer.Load((offset + 48u)))}}};
   return tint_symbol_3;
 }
 
@@ -34,22 +37,22 @@ struct tint_array_wrapper_1 {
 groupshared tint_array_wrapper_1 dst_nested;
 
 tint_array_wrapper ret_arr() {
-  const tint_array_wrapper tint_symbol_4 = {{0, 0, 0, 0}};
+  const tint_array_wrapper tint_symbol_4 = {{{0}, {0}, {0}, {0}}};
   return tint_symbol_4;
 }
 
 S ret_struct_arr() {
-  const S tint_symbol_5 = {{{0, 0, 0, 0}}};
+  const S tint_symbol_5 = {{{{0}, {0}, {0}, {0}}}};
   return tint_symbol_5;
 }
 
 void foo(tint_array_wrapper src_param) {
-  tint_array_wrapper src_function = {{0, 0, 0, 0}};
+  tint_array_wrapper src_function = {{{0}, {0}, {0}, {0}}};
-  const tint_array_wrapper tint_symbol_6 = {{1, 2, 3, 3}};
+  const tint_array_wrapper tint_symbol_6 = {{{1}, {2}, {3}, {3}}};
   tint_symbol = tint_symbol_6;
   tint_symbol = src_param;
   tint_symbol = ret_arr();
-  const tint_array_wrapper src_let = {{0, 0, 0, 0}};
+  const tint_array_wrapper src_let = {{{0}, {0}, {0}, {0}}};
   tint_symbol = src_let;
   tint_symbol = src_function;
   tint_symbol = src_private;