[spirv-reader] Use type aliases pervasively

When a type alias is created, map the SPIR-V type ID to the
type alias, not the underlying type. Only unpack the alias as
needed when inspecting the content structure to make values.

Bug: tint:3
Change-Id: I11011ddd190d89c81d3323f684a5e13f17dde09d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/23582
Reviewed-by: dan sinclair <dsinclair@google.com>

src/reader/spirv/function_composite_test.cc

@@ -218,10 +218,10 @@ TEST_F(SpvParserTest_Composite_Construct, Struct) {
   Variable{
     x_1
     none
-    __struct_S
+    __alias_S__struct_S
     {
       TypeConstructor{
-        __struct_S
+        __alias_S__struct_S
         TypeConstructor{
           __vec_2__f32
           ScalarConstructor{50.000000}

src/reader/spirv/function_var_test.cc

@@ -431,7 +431,108 @@ TEST_F(SpvParserTest, EmitFunctionVariables_ArrayInitializer) {
     }
   }
 }
-)"));
+)")) << ToString(fe.ast_body());
+}
+
+TEST_F(SpvParserTest, EmitFunctionVariables_ArrayInitializer_AliasType) {
+  auto* p = parser(test::Assemble(
+      std::string("OpDecorate %arr2uint ArrayStride 16\n") + CommonTypes() + R"(
+     %ptr = OpTypePointer Function %arr2uint
+     %two = OpConstant %uint 2
+     %const = OpConstantComposite %arr2uint %uint_1 %two
+
+     %100 = OpFunction %void None %voidfn
+     %entry = OpLabel
+     %200 = OpVariable %ptr Function %const
+     OpReturn
+     OpFunctionEnd
+  )"));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_TRUE(fe.EmitFunctionVariables());
+
+  EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
+  Variable{
+    x_200
+    function
+    __alias_Arr__array__u32_2_16
+    {
+      TypeConstructor{
+        __alias_Arr__array__u32_2_16
+        ScalarConstructor{1}
+        ScalarConstructor{2}
+      }
+    }
+  }
+}
+)")) << ToString(fe.ast_body());
+}
+
+TEST_F(SpvParserTest, EmitFunctionVariables_ArrayInitializer_Null) {
+  auto* p = parser(test::Assemble(CommonTypes() + R"(
+     %ptr = OpTypePointer Function %arr2uint
+     %two = OpConstant %uint 2
+     %const = OpConstantNull %arr2uint
+
+     %100 = OpFunction %void None %voidfn
+     %entry = OpLabel
+     %200 = OpVariable %ptr Function %const
+     OpReturn
+     OpFunctionEnd
+  )"));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_TRUE(fe.EmitFunctionVariables());
+
+  EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
+  Variable{
+    x_200
+    function
+    __array__u32_2
+    {
+      TypeConstructor{
+        __array__u32_2
+        ScalarConstructor{0}
+        ScalarConstructor{0}
+      }
+    }
+  }
+}
+)")) << ToString(fe.ast_body());
+}
+
+TEST_F(SpvParserTest, EmitFunctionVariables_ArrayInitializer_AliasType_Null) {
+  auto* p = parser(test::Assemble(
+      std::string("OpDecorate %arr2uint ArrayStride 16\n") + CommonTypes() + R"(
+     %ptr = OpTypePointer Function %arr2uint
+     %two = OpConstant %uint 2
+     %const = OpConstantNull %arr2uint
+
+     %100 = OpFunction %void None %voidfn
+     %entry = OpLabel
+     %200 = OpVariable %ptr Function %const
+     OpReturn
+     OpFunctionEnd
+  )"));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_TRUE(fe.EmitFunctionVariables());
+
+  EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
+  Variable{
+    x_200
+    function
+    __alias_Arr__array__u32_2_16
+    {
+      TypeConstructor{
+        __alias_Arr__array__u32_2_16
+        ScalarConstructor{0}
+        ScalarConstructor{0}
+      }
+    }
+  }
+}
+)")) << ToString(fe.ast_body());
 }
 
 TEST_F(SpvParserTest, EmitFunctionVariables_StructInitializer) {
@@ -455,10 +556,10 @@ TEST_F(SpvParserTest, EmitFunctionVariables_StructInitializer) {
   Variable{
     x_200
     function
-    __struct_S
+    __alias_S__struct_S
     {
       TypeConstructor{
-        __struct_S
+        __alias_S__struct_S
         ScalarConstructor{1}
         ScalarConstructor{1.500000}
         TypeConstructor{
@@ -470,7 +571,46 @@ TEST_F(SpvParserTest, EmitFunctionVariables_StructInitializer) {
     }
   }
 }
-)"));
+)")) << ToString(fe.ast_body());
+}
+
+TEST_F(SpvParserTest, EmitFunctionVariables_StructInitializer_Null) {
+  auto* p = parser(test::Assemble(CommonTypes() + R"(
+     %ptr = OpTypePointer Function %strct
+     %two = OpConstant %uint 2
+     %arrconst = OpConstantComposite %arr2uint %uint_1 %two
+     %const = OpConstantNull %strct
+
+     %100 = OpFunction %void None %voidfn
+     %entry = OpLabel
+     %200 = OpVariable %ptr Function %const
+     OpReturn
+     OpFunctionEnd
+  )"));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_TRUE(fe.EmitFunctionVariables());
+
+  EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{
+  Variable{
+    x_200
+    function
+    __alias_S__struct_S
+    {
+      TypeConstructor{
+        __alias_S__struct_S
+        ScalarConstructor{0}
+        ScalarConstructor{0.000000}
+        TypeConstructor{
+          __array__u32_2
+          ScalarConstructor{0}
+          ScalarConstructor{0}
+        }
+      }
+    }
+  }
+}
+)")) << ToString(fe.ast_body());
 }
 
 }  // namespace

