From 884a4e2172efc278e093684b61552ef4084fe916 Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 12 May 2021 17:02:51 +0000 Subject: [PATCH] spirv-reader: Refactor decoration-getting for vars Bug: tint:508 Change-Id: Ib176eb5e0dfdd6901635a9dd627aa9c7316f0a80 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49301 Commit-Queue: David Neto Auto-Submit: David Neto Reviewed-by: Antonio Maiorano --- src/reader/spirv/parser_impl.cc | 63 ++++++++++++++++++--------------- src/reader/spirv/parser_impl.h | 15 ++++++++ 2 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 8b58fc306d..c8d3c64f1c 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -1365,32 +1365,44 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id, sc = ast::StorageClass::kNone; } + if (!ConvertDecorationsForVariable(id, &type, &decorations)) { + return nullptr; + } + + std::string name = namer_.Name(id); + return create(Source{}, builder_.Symbols().Register(name), sc, + type->Build(builder_), is_const, constructor, + decorations); +} + +bool ParserImpl::ConvertDecorationsForVariable( + uint32_t id, + const Type** type, + ast::DecorationList* decorations) { for (auto& deco : GetDecorationsFor(id)) { if (deco.empty()) { - Fail() << "malformed decoration on ID " << id << ": it is empty"; - return nullptr; + return Fail() << "malformed decoration on ID " << id << ": it is empty"; } if (deco[0] == SpvDecorationBuiltIn) { if (deco.size() == 1) { - Fail() << "malformed BuiltIn decoration on ID " << id - << ": has no operand"; - return nullptr; + return Fail() << "malformed BuiltIn decoration on ID " << id + << ": has no operand"; } const auto spv_builtin = static_cast(deco[1]); switch (spv_builtin) { case SpvBuiltInPointSize: special_builtins_[id] = spv_builtin; - return nullptr; + return false; // This is not an error case SpvBuiltInSampleId: case SpvBuiltInVertexIndex: case SpvBuiltInInstanceIndex: // The SPIR-V variable is likely to be signed (because GLSL // requires signed), but WGSL requires unsigned. Handle specially // so we always perform the conversion at load and store. - if (auto* forced_type = UnsignedTypeFor(type)) { + if (auto* forced_type = UnsignedTypeFor(*type)) { // Requires conversion and special handling in code generation. special_builtins_[id] = spv_builtin; - type = forced_type; + *type = forced_type; } break; case SpvBuiltInSampleMask: { @@ -1404,7 +1416,7 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id, "SampleMask must be an array of 1 element."; } special_builtins_[id] = spv_builtin; - type = ty_.U32(); + *type = ty_.U32(); break; } default: @@ -1412,43 +1424,38 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id, } auto ast_builtin = enum_converter_.ToBuiltin(spv_builtin); if (ast_builtin == ast::Builtin::kNone) { - return nullptr; + // A diagnostic has already been emitted. + return false; } - decorations.emplace_back( + decorations->emplace_back( create(Source{}, ast_builtin)); } if (deco[0] == SpvDecorationLocation) { if (deco.size() != 2) { - Fail() << "malformed Location decoration on ID " << id - << ": requires one literal operand"; - return nullptr; + return Fail() << "malformed Location decoration on ID " << id + << ": requires one literal operand"; } - decorations.emplace_back( + decorations->emplace_back( create(Source{}, deco[1])); } if (deco[0] == SpvDecorationDescriptorSet) { if (deco.size() == 1) { - Fail() << "malformed DescriptorSet decoration on ID " << id - << ": has no operand"; - return nullptr; + return Fail() << "malformed DescriptorSet decoration on ID " << id + << ": has no operand"; } - decorations.emplace_back(create(Source{}, deco[1])); + decorations->emplace_back( + create(Source{}, deco[1])); } if (deco[0] == SpvDecorationBinding) { if (deco.size() == 1) { - Fail() << "malformed Binding decoration on ID " << id - << ": has no operand"; - return nullptr; + return Fail() << "malformed Binding decoration on ID " << id + << ": has no operand"; } - decorations.emplace_back( + decorations->emplace_back( create(Source{}, deco[1])); } } - - std::string name = namer_.Name(id); - return create(Source{}, builder_.Symbols().Register(name), sc, - type->Build(builder_), is_const, constructor, - decorations); + return success(); } TypedExpression ParserImpl::MakeConstantExpression(uint32_t id) { diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h index 683df891fd..834c1a5345 100644 --- a/src/reader/spirv/parser_impl.h +++ b/src/reader/spirv/parser_impl.h @@ -200,6 +200,21 @@ class ParserImpl : Reader { DecorationList GetDecorationsForMember(uint32_t id, uint32_t member_index) const; + /// Converts SPIR-V decorations for the variable with the given ID. + /// Registers the IDs of variables that require special handling by code + /// generation. If the WGSL type differs from the store type for SPIR-V, + /// then the `type` parameter is updated. Returns false on failure (with + /// a diagnostic), or when the variable should not be emitted, e.g. for a + /// PointSize builtin. + /// @param id the ID of the SPIR-V variable + /// @param type the WGSL store type for the variable, which should be + /// prepopulatd + /// @param ast_decos the decoration list to populate + /// @returns false when the variable should not be emitted as a variable + bool ConvertDecorationsForVariable(uint32_t id, + const Type** type, + ast::DecorationList* ast_decos); + /// Converts a SPIR-V struct member decoration. If the decoration is /// recognized but deliberately dropped, then returns nullptr without a /// diagnostic. On failure, emits a diagnostic and returns nullptr.