[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>
diff --git a/src/reader/spirv/function_composite_test.cc b/src/reader/spirv/function_composite_test.cc
index 8d5cb70..f3caa24 100644
--- a/src/reader/spirv/function_composite_test.cc
+++ b/src/reader/spirv/function_composite_test.cc
@@ -218,10 +218,10 @@
Variable{
x_1
none
- __struct_S
+ __alias_S__struct_S
{
TypeConstructor{
- __struct_S
+ __alias_S__struct_S
TypeConstructor{
__vec_2__f32
ScalarConstructor{50.000000}
diff --git a/src/reader/spirv/function_var_test.cc b/src/reader/spirv/function_var_test.cc
index 1f929a7..dd9f283 100644
--- a/src/reader/spirv/function_var_test.cc
+++ b/src/reader/spirv/function_var_test.cc
@@ -431,7 +431,108 @@
}
}
}
-)"));
+)")) << 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 @@
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 @@
}
}
}
-)"));
+)")) << 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
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc
index f5b896f..5d53d6d 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -274,10 +274,11 @@
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 @@
if (!RegisterTypes()) {
return false;
}
- if (!EmitAliasTypes()) {
- return false;
- }
if (!EmitModuleScopeVariables()) {
return false;
}
@@ -757,52 +755,46 @@
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
- // the SPIR-V module. This is valid because:
- //
- // - There are no back-references. OpTypeForwarddPointer is not supported
- // by the WebGPU shader programming model.
- // - Arrays are always sized by an OpConstant of scalar integral type.
- // WGSL currently doesn't have specialization constants.
- // crbug.com/32 tracks implementation in case they are added.
- for (auto& type_or_const : module_->types_values()) {
- const auto type_id = type_or_const.result_id();
- // We only care about struct, arrays, and runtime arrays.
- switch (type_or_const.opcode()) {
- case SpvOpTypeStruct:
- // The struct already got a name when the type was first registered.
- break;
- case SpvOpTypeRuntimeArray:
- // Runtime arrays are always decorated with ArrayStride so always get a
- // type alias.
- namer_.SuggestSanitizedName(type_id, "RTArr");
- break;
- case SpvOpTypeArray:
- // Only make a type aliase for arrays with decorations.
- 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());
+ const auto type_id = type_mgr_->GetId(type);
+
+ // We only care about struct, arrays, and runtime arrays.
+ switch (type->kind()) {
+ case spvtools::opt::analysis::Type::kStruct:
+ // The struct already got a name when the type was first registered.
+ break;
+ case spvtools::opt::analysis::Type::kRuntimeArray:
+ // Runtime arrays are always decorated with ArrayStride so always get a
+ // type alias.
+ namer_.SuggestSanitizedName(type_id, "RTArr");
+ break;
+ case spvtools::opt::analysis::Type::kArray:
+ // Only make a type aliase for arrays with decorations.
+ if (GetDecorationsFor(type_id).empty()) {
+ return;
+ }
+ namer_.SuggestSanitizedName(type_id, "Arr");
+ break;
+ default:
+ // Ignore constants, and any other types.
+ return;
}
- 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 @@
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 @@
Fail() << "ID " << id << " is not a registered instruction";
return {};
}
- auto* ast_type = ConvertType(inst->type_id());
- if (ast_type == nullptr) {
+ auto* original_ast_type = ConvertType(inst->type_id());
+ if (original_ast_type == nullptr) {
return {};
}
// TODO(dneto): Handle spec constants too?
@@ -906,6 +897,8 @@
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 @@
}
ast_components.emplace_back(std::move(ast_component.expr));
}
- return {ast_type, std::make_unique<ast::TypeConstructorExpression>(
- ast_type, std::move(ast_components))};
+ return {original_ast_type,
+ 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 @@
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 @@
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 @@
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;
diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h
index 8c66bcb..b235b1b 100644
--- a/src/reader/spirv/parser_impl.h
+++ b/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 @@
/// @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 @@
/// @returns true if parser is still successful.
bool EmitEntryPoints();
- /// Register Tint AST types for SPIR-V types.
- /// This is a no-op if the parser has already failed.
+ /// Register Tint AST types for SPIR-V types, including type aliases as
+ /// 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 @@
// 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.
diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc
index ef06c26..ffbe3cc 100644
--- a/src/reader/spirv/parser_impl_module_var_test.cc
+++ b/src/reader/spirv/parser_impl_module_var_test.cc
@@ -548,10 +548,10 @@
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 @@
}
}
}
- })"));
+ })"))
+ << module_str;
}
TEST_F(SpvParserTest, ModuleScopeVar_StructNullInitializer) {
@@ -570,16 +571,16 @@
%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 @@
}
}
}
- })"));
+ })"))
+ << module_str;
}
} // namespace