src/reader/spirv/parser_impl.cc

@@ -274,10 +274,11 @@ ast::type::Type* ParserImpl::ConvertType(uint32_t type_id) {
     return nullptr;
   }
 
-  auto save = [this, type_id](ast::type::Type* type) {
+  auto save = [this, type_id, spirv_type](ast::type::Type* type) {
     if (type != nullptr) {
       id_to_type_[type_id] = type;
     }
+    MaybeGenerateAlias(spirv_type);
     return type;
   };
 
@@ -438,9 +439,6 @@ bool ParserImpl::ParseInternalModuleExceptFunctions() {
   if (!RegisterTypes()) {
     return false;
   }
-  if (!EmitAliasTypes()) {
-    return false;
-  }
   if (!EmitModuleScopeVariables()) {
     return false;
   }
@@ -757,52 +755,46 @@ bool ParserImpl::RegisterTypes() {
   return success_;
 }
 
-bool ParserImpl::EmitAliasTypes() {
+void ParserImpl::MaybeGenerateAlias(const spvtools::opt::analysis::Type* type) {
   if (!success_) {
-    return false;
+    return;
   }
-  // The algorithm here emits type definitions in the order presented in
+  const auto type_id = type_mgr_->GetId(type);
-  // the SPIR-V module.  This is valid because:
+
-  //
+  // We only care about struct, arrays, and runtime arrays.
-  // - There are no back-references.  OpTypeForwarddPointer is not supported
+  switch (type->kind()) {
-  //   by the WebGPU shader programming model.
+    case spvtools::opt::analysis::Type::kStruct:
-  // - Arrays are always sized by an OpConstant of scalar integral type.
+      // The struct already got a name when the type was first registered.
-  //   WGSL currently doesn't have specialization constants.
+      break;
-  //   crbug.com/32 tracks implementation in case they are added.
+    case spvtools::opt::analysis::Type::kRuntimeArray:
-  for (auto& type_or_const : module_->types_values()) {
+      // Runtime arrays are always decorated with ArrayStride so always get a
-    const auto type_id = type_or_const.result_id();
+      // type alias.
-    // We only care about struct, arrays, and runtime arrays.
+      namer_.SuggestSanitizedName(type_id, "RTArr");
-    switch (type_or_const.opcode()) {
+      break;
-      case SpvOpTypeStruct:
+    case spvtools::opt::analysis::Type::kArray:
-        // The struct already got a name when the type was first registered.
+      // Only make a type aliase for arrays with decorations.
-        break;
+      if (GetDecorationsFor(type_id).empty()) {
-      case SpvOpTypeRuntimeArray:
+        return;
-        // Runtime arrays are always decorated with ArrayStride so always get a
+      }
-        // type alias.
+      namer_.SuggestSanitizedName(type_id, "Arr");
-        namer_.SuggestSanitizedName(type_id, "RTArr");
+      break;
-        break;
+    default:
-      case SpvOpTypeArray:
+      // Ignore constants, and any other types.
-        // Only make a type aliase for arrays with decorations.
+      return;
-        if (GetDecorationsFor(type_id).empty()) {
-          continue;
-        }
-        namer_.SuggestSanitizedName(type_id, "Arr");
-        break;
-      default:
-        // Ignore constants, and any other types.
-        continue;
-    }
-    auto* ast_underlying_type = id_to_type_[type_id];
-    if (ast_underlying_type == nullptr) {
-      Fail() << "internal error: no type registered for SPIR-V ID: " << type_id;
-      return false;
-    }
-    const auto name = namer_.GetName(type_id);
-    auto* ast_type = ctx_.type_mgr().Get(
-        std::make_unique<ast::type::AliasType>(name, ast_underlying_type));
-    ast_module_.AddAliasType(ast_type->AsAlias());
   }
-  return success_;
+  auto* ast_underlying_type = id_to_type_[type_id];
+  if (ast_underlying_type == nullptr) {
+    Fail() << "internal error: no type registered for SPIR-V ID: " << type_id;
+    return;
+  }
+  const auto name = namer_.GetName(type_id);
+  auto* ast_alias_type = ctx_.type_mgr()
+                             .Get(std::make_unique<ast::type::AliasType>(
+                                 name, ast_underlying_type))
+                             ->AsAlias();
+  // Record this new alias as the AST type for this SPIR-V ID.
+  id_to_type_[type_id] = ast_alias_type;
+  ast_module_.AddAliasType(ast_alias_type);
 }
 
 bool ParserImpl::EmitModuleScopeVariables() {
@@ -853,7 +845,6 @@ std::unique_ptr<ast::Variable> ParserImpl::MakeVariable(uint32_t id,
     Fail() << "internal error: can't make ast::Variable for null type";
     return nullptr;
   }
-
   auto ast_var = std::make_unique<ast::Variable>(namer_.Name(id), sc, type);
 
   ast::VariableDecorationList ast_decorations;
@@ -895,8 +886,8 @@ TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) {
     Fail() << "ID " << id << " is not a registered instruction";
     return {};
   }
-  auto* ast_type = ConvertType(inst->type_id());
+  auto* original_ast_type = ConvertType(inst->type_id());
-  if (ast_type == nullptr) {
+  if (original_ast_type == nullptr) {
     return {};
   }
   // TODO(dneto): Handle spec constants too?
@@ -906,6 +897,8 @@ TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) {
     return {};
   }
 
+  auto* ast_type = original_ast_type->UnwrapAliasesIfNeeded();
+
   // TODO(dneto): Note: NullConstant for int, uint, float map to a regular 0.
   // So canonicalization should map that way too.
   // Currently "null<type>" is missing from the WGSL parser.
@@ -955,12 +948,13 @@ TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) {
       }
       ast_components.emplace_back(std::move(ast_component.expr));
     }
-    return {ast_type, std::make_unique<ast::TypeConstructorExpression>(
+    return {original_ast_type,
-                          ast_type, std::move(ast_components))};
+            std::make_unique<ast::TypeConstructorExpression>(
+                original_ast_type, std::move(ast_components))};
   }
   auto* spirv_null_const = spirv_const->AsNullConstant();
   if (spirv_null_const != nullptr) {
-    return {ast_type, MakeNullValue(ast_type)};
+    return {original_ast_type, MakeNullValue(original_ast_type)};
   }
   Fail() << "Unhandled constant type " << inst->type_id() << " for value ID "
          << id;
@@ -979,6 +973,9 @@ std::unique_ptr<ast::Expression> ParserImpl::MakeNullValue(
     return nullptr;
   }
 
+  auto* original_type = type;
+  type = type->UnwrapAliasesIfNeeded();
+
   if (type->IsBool()) {
     return std::make_unique<ast::ScalarConstructorExpression>(
         std::make_unique<ast::BoolLiteral>(type, false));
@@ -1024,7 +1021,7 @@ std::unique_ptr<ast::Expression> ParserImpl::MakeNullValue(
       ast_components.emplace_back(MakeNullValue(arr_ty->type()));
     }
     return std::make_unique<ast::TypeConstructorExpression>(
-        type, std::move(ast_components));
+        original_type, std::move(ast_components));
   }
   if (type->IsStruct()) {
     auto* struct_ty = type->AsStruct();
@@ -1033,7 +1030,7 @@ std::unique_ptr<ast::Expression> ParserImpl::MakeNullValue(
       ast_components.emplace_back(MakeNullValue(member->type()));
     }
     return std::make_unique<ast::TypeConstructorExpression>(
-        type, std::move(ast_components));
+        original_type, std::move(ast_components));
   }
   Fail() << "can't make null value for type: " << type->type_name();
   return nullptr;

