From 6941c5354c0c93ffe476e719ca9d4735ac83a437 Mon Sep 17 00:00:00 2001
From: dan sinclair <dsinclair@google.com>
Date: Mon, 15 Jun 2020 20:58:28 +0000
Subject: [PATCH] [spirv-writer] Allow emitting an array stride.

This CL adds the ability to attach a stride to an array type and have it
emitted during SPIR-V generation.

Bug: tint:5
Change-Id: I9c0f0a6afef6ae6662b64f4da2c150ba3f8da29f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/23223
Reviewed-by: David Neto <dneto@google.com>
---
 src/ast/type/array_type.cc            |  2 ++
 src/ast/type/array_type.h             |  9 +++++++++
 src/ast/type/array_type_test.cc       | 13 +++++++++++++
 src/ast/type/struct_type.h            |  5 +++++
 src/writer/spirv/builder.cc           | 12 ++++++++++++
 src/writer/spirv/builder_type_test.cc | 21 +++++++++++++++++++++
 6 files changed, 62 insertions(+)

diff --git a/src/ast/type/array_type.cc b/src/ast/type/array_type.cc
index 2c840d30ec..6fb5f95834 100644
--- a/src/ast/type/array_type.cc
+++ b/src/ast/type/array_type.cc
@@ -37,6 +37,8 @@ std::string ArrayType::type_name() const {
   std::string type_name = "__array" + subtype_->type_name();
   if (!IsRuntimeArray())
     type_name += "_" + std::to_string(size_);
+  if (has_array_stride())
+    type_name += "_" + std::to_string(array_stride_);
 
   return type_name;
 }
diff --git a/src/ast/type/array_type.h b/src/ast/type/array_type.h
index 7bc802393f..e89684c307 100644
--- a/src/ast/type/array_type.h
+++ b/src/ast/type/array_type.h
@@ -45,6 +45,14 @@ class ArrayType : public Type {
   /// i.e. the size is determined at runtime
   bool IsRuntimeArray() const { return size_ == 0; }
 
+  /// Sets the array stride
+  /// @param stride the stride to set
+  void set_array_stride(uint32_t stride) { array_stride_ = stride; }
+  /// @returns the array stride or 0 if none set.
+  uint32_t array_stride() const { return array_stride_; }
+  /// @returns true if the array has a stride set
+  bool has_array_stride() const { return array_stride_ != 0; }
+
   /// @returns the array type
   Type* type() const { return subtype_; }
   /// @returns the array size. Size is 0 for a runtime array
@@ -56,6 +64,7 @@ class ArrayType : public Type {
  private:
   Type* subtype_ = nullptr;
   uint32_t size_ = 0;
+  uint32_t array_stride_ = 0;
 };
 
 }  // namespace type
diff --git a/src/ast/type/array_type_test.cc b/src/ast/type/array_type_test.cc
index b7ff81971e..a4e4eae1b0 100644
--- a/src/ast/type/array_type_test.cc
+++ b/src/ast/type/array_type_test.cc
@@ -60,11 +60,24 @@ TEST_F(ArrayTypeTest, Is) {
 }
 
 TEST_F(ArrayTypeTest, TypeName) {
+  I32Type i32;
+  ArrayType arr{&i32};
+  EXPECT_EQ(arr.type_name(), "__array__i32");
+}
+
+TEST_F(ArrayTypeTest, TypeName_RuntimeArray) {
   I32Type i32;
   ArrayType arr{&i32, 3};
   EXPECT_EQ(arr.type_name(), "__array__i32_3");
 }
 
+TEST_F(ArrayTypeTest, TypeName_WithStride) {
+  I32Type i32;
+  ArrayType arr{&i32, 3};
+  arr.set_array_stride(16);
+  EXPECT_EQ(arr.type_name(), "__array__i32_3_16");
+}
+
 }  // namespace
 }  // namespace type
 }  // namespace ast
diff --git a/src/ast/type/struct_type.h b/src/ast/type/struct_type.h
index 18ce29622e..97cb4f2daa 100644
--- a/src/ast/type/struct_type.h
+++ b/src/ast/type/struct_type.h
@@ -41,6 +41,11 @@ class StructType : public Type {
   /// @returns the struct name
   const std::string& name() const { return name_; }
 
+  /// @returns true if the struct has a block decoration
+  bool IsBlockDecorated() const {
+    return struct_->decoration() == StructDecoration::kBlock;
+  }
+
   /// @returns true if the type is a struct type
   bool IsStruct() const override;
 
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index cf3ee87ef5..607f2ce92f 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -1681,6 +1681,7 @@ bool Builder::GenerateArrayType(ast::type::ArrayType* ary,
     return false;
   }
 
+  auto result_id = result.to_i();
   if (ary->IsRuntimeArray()) {
     push_type(spv::Op::OpTypeRuntimeArray, {result, Operand::Int(elem_type)});
   } else {
@@ -1692,6 +1693,17 @@ bool Builder::GenerateArrayType(ast::type::ArrayType* ary,
     push_type(spv::Op::OpTypeArray,
               {result, Operand::Int(elem_type), Operand::Int(len_id)});
   }
+
+  // SPIR-V explicitly requires no array stride if the array contains a struct
+  // which has a Block decoration.
+  if (ary->type()->IsStruct() && ary->type()->AsStruct()->IsBlockDecorated()) {
+    return true;
+  }
+  if (ary->has_array_stride()) {
+    push_annot(spv::Op::OpDecorate,
+               {Operand::Int(result_id), Operand::Int(SpvDecorationArrayStride),
+                Operand::Int(ary->array_stride())});
+  }
   return true;
 }
 
diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc
index b49e987546..b554b311e9 100644
--- a/src/writer/spirv/builder_type_test.cc
+++ b/src/writer/spirv/builder_type_test.cc
@@ -118,6 +118,27 @@ TEST_F(BuilderTest_Type, GenerateArray) {
 )");
 }
 
+TEST_F(BuilderTest_Type, GenerateArray_WithStride) {
+  ast::type::I32Type i32;
+  ast::type::ArrayType ary(&i32, 4);
+  ary.set_array_stride(16u);
+
+  ast::Module mod;
+  Builder b(&mod);
+  auto id = b.GenerateTypeIfNeeded(&ary);
+  ASSERT_FALSE(b.has_error()) << b.error();
+  EXPECT_EQ(1u, id);
+
+  EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 ArrayStride 16
+)");
+
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeInt 32 1
+%3 = OpTypeInt 32 0
+%4 = OpConstant %3 4
+%1 = OpTypeArray %2 %4
+)");
+}
+
 TEST_F(BuilderTest_Type, ReturnsGeneratedArray) {
   ast::type::I32Type i32;
   ast::type::ArrayType ary(&i32, 4);