spirv-reader: refactor handling pipeline IO decorations
This is in preparation for handling the "invariant" decoration.
Bug: tint:972
Change-Id: I17465946932ab37a32dfd3c477525649ab622c6f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/58580
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index e5ca12f..c618d3a 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -981,12 +981,10 @@
index_prefix.push_back(0);
for (int i = 0; i < static_cast<int>(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 @@
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 @@
}
}
-ast::Decoration* FunctionEmitter::SetLocation(ast::DecorationList* decos,
- ast::Decoration* replacement) {
- if (!replacement) {
- return nullptr;
- }
- for (auto*& deco : *decos) {
- if (deco->Is<ast::LocationDecoration>()) {
- // 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 @@
index_prefix.push_back(0);
for (int i = 0; i < static_cast<int>(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 @@
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 9d6a82e..1cfed62 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -473,17 +473,6 @@
/// @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 32d3f9f..81a28057 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -234,12 +234,14 @@
}
// @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 @@
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 @@
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 @@
create<ast::BuiltinDecoration>(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<ast::LocationDecoration>(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 @@
}
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 @@
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<ast::LocationDecoration>()) {
+ // 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<ast::InterpolateDecoration>(
- 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<ast::LocationDecoration>(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<ast::InterpolateDecoration>(
+ 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 @@
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<ast::LocationDecoration>(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 99b7cf7..7a69708 100644
--- a/src/reader/spirv/parser_impl.h
+++ b/src/reader/spirv/parser_impl.h
@@ -252,14 +252,25 @@
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 @@
/// @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.