src/reader/spirv/parser_impl.h

@@ -35,6 +35,7 @@
 #include "src/ast/import.h"
 #include "src/ast/module.h"
 #include "src/ast/struct_member_decoration.h"
+#include "src/ast/type/alias_type.h"
 #include "src/ast/type/type.h"
 #include "src/reader/reader.h"
 #include "src/reader/spirv/enum_converter.h"
@@ -138,6 +139,16 @@ class ParserImpl : Reader {
   /// @returns a Tint type, or nullptr
   ast::type::Type* ConvertType(uint32_t type_id);
 
+  /// Emits an alias type declaration for the given type, if necessary, and
+  /// also updates the mapping of the SPIR-V type ID to the alias type.
+  /// Do so for the types requiring user-specified names:
+  /// - struct types
+  /// - decorated arrays and runtime arrays
+  /// TODO(dneto): I expect images and samplers to require names as well.
+  /// This is a no-op if the parser has already failed.
+  /// @param type the type that might get an alias
+  void MaybeGenerateAlias(const spvtools::opt::analysis::Type* type);
+
   /// @returns the fail stream object
   FailStream& fail_stream() { return fail_stream_; }
   /// @returns the namer object
@@ -209,19 +220,11 @@ class ParserImpl : Reader {
   /// @returns true if parser is still successful.
   bool EmitEntryPoints();
 
-  /// Register Tint AST types for SPIR-V types.
+  /// Register Tint AST types for SPIR-V types, including type aliases as
-  /// This is a no-op if the parser has already failed.
+  /// needed.  This is a no-op if the parser has already failed.
   /// @returns true if parser is still successful.
   bool RegisterTypes();
 
-  /// Emit type alias declarations for types requiring user-specified names:
-  /// - struct types
-  /// - decorated arrays and runtime arrays
-  /// TODO(dneto): I expect images and samplers to require names as well.
-  /// This is a no-op if the parser has already failed.
-  /// @returns true if parser is still successful.
-  bool EmitAliasTypes();
-
   /// Emits module-scope variables.
   /// This is a no-op if the parser has already failed.
   /// @returns true if parser is still successful.
@@ -377,7 +380,7 @@ class ParserImpl : Reader {
   // sets.
   std::unordered_set<uint32_t> glsl_std_450_imports_;
 
-  // Maps a SPIR-V type ID to a Tint type.
+  // Maps a SPIR-V type ID to the corresponding Tint type.
   std::unordered_map<uint32_t, ast::type::Type*> id_to_type_;
 
   // Maps an unsigned type corresponding to the given signed type.

src/reader/spirv/parser_impl_module_var_test.cc

@@ -548,10 +548,10 @@ TEST_F(SpvParserTest, ModuleScopeVar_StructInitializer) {
   EXPECT_THAT(module_str, HasSubstr(R"(Variable{
     x_200
     private
-    __struct_S
+    __alias_S__struct_S
     {
       TypeConstructor{
-        __struct_S
+        __alias_S__struct_S
         ScalarConstructor{1}
         ScalarConstructor{1.500000}
         TypeConstructor{
@@ -561,7 +561,8 @@ TEST_F(SpvParserTest, ModuleScopeVar_StructInitializer) {
         }
       }
     }
-  })"));
+  })"))
+      << module_str;
 }
 
 TEST_F(SpvParserTest, ModuleScopeVar_StructNullInitializer) {
@@ -570,16 +571,16 @@ TEST_F(SpvParserTest, ModuleScopeVar_StructNullInitializer) {
      %const = OpConstantNull %strct
      %200 = OpVariable %ptr Private %const
   )"));
-  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
   EXPECT_TRUE(p->error().empty());
   const auto module_str = p->module().to_str();
   EXPECT_THAT(module_str, HasSubstr(R"(Variable{
     x_200
     private
-    __struct_S
+    __alias_S__struct_S
     {
       TypeConstructor{
-        __struct_S
+        __alias_S__struct_S
         ScalarConstructor{0}
         ScalarConstructor{0.000000}
         TypeConstructor{
@@ -589,7 +590,8 @@ TEST_F(SpvParserTest, ModuleScopeVar_StructNullInitializer) {
         }
       }
     }
-  })"));
+  })"))
+      << module_str;
 }
 
 }  // namespace