diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index e5ca12f00e..c618d3a105 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -981,12 +981,10 @@ bool FunctionEmitter::EmitPipelineInput(std::string var_name, index_prefix.push_back(0); for (int i = 0; i < static_cast(members.size()); ++i) { index_prefix.back() = i; - auto* location = parser_impl_.GetMemberLocation(*struct_type, i); - SetLocation(decos, location); ast::DecorationList member_decos(*decos); - if (!parser_impl_.ConvertInterpolationDecorations( + if (!parser_impl_.ConvertPipelineDecorations( struct_type, - parser_impl_.GetMemberInterpolationDecorations(*struct_type, i), + parser_impl_.GetMemberPipelineDecorations(*struct_type, i), &member_decos)) { return false; } @@ -996,7 +994,7 @@ bool FunctionEmitter::EmitPipelineInput(std::string var_name, return false; } // Copy the location as updated by nested expansion of the member. - SetLocation(decos, GetLocation(member_decos)); + parser_impl_.SetLocation(decos, GetLocation(member_decos)); } return success(); } @@ -1064,26 +1062,6 @@ void FunctionEmitter::IncrementLocation(ast::DecorationList* decos) { } } -ast::Decoration* FunctionEmitter::SetLocation(ast::DecorationList* decos, - ast::Decoration* replacement) { - if (!replacement) { - return nullptr; - } - for (auto*& deco : *decos) { - if (deco->Is()) { - // Replace this location decoration with the replacement. - // The old one doesn't leak because it's kept in the builder's AST node - // list. - ast::Decoration* result = deco; - deco = replacement; - return result; - } - } - // The list didn't have a location. Add it. - decos->push_back(replacement); - return nullptr; -} - ast::Decoration* FunctionEmitter::GetLocation( const ast::DecorationList& decos) { for (auto* const& deco : decos) { @@ -1141,12 +1119,10 @@ bool FunctionEmitter::EmitPipelineOutput(std::string var_name, index_prefix.push_back(0); for (int i = 0; i < static_cast(members.size()); ++i) { index_prefix.back() = i; - auto* location = parser_impl_.GetMemberLocation(*struct_type, i); - SetLocation(decos, location); ast::DecorationList member_decos(*decos); - if (!parser_impl_.ConvertInterpolationDecorations( + if (!parser_impl_.ConvertPipelineDecorations( struct_type, - parser_impl_.GetMemberInterpolationDecorations(*struct_type, i), + parser_impl_.GetMemberPipelineDecorations(*struct_type, i), &member_decos)) { return false; } @@ -1156,7 +1132,7 @@ bool FunctionEmitter::EmitPipelineOutput(std::string var_name, return false; } // Copy the location as updated by nested expansion of the member. - SetLocation(decos, GetLocation(member_decos)); + parser_impl_.SetLocation(decos, GetLocation(member_decos)); } return success(); } diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index 9d6a82e976..1cfed625a8 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -473,17 +473,6 @@ class FunctionEmitter { /// @param decos the decoration list to modify void IncrementLocation(ast::DecorationList* decos); - /// Updates the decoration list, placing a non-null location decoration into - /// the list, replacing an existing one if it exists. Does nothing if the - /// replacement is nullptr. - /// Assumes the list contains at most one Location decoration. - /// @param decos the decoration list to modify - /// @param replacement the location decoration to place into the list - /// @returns the location decoration that was replaced, if one was replaced, - /// or null otherwise. - ast::Decoration* SetLocation(ast::DecorationList* decos, - ast::Decoration* replacement); - /// Returns the Location dcoration, if it exists. /// @param decos the list of decorations to search /// @returns the Location decoration, or nullptr if it doesn't exist diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 32d3f9f603..81a280578b 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -234,12 +234,14 @@ bool AssumesResultSignednessMatchesFirstOperand(GLSLstd450 extended_opcode) { } // @param a SPIR-V decoration -// @return true when the given decoration is an interpolation decoration. -bool IsInterpolationDecoration(const Decoration& deco) { +// @return true when the given decoration is a pipeline decoration other than a +// bulitin variable. +bool IsPipelineDecoration(const Decoration& deco) { if (deco.size() < 1) { return false; } switch (deco[0]) { + case SpvDecorationLocation: case SpvDecorationFlat: case SpvDecorationNoPerspective: case SpvDecorationCentroid: @@ -1115,31 +1117,22 @@ const Type* ParserImpl::ConvertType( bool is_non_writable = false; ast::DecorationList ast_member_decorations; for (auto& decoration : GetDecorationsForMember(type_id, member_index)) { - switch (decoration[0]) { - case SpvDecorationNonWritable: - - // WGSL doesn't represent individual members as non-writable. Instead, - // apply the ReadOnly access control to the containing struct if all - // the members are non-writable. - is_non_writable = true; - break; - case SpvDecorationLocation: - case SpvDecorationFlat: - case SpvDecorationNoPerspective: - case SpvDecorationCentroid: - case SpvDecorationSample: - // IO decorations are handled when emitting the entry point. - break; - default: { - auto* ast_member_decoration = - ConvertMemberDecoration(type_id, member_index, decoration); - if (!success_) { - return nullptr; - } - if (ast_member_decoration) { - ast_member_decorations.push_back(ast_member_decoration); - } - break; + if (IsPipelineDecoration(decoration)) { + // IO decorations are handled when emitting the entry point. + continue; + } else if (decoration[0] == SpvDecorationNonWritable) { + // WGSL doesn't represent individual members as non-writable. Instead, + // apply the ReadOnly access control to the containing struct if all + // the members are non-writable. + is_non_writable = true; + } else { + auto* ast_member_decoration = + ConvertMemberDecoration(type_id, member_index, decoration); + if (!success_) { + return nullptr; + } + if (ast_member_decoration) { + ast_member_decorations.push_back(ast_member_decoration); } } } @@ -1622,7 +1615,7 @@ bool ParserImpl::ConvertDecorationsForVariable(uint32_t id, const Type** store_type, ast::DecorationList* decorations, bool transfer_pipeline_io) { - DecorationList interpolation_decorations; + DecorationList non_builtin_pipeline_decorations; for (auto& deco : GetDecorationsFor(id)) { if (deco.empty()) { return Fail() << "malformed decoration on ID " << id << ": it is empty"; @@ -1685,18 +1678,8 @@ bool ParserImpl::ConvertDecorationsForVariable(uint32_t id, create(Source{}, ast_builtin)); } } - if (deco[0] == SpvDecorationLocation) { - if (deco.size() != 2) { - return Fail() << "malformed Location decoration on ID " << id - << ": requires one literal operand"; - } - if (transfer_pipeline_io) { - decorations->emplace_back( - create(Source{}, deco[1])); - } - } - if (transfer_pipeline_io && IsInterpolationDecoration(deco)) { - interpolation_decorations.push_back(deco); + if (transfer_pipeline_io && IsPipelineDecoration(deco)) { + non_builtin_pipeline_decorations.push_back(deco); } if (deco[0] == SpvDecorationDescriptorSet) { if (deco.size() == 1) { @@ -1717,8 +1700,8 @@ bool ParserImpl::ConvertDecorationsForVariable(uint32_t id, } if (transfer_pipeline_io) { - if (!ConvertInterpolationDecorations(*store_type, interpolation_decorations, - decorations)) { + if (!ConvertPipelineDecorations( + *store_type, non_builtin_pipeline_decorations, decorations)) { return false; } } @@ -1726,62 +1709,95 @@ bool ParserImpl::ConvertDecorationsForVariable(uint32_t id, return success(); } -DecorationList ParserImpl::GetMemberInterpolationDecorations( +DecorationList ParserImpl::GetMemberPipelineDecorations( const Struct& struct_type, int member_index) { // Yes, I could have used std::copy_if or std::copy_if. DecorationList result; for (const auto& deco : GetDecorationsForMember( struct_id_for_symbol_[struct_type.name], member_index)) { - if (IsInterpolationDecoration(deco)) { + if (IsPipelineDecoration(deco)) { result.emplace_back(deco); } } return result; } -bool ParserImpl::ConvertInterpolationDecorations( - const Type* store_type, - const DecorationList& decorations, - ast::DecorationList* ast_decos) { +ast::Decoration* ParserImpl::SetLocation(ast::DecorationList* decos, + ast::Decoration* replacement) { + if (!replacement) { + return nullptr; + } + for (auto*& deco : *decos) { + if (deco->Is()) { + // Replace this location decoration with the replacement. + // The old one doesn't leak because it's kept in the builder's AST node + // list. + ast::Decoration* result = nullptr; + result = deco; + deco = replacement; + return result; // Assume there is only one such decoration. + } + } + // The list didn't have a location. Add it. + decos->push_back(replacement); + return nullptr; +} + +bool ParserImpl::ConvertPipelineDecorations(const Type* store_type, + const DecorationList& decorations, + ast::DecorationList* ast_decos) { bool has_interpolate_no_perspective = false; bool has_interpolate_sampling_centroid = false; bool has_interpolate_sampling_sample = false; for (const auto& deco : decorations) { - if (deco[0] == SpvDecorationFlat) { - // In WGSL, integral types are always flat, and so the decoration - // is never specified. - if (!store_type->IsIntegerScalarOrVector()) { - ast_decos->emplace_back(create( - Source{}, ast::InterpolationType::kFlat, - ast::InterpolationSampling::kNone)); - // Only one interpolate attribute is allowed. - return true; - } - } - if (deco[0] == SpvDecorationNoPerspective) { - if (store_type->IsIntegerScalarOrVector()) { - // This doesn't capture the array or struct case. - return Fail() << "NoPerspective is invalid on integral IO"; - } - has_interpolate_no_perspective = true; - } - if (deco[0] == SpvDecorationCentroid) { - if (store_type->IsIntegerScalarOrVector()) { - // This doesn't capture the array or struct case. - return Fail() - << "Centroid interpolation sampling is invalid on integral IO"; - } - has_interpolate_sampling_centroid = true; - } - if (deco[0] == SpvDecorationSample) { - if (store_type->IsIntegerScalarOrVector()) { - // This doesn't capture the array or struct case. - return Fail() - << "Sample interpolation sampling is invalid on integral IO"; - } - has_interpolate_sampling_sample = true; + TINT_ASSERT(Reader, deco.size() > 0); + switch (deco[0]) { + case SpvDecorationLocation: + if (deco.size() != 2) { + return Fail() << "malformed Location decoration on ID requires one " + "literal operand"; + } + SetLocation(ast_decos, + create(Source{}, deco[1])); + break; + case SpvDecorationFlat: + // In WGSL, integral types are always flat, and so the decoration + // is never specified. + if (!store_type->IsIntegerScalarOrVector()) { + ast_decos->emplace_back(create( + Source{}, ast::InterpolationType::kFlat, + ast::InterpolationSampling::kNone)); + // Only one interpolate attribute is allowed. + return true; + } + break; + case SpvDecorationNoPerspective: + if (store_type->IsIntegerScalarOrVector()) { + // This doesn't capture the array or struct case. + return Fail() << "NoPerspective is invalid on integral IO"; + } + has_interpolate_no_perspective = true; + break; + case SpvDecorationCentroid: + if (store_type->IsIntegerScalarOrVector()) { + // This doesn't capture the array or struct case. + return Fail() + << "Centroid interpolation sampling is invalid on integral IO"; + } + has_interpolate_sampling_centroid = true; + break; + case SpvDecorationSample: + if (store_type->IsIntegerScalarOrVector()) { + // This doesn't capture the array or struct case. + return Fail() + << "Sample interpolation sampling is invalid on integral IO"; + } + has_interpolate_sampling_sample = true; + break; + default: + break; } } @@ -2818,22 +2834,6 @@ std::string ParserImpl::GetMemberName(const Struct& struct_type, return namer_.GetMemberName(where->second, member_index); } -ast::Decoration* ParserImpl::GetMemberLocation(const Struct& struct_type, - int member_index) { - auto where = struct_id_for_symbol_.find(struct_type.name); - if (where == struct_id_for_symbol_.end()) { - Fail() << "no structure type registered for symbol"; - return nullptr; - } - const auto type_id = where->second; - for (auto& deco : GetDecorationsForMember(type_id, member_index)) { - if ((deco.size() == 2) && (deco[0] == SpvDecorationLocation)) { - return create(Source{}, deco[1]); - } - } - return nullptr; -} - WorkgroupSizeInfo::WorkgroupSizeInfo() = default; WorkgroupSizeInfo::~WorkgroupSizeInfo() = default; diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h index 99b7cf7952..7a697084ee 100644 --- a/src/reader/spirv/parser_impl.h +++ b/src/reader/spirv/parser_impl.h @@ -252,14 +252,25 @@ class ParserImpl : Reader { ast::DecorationList* ast_decos, bool transfer_pipeline_io); - /// Converts SPIR-V interpolation decorations into AST decorations. + /// Converts SPIR-V decorations for pipeline IO into AST decorations. /// @param store_type the store type for the variable or member /// @param decorations the SPIR-V interpolation decorations /// @param ast_decos the decoration list to populate. /// @returns false if conversion fails - bool ConvertInterpolationDecorations(const Type* store_type, - const DecorationList& decorations, - ast::DecorationList* ast_decos); + bool ConvertPipelineDecorations(const Type* store_type, + const DecorationList& decorations, + ast::DecorationList* ast_decos); + + /// Updates the decoration list, placing a non-null location decoration into + /// the list, replacing an existing one if it exists. Does nothing if the + /// replacement is nullptr. + /// Assumes the list contains at most one Location decoration. + /// @param decos the decoration list to modify + /// @param replacement the location decoration to place into the list + /// @returns the location decoration that was replaced, if one was replaced, + /// or null otherwise. + ast::Decoration* SetLocation(ast::DecorationList* decos, + ast::Decoration* replacement); /// Converts a SPIR-V struct member decoration. If the decoration is /// recognized but deliberately dropped, then returns nullptr without a @@ -393,19 +404,13 @@ class ParserImpl : Reader { /// @returns the field name std::string GetMemberName(const Struct& struct_type, int member_index); - /// Returns the location decoration, if any on a struct member. - /// @param struct_type the parser's structure type. - /// @param member_index the member index - /// @returns a newly created location node, or nullptr - ast::Decoration* GetMemberLocation(const Struct& struct_type, - int member_index); - - /// Returns the SPIR-V interpolation decorations, if any, on a struct member. + /// Returns the SPIR-V decorations for pipeline IO, if any, on a struct + /// member. /// @param struct_type the parser's structure type. /// @param member_index the member index /// @returns a list of SPIR-V decorations. - DecorationList GetMemberInterpolationDecorations(const Struct& struct_type, - int member_index); + DecorationList GetMemberPipelineDecorations(const Struct& struct_type, + int member_index); /// Creates an AST Variable node for a SPIR-V ID, including any attached /// decorations, unless it's an ignorable builtin variable.