sem::Type: Replace GetDefaultAlignAndSize() with Size() and Align()

This is a cleaner API, and the implementation doesn't have to know a bunch of information about all the derived types.

Change-Id: I96bebcb9f3ceda86fa34bd8e70961dee63fd7e13
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/59301
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>

src/inspector/inspector.cc

@@ -546,9 +546,9 @@ uint32_t Inspector::GetWorkgroupStorageSize(const std::string& entry_point) {
   auto* func_sem = program_->Sem().Get(func);
   for (const sem::Variable* var : func_sem->ReferencedModuleVariables()) {
     if (var->StorageClass() == ast::StorageClass::kWorkgroup) {
-      uint32_t align = 0;
+      auto* ty = var->Type()->UnwrapRef();
-      uint32_t size = 0;
+      uint32_t align = ty->Align();
-      var->Type()->UnwrapRef()->GetDefaultAlignAndSize(align, size);
+      uint32_t size = ty->Size();
 
       // This essentially matches std430 layout rules from GLSL, which are in
       // turn specified as an upper bound for Vulkan layout sizing. Since D3D

src/resolver/resolver.cc

@@ -693,9 +693,7 @@ bool Resolver::ValidateStorageClassLayout(const sem::Struct* str,
   };
 
   auto required_alignment_of = [&](const sem::Type* ty) {
-    uint32_t actual_align = 0;
+    uint32_t actual_align = ty->Align();
-    uint32_t actual_size = 0;
-    ty->GetDefaultAlignAndSize(actual_align, actual_size);
     uint32_t required_align = actual_align;
     if (is_uniform_struct_or_array(ty)) {
       required_align = utils::RoundUp(16u, actual_align);
@@ -3813,9 +3811,8 @@ sem::Array* Resolver::Array(const ast::Array* arr) {
     return nullptr;
   }
 
-  uint32_t el_align = 0;
+  uint32_t el_align = el_ty->Align();
-  uint32_t el_size = 0;
+  uint32_t el_size = el_ty->Size();
-  el_ty->GetDefaultAlignAndSize(el_align, el_size);
 
   if (!ValidateNoDuplicateDecorations(arr->decorations())) {
     return nullptr;
@@ -4035,9 +4032,8 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) {
     }
 
     uint32_t offset = struct_size;
-    uint32_t align = 0;
+    uint32_t align = type->Align();
-    uint32_t size = 0;
+    uint32_t size = type->Size();
-    type->GetDefaultAlignAndSize(align, size);
 
     if (!ValidateNoDuplicateDecorations(member->decorations())) {
       return nullptr;

src/sem/array.cc

@@ -68,5 +68,13 @@ std::string Array::FriendlyName(const SymbolTable& symbols) const {
   return out.str();
 }
 
+uint32_t Array::Align() const {
+  return align_;
+}
+
+uint32_t Array::Size() const {
+  return size_;
+}
+
 }  // namespace sem
 }  // namespace tint

src/sem/array.h

@@ -62,12 +62,12 @@ class Array : public Castable<Array, Type> {
   /// @returns the byte alignment of the array
   /// @note this may differ from the alignment of a structure member of this
   /// array type, if the member is annotated with the `[[align(n)]]` decoration.
-  uint32_t Align() const { return align_; }
+  uint32_t Align() const override;
 
   /// @returns the byte size of the array
   /// @note this may differ from the size of a structure member of this array
   /// type, if the member is annotated with the `[[size(n)]]` decoration.
-  uint32_t SizeInBytes() const { return size_; }
+  uint32_t Size() const override;
 
   /// @returns the number of bytes from the start of one element of the
   /// array to the start of the next element

src/sem/atomic_type.cc

@@ -38,6 +38,14 @@ std::string Atomic::FriendlyName(const SymbolTable& symbols) const {
   return out.str();
 }
 
+uint32_t Atomic::Size() const {
+  return subtype_->Size();
+}
+
+uint32_t Atomic::Align() const {
+  return subtype_->Align();
+}
+
 Atomic::Atomic(Atomic&&) = default;
 
 Atomic::~Atomic() = default;

src/sem/atomic_type.h

@@ -44,6 +44,12 @@ class Atomic : public Castable<Atomic, Type> {
   /// declared in WGSL.
   std::string FriendlyName(const SymbolTable& symbols) const override;
 
+  /// @returns the size in bytes of the type.
+  uint32_t Size() const override;
+
+  /// @returns the alignment in bytes of the type.
+  uint32_t Align() const override;
+
  private:
   sem::Type const* const subtype_;
 };

src/sem/bool_type.cc

@@ -39,5 +39,13 @@ bool Bool::IsConstructible() const {
   return true;
 }
 
+uint32_t Bool::Size() const {
+  return 4;
+}
+
+uint32_t Bool::Align() const {
+  return 4;
+}
+
 }  // namespace sem
 }  // namespace tint

src/sem/bool_type.h

@@ -48,6 +48,16 @@ class Bool : public Castable<Bool, Type> {
   /// @returns true if constructible as per
   /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
   bool IsConstructible() const override;
+
+  /// @returns the size in bytes of the type.
+  /// @note: booleans are not host-sharable, but still may exist in workgroup
+  /// storage.
+  uint32_t Size() const override;
+
+  /// @returns the alignment in bytes of the type.
+  /// @note: booleans are not host-sharable, but still may exist in workgroup
+  /// storage.
+  uint32_t Align() const override;
 };
 
 }  // namespace sem

src/sem/f32_type.cc

@@ -39,5 +39,13 @@ bool F32::IsConstructible() const {
   return true;
 }
 
+uint32_t F32::Size() const {
+  return 4;
+}
+
+uint32_t F32::Align() const {
+  return 4;
+}
+
 }  // namespace sem
 }  // namespace tint

src/sem/f32_type.h

@@ -42,6 +42,12 @@ class F32 : public Castable<F32, Type> {
   /// @returns true if constructible as per
   /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
   bool IsConstructible() const override;
+
+  /// @returns the size in bytes of the type.
+  uint32_t Size() const override;
+
+  /// @returns the alignment in bytes of the type.
+  uint32_t Align() const override;
 };
 
 }  // namespace sem

src/sem/i32_type.cc

@@ -39,5 +39,13 @@ bool I32::IsConstructible() const {
   return true;
 }
 
+uint32_t I32::Size() const {
+  return 4;
+}
+
+uint32_t I32::Align() const {
+  return 4;
+}
+
 }  // namespace sem
 }  // namespace tint

src/sem/i32_type.h

@@ -42,6 +42,12 @@ class I32 : public Castable<I32, Type> {
   /// @returns true if constructible as per
   /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
   bool IsConstructible() const override;
+
+  /// @returns the size in bytes of the type.
+  uint32_t Size() const override;
+
+  /// @returns the alignment in bytes of the type.
+  uint32_t Align() const override;
 };
 
 }  // namespace sem

src/sem/matrix_type.cc

@@ -53,5 +53,17 @@ bool Matrix::IsConstructible() const {
   return true;
 }
 
+uint32_t Matrix::Size() const {
+  return column_type_->Align() * columns();
+}
+
+uint32_t Matrix::Align() const {
+  return column_type_->Align();
+}
+
+uint32_t Matrix::ColumnStride() const {
+  return column_type_->Align();
+}
+
 }  // namespace sem
 }  // namespace tint

src/sem/matrix_type.h

@@ -58,6 +58,16 @@ class Matrix : public Castable<Matrix, Type> {
   /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
   bool IsConstructible() const override;
 
+  /// @returns the size in bytes of the type. This may include tail padding.
+  uint32_t Size() const override;
+
+  /// @returns the alignment in bytes of the type. This may include tail
+  /// padding.
+  uint32_t Align() const override;
+
+  /// @returns the number of bytes between columns of the matrix
+  uint32_t ColumnStride() const;
+
  private:
   Type* const subtype_;
   Vector* const column_type_;

src/sem/sem_array_test.cc

@@ -27,7 +27,7 @@ TEST_F(ArrayTest, CreateSizedArray) {
   EXPECT_EQ(arr->ElemType(), &u32);
   EXPECT_EQ(arr->Count(), 2u);
   EXPECT_EQ(arr->Align(), 4u);
-  EXPECT_EQ(arr->SizeInBytes(), 8u);
+  EXPECT_EQ(arr->Size(), 8u);
   EXPECT_EQ(arr->Stride(), 32u);
   EXPECT_EQ(arr->ImplicitStride(), 16u);
   EXPECT_FALSE(arr->IsStrideImplicit());
@@ -40,7 +40,7 @@ TEST_F(ArrayTest, CreateRuntimeArray) {
   EXPECT_EQ(arr->ElemType(), &u32);
   EXPECT_EQ(arr->Count(), 0u);
   EXPECT_EQ(arr->Align(), 4u);
-  EXPECT_EQ(arr->SizeInBytes(), 8u);
+  EXPECT_EQ(arr->Size(), 8u);
   EXPECT_EQ(arr->Stride(), 32u);
   EXPECT_EQ(arr->ImplicitStride(), 32u);
   EXPECT_TRUE(arr->IsStrideImplicit());

src/sem/struct.cc

@@ -60,6 +60,14 @@ std::string Struct::type_name() const {
   return declaration_->type_name();
 }
 
+uint32_t Struct::Align() const {
+  return align_;
+}
+
+uint32_t Struct::Size() const {
+  return size_;
+}
+
 std::string Struct::FriendlyName(const SymbolTable& symbols) const {
   return symbols.NameFor(declaration_->name());
 }

src/sem/struct.h

@@ -86,13 +86,13 @@ class Struct : public Castable<Struct, Type> {
   /// @note this may differ from the alignment of a structure member of this
   /// structure type, if the member is annotated with the `[[align(n)]]`
   /// decoration.
-  uint32_t Align() const { return align_; }
+  uint32_t Align() const override;
 
   /// @returns the byte size of the structure
   /// @note this may differ from the size of a structure member of this
   /// structure type, if the member is annotated with the `[[size(n)]]`
   /// decoration.
-  uint32_t Size() const { return size_; }
+  uint32_t Size() const override;
 
   /// @returns the byte size of the members without the end of structure
   /// alignment padding

src/sem/type.cc

@@ -14,9 +14,6 @@
 
 #include "src/sem/type.h"
 
-#include "src/debug.h"
-#include "src/sem/array.h"
-#include "src/sem/atomic_type.h"
 #include "src/sem/bool_type.h"
 #include "src/sem/f32_type.h"
 #include "src/sem/i32_type.h"
@@ -24,7 +21,6 @@
 #include "src/sem/pointer_type.h"
 #include "src/sem/reference_type.h"
 #include "src/sem/sampler_type.h"
-#include "src/sem/struct.h"
 #include "src/sem/texture_type.h"
 #include "src/sem/u32_type.h"
 #include "src/sem/vector_type.h"
@@ -56,59 +52,12 @@ const Type* Type::UnwrapRef() const {
   return type;
 }
 
-void Type::GetDefaultAlignAndSize(uint32_t& align, uint32_t& size) const {
+uint32_t Type::Size() const {
-  TINT_ASSERT(Semantic, !As<Reference>());
+  return 0;
-  TINT_ASSERT(Semantic, !As<Pointer>());
+}
 
-  static constexpr uint32_t vector_size[] = {
+uint32_t Type::Align() const {
-      /* padding */ 0,
+  return 0;
-      /* padding */ 0,
-      /*vec2*/ 8,
-      /*vec3*/ 12,
-      /*vec4*/ 16,
-  };
-  static constexpr uint32_t vector_align[] = {
-      /* padding */ 0,
-      /* padding */ 0,
-      /*vec2*/ 8,
-      /*vec3*/ 16,
-      /*vec4*/ 16,
-  };
-
-  if (is_scalar()) {
-    // Note: Also captures booleans, but these are not host-shareable.
-    align = 4;
-    size = 4;
-    return;
-  }
-  if (auto* vec = As<Vector>()) {
-    TINT_ASSERT(Semantic, vec->Width() >= 2 && vec->Width() <= 4);
-    align = vector_align[vec->Width()];
-    size = vector_size[vec->Width()];
-    return;
-  }
-  if (auto* mat = As<Matrix>()) {
-    TINT_ASSERT(Semantic, mat->columns() >= 2 && mat->columns() <= 4);
-    TINT_ASSERT(Semantic, mat->rows() >= 2 && mat->rows() <= 4);
-    align = vector_align[mat->rows()];
-    size = vector_align[mat->rows()] * mat->columns();
-    return;
-  }
-  if (auto* s = As<Struct>()) {
-    align = s->Align();
-    size = s->Size();
-    return;
-  }
-  if (auto* a = As<Array>()) {
-    align = a->Align();
-    size = a->SizeInBytes();
-    return;
-  }
-  if (auto* a = As<Atomic>()) {
-    return a->Type()->GetDefaultAlignAndSize(align, size);
-  }
-
-  TINT_ASSERT(Semantic, false);
 }
 
 bool Type::IsConstructible() const {

src/sem/type.h

@@ -52,9 +52,14 @@ class Type : public Castable<Type, Node> {
   /// @returns the inner type if this is a reference, `this` otherwise
   const Type* UnwrapRef() const;
 
-  /// @param align the output default alignment in bytes for this type.
+  /// @returns the size in bytes of the type. This may include tail padding.
-  /// @param size the output default size in bytes for this type.
+  /// @note opaque types will return a size of 0.
-  void GetDefaultAlignAndSize(uint32_t& align, uint32_t& size) const;
+  virtual uint32_t Size() const;
+
+  /// @returns the alignment in bytes of the type. This may include tail
+  /// padding.
+  /// @note opaque types will return a size of 0.
+  virtual uint32_t Align() const;
 
   /// @returns true if constructible as per
   /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types

src/sem/u32_type.cc

@@ -39,5 +39,13 @@ bool U32::IsConstructible() const {
   return true;
 }
 
+uint32_t U32::Size() const {
+  return 4;
+}
+
+uint32_t U32::Align() const {
+  return 4;
+}
+
 }  // namespace sem
 }  // namespace tint

src/sem/u32_type.h

@@ -42,6 +42,12 @@ class U32 : public Castable<U32, Type> {
   /// @returns true if constructible as per
   /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
   bool IsConstructible() const override;
+
+  /// @returns the size in bytes of the type.
+  uint32_t Size() const override;
+
+  /// @returns the alignment in bytes of the type.
+  uint32_t Align() const override;
 };
 
 }  // namespace sem

src/sem/vector_type.cc

@@ -45,5 +45,37 @@ bool Vector::IsConstructible() const {
   return true;
 }
 
+uint32_t Vector::Size() const {
+  return SizeOf(width_);
+}
+
+uint32_t Vector::Align() const {
+  return AlignOf(width_);
+}
+
+uint32_t Vector::SizeOf(uint32_t width) {
+  switch (width) {
+    case 2:
+      return 8;
+    case 3:
+      return 12;
+    case 4:
+      return 16;
+  }
+  return 0;  // Unreachable
+}
+
+uint32_t Vector::AlignOf(uint32_t width) {
+  switch (width) {
+    case 2:
+      return 8;
+    case 3:
+      return 16;
+    case 4:
+      return 16;
+  }
+  return 0;  // Unreachable
+}
+
 }  // namespace sem
 }  // namespace tint

src/sem/vector_type.h

@@ -39,9 +39,6 @@ class Vector : public Castable<Vector, Type> {
   /// @returns the name for th type
   std::string type_name() const override;
 
-  /// @returns the width of the vector
-  uint32_t Width() const { return width_; }
-
   /// @param symbols the program's symbol table
   /// @returns the name for this type that closely resembles how it would be
   /// declared in WGSL.
@@ -51,6 +48,24 @@ class Vector : public Castable<Vector, Type> {
   /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
   bool IsConstructible() const override;
 
+  /// @returns the number of elements in the vector
+  uint32_t Width() const { return width_; }
+
+  /// @returns the size in bytes of the type. This may include tail padding.
+  uint32_t Size() const override;
+
+  /// @returns the alignment in bytes of the type. This may include tail
+  /// padding.
+  uint32_t Align() const override;
+
+  /// @param width the width of the vector
+  /// @returns the size in bytes of a vector of the given width.
+  static uint32_t SizeOf(uint32_t width);
+
+  /// @param width the width of the vector
+  /// @returns the alignment in bytes of a vector of the given width.
+  static uint32_t AlignOf(uint32_t width);
+
  private:
   Type const* const subtype_;
   uint32_t const width_;

src/transform/decompose_memory_access.cc

@@ -127,17 +127,6 @@ struct AtomicKey {
   };
 };
 
-/// @returns the size in bytes of a scalar
-uint32_t ScalarSize(const sem::Type*) {
-  // TODO(bclayton): Assumes 32-bit elements
-  return 4;
-}
-
-/// @returns the number of bytes between columns of the given matrix
-uint32_t MatrixColumnStride(const sem::Matrix* mat) {
-  return ScalarSize(mat->type()) * ((mat->rows() == 2) ? 2 : 4);
-}
-
 bool IntrinsicDataTypeFor(const sem::Type* ty,
                           DecomposeMemoryAccess::Intrinsic::DataType& out) {
   if (ty->Is<sem::I32>()) {
@@ -525,7 +514,7 @@ struct DecomposeMemoryAccess::State {
               auto* vec_ty = mat_ty->ColumnType();
               Symbol load = LoadFunc(buf_ty, vec_ty, var_user);
               for (uint32_t i = 0; i < mat_ty->columns(); i++) {
-                auto* offset = b.Add("offset", i * MatrixColumnStride(mat_ty));
+                auto* offset = b.Add("offset", i * mat_ty->ColumnStride());
                 values.emplace_back(b.Call(load, "buffer", offset));
               }
             } else if (auto* str = el_ty->As<sem::Struct>()) {
@@ -624,7 +613,7 @@ struct DecomposeMemoryAccess::State {
               auto* vec_ty = mat_ty->ColumnType();
               Symbol store = StoreFunc(buf_ty, vec_ty, var_user);
               for (uint32_t i = 0; i < mat_ty->columns(); i++) {
-                auto* offset = b.Add("offset", i * MatrixColumnStride(mat_ty));
+                auto* offset = b.Add("offset", i * mat_ty->ColumnStride());
                 auto* access = b.IndexAccessor("value", i);
                 auto* call = b.Call(store, "buffer", offset, access);
                 body.emplace_back(b.create<ast::CallStatement>(call));
@@ -845,7 +834,7 @@ void DecomposeMemoryAccess::Run(CloneContext& ctx, const DataMap&, DataMap&) {
           if (auto access = state.TakeAccess(accessor->structure())) {
             auto* vec_ty = access.type->As<sem::Vector>();
             auto* offset =
-                state.Mul(ScalarSize(vec_ty->type()), swizzle->Indices()[0]);
+                state.Mul(vec_ty->type()->Size(), swizzle->Indices()[0]);
             state.AddAccess(accessor, {
                                           access.var,
                                           state.Add(access.offset, offset),
@@ -882,7 +871,7 @@ void DecomposeMemoryAccess::Run(CloneContext& ctx, const DataMap&, DataMap&) {
         }
         if (auto* vec_ty = access.type->As<sem::Vector>()) {
           auto* offset =
-              state.Mul(ScalarSize(vec_ty->type()), accessor->idx_expr());
+              state.Mul(vec_ty->type()->Size(), accessor->idx_expr());
           state.AddAccess(accessor, {
                                         access.var,
                                         state.Add(access.offset, offset),
@@ -892,7 +881,7 @@ void DecomposeMemoryAccess::Run(CloneContext& ctx, const DataMap&, DataMap&) {
         }
         if (auto* mat_ty = access.type->As<sem::Matrix>()) {
           auto* offset =
-              state.Mul(MatrixColumnStride(mat_ty), accessor->idx_expr());
+              state.Mul(mat_ty->ColumnStride(), accessor->idx_expr());
           state.AddAccess(accessor, {
                                         access.var,
                                         state.Add(access.offset, offset),

src/transform/zero_init_workgroup_memory.cc

@@ -130,7 +130,7 @@ struct ZeroInitWorkgroupMemory::State {
     // we need to return true for these arrays.
     // See https://github.com/gpuweb/gpuweb/pull/1792
     return (cfg.init_arrays_with_loop_size_threshold != 0) &&
-           (array->SizeInBytes() >= cfg.init_arrays_with_loop_size_threshold);
+           (array->Size() >= cfg.init_arrays_with_loop_size_threshold);
   }
 };
 

src/writer/hlsl/generator_impl.cc

@@ -2970,21 +2970,21 @@ bool GeneratorImpl::EmitType(std::ostream& out,
   } else if (type->Is<sem::U32>()) {
     out << "uint";
   } else if (auto* vec = type->As<sem::Vector>()) {
-    auto size = vec->Width();
+    auto width = vec->Width();
-    if (vec->type()->Is<sem::F32>() && size >= 1 && size <= 4) {
+    if (vec->type()->Is<sem::F32>() && width >= 1 && width <= 4) {
-      out << "float" << size;
+      out << "float" << width;
-    } else if (vec->type()->Is<sem::I32>() && size >= 1 && size <= 4) {
+    } else if (vec->type()->Is<sem::I32>() && width >= 1 && width <= 4) {
-      out << "int" << size;
+      out << "int" << width;
-    } else if (vec->type()->Is<sem::U32>() && size >= 1 && size <= 4) {
+    } else if (vec->type()->Is<sem::U32>() && width >= 1 && width <= 4) {
-      out << "uint" << size;
+      out << "uint" << width;
-    } else if (vec->type()->Is<sem::Bool>() && size >= 1 && size <= 4) {
+    } else if (vec->type()->Is<sem::Bool>() && width >= 1 && width <= 4) {
-      out << "bool" << size;
+      out << "bool" << width;
     } else {
       out << "vector<";
       if (!EmitType(out, vec->type(), storage_class, access, "")) {
         return false;
       }
-      out << ", " << size << ">";
+      out << ", " << width << ">";
     }
   } else if (auto* atomic = type->As<sem::Atomic>()) {
     if (!EmitType(out, atomic->Type(), storage_class, access, name)) {