From 576ba1c4939288fdc1ae723f173d155a63c7c8e7 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 27 Apr 2023 17:58:25 +0000 Subject: [PATCH] tint: Add StructMember attributes to sem. Removes the need to examine AST attributes. Change-Id: Iaaa6b10fd56baf732057c4c3960c1bfc5bbdeaa6 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/129621 Reviewed-by: Dan Sinclair Commit-Queue: Dan Sinclair Kokoro: Kokoro --- src/tint/BUILD.gn | 1 + src/tint/builtin/interpolation.h | 33 +++++ src/tint/inspector/inspector.cc | 4 +- src/tint/resolver/builtin_structs.cc | 2 +- src/tint/resolver/inferred_type_test.cc | 11 +- src/tint/resolver/resolver.cc | 52 ++++++-- src/tint/resolver/resolver.h | 3 +- .../struct_pipeline_stage_use_test.cc | 4 +- src/tint/resolver/validator.cc | 8 +- src/tint/sem/struct.cc | 4 +- src/tint/sem/struct.h | 4 +- .../transform/canonicalize_entry_point_io.cc | 8 +- .../truncate_interstage_variables.cc | 16 +-- src/tint/transform/vertex_pulling.cc | 4 +- src/tint/type/struct.cc | 6 +- src/tint/type/struct.h | 23 +++- src/tint/type/struct_test.cc | 13 +- src/tint/type/type_test.cc | 6 +- src/tint/writer/glsl/generator_impl.h | 2 +- src/tint/writer/hlsl/generator_impl.cc | 103 ++++++--------- src/tint/writer/hlsl/generator_impl.h | 2 +- src/tint/writer/msl/generator_impl.cc | 123 ++++++------------ src/tint/writer/msl/generator_impl.h | 2 +- 23 files changed, 223 insertions(+), 211 deletions(-) create mode 100644 src/tint/builtin/interpolation.h diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index 70b3d97dbc..56c2031650 100644 --- a/src/tint/BUILD.gn +++ b/src/tint/BUILD.gn @@ -747,6 +747,7 @@ libtint_source_set("libtint_builtins_src") { "builtin/extension.h", "builtin/function.cc", "builtin/function.h", + "builtin/interpolation.h", "builtin/interpolation_sampling.cc", "builtin/interpolation_sampling.h", "builtin/interpolation_type.cc", diff --git a/src/tint/builtin/interpolation.h b/src/tint/builtin/interpolation.h new file mode 100644 index 0000000000..ad73c30f50 --- /dev/null +++ b/src/tint/builtin/interpolation.h @@ -0,0 +1,33 @@ +// Copyright 2023 The Tint Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef SRC_TINT_BUILTIN_INTERPOLATION_H_ +#define SRC_TINT_BUILTIN_INTERPOLATION_H_ + +#include "src/tint/builtin/interpolation_sampling.h" +#include "src/tint/builtin/interpolation_type.h" + +namespace tint::builtin { + +/// The values of an `@interpolate` attribute +struct Interpolation { + /// The first argument of a `@interpolate` attribute + builtin::InterpolationType type = builtin::InterpolationType::kUndefined; + /// The second argument of a `@interpolate` attribute + builtin::InterpolationSampling sampling = builtin::InterpolationSampling::kUndefined; +}; + +} // namespace tint::builtin + +#endif // SRC_TINT_BUILTIN_INTERPOLATION_H_ diff --git a/src/tint/inspector/inspector.cc b/src/tint/inspector/inspector.cc index 246943d6b4..adafe3acb8 100644 --- a/src/tint/inspector/inspector.cc +++ b/src/tint/inspector/inspector.cc @@ -621,8 +621,8 @@ void Inspector::AddEntryPointInOutVariables(std::string name, // Recurse into members. for (auto* member : struct_ty->Members()) { AddEntryPointInOutVariables(name + "." + member->Name().Name(), member->Type(), - member->Declaration()->attributes, member->Location(), - variables); + member->Declaration()->attributes, + member->Attributes().location, variables); } return; } diff --git a/src/tint/resolver/builtin_structs.cc b/src/tint/resolver/builtin_structs.cc index ce4f38c7d9..f035f0c7e7 100644 --- a/src/tint/resolver/builtin_structs.cc +++ b/src/tint/resolver/builtin_structs.cc @@ -53,7 +53,7 @@ sem::Struct* BuildStruct(ProgramBuilder& b, /* offset */ offset, /* align */ align, /* size */ size, - /* location */ std::nullopt)); + /* attributes */ type::StructMemberAttributes{})); offset += size; } uint32_t size_without_padding = offset; diff --git a/src/tint/resolver/inferred_type_test.cc b/src/tint/resolver/inferred_type_test.cc index ab2478bb2e..2fb844605e 100644 --- a/src/tint/resolver/inferred_type_test.cc +++ b/src/tint/resolver/inferred_type_test.cc @@ -150,11 +150,12 @@ TEST_F(ResolverInferredTypeTest, InferStruct_Pass) { auto* member = Member("x", ty.i32()); auto* str = Structure("S", utils::Vector{member}); - auto* expected_type = create( - str, str->source, str->name->symbol, - utils::Vector{create(member, member->source, member->name->symbol, - create(), 0u, 0u, 0u, 4u, std::nullopt)}, - 0u, 4u, 4u); + auto* expected_type = + create(str, str->source, str->name->symbol, + utils::Vector{create( + member, member->source, member->name->symbol, create(), + 0u, 0u, 0u, 4u, type::StructMemberAttributes{})}, + 0u, 4u, 4u); auto* ctor_expr = Call(ty.Of(str)); diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 638cac0cbd..322b3ee31e 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -3721,14 +3721,22 @@ bool Resolver::StrideAttribute(const ast::StrideAttribute*) { return true; } -bool Resolver::InterpolateAttribute(const ast::InterpolateAttribute* attr) { - if (!InterpolationType(attr->type)) { - return false; +utils::Result Resolver::InterpolateAttribute( + const ast::InterpolateAttribute* attr) { + builtin::Interpolation out; + auto* type = InterpolationType(attr->type); + if (!type) { + return utils::Failure; } - if (attr->sampling && !InterpolationSampling(attr->sampling)) { - return false; + out.type = type->Value(); + if (attr->sampling) { + auto* sampling = InterpolationSampling(attr->sampling); + if (!sampling) { + return utils::Failure; + } + out.sampling = sampling->Value(); } - return true; + return out; } bool Resolver::InternalAttribute(const ast::InternalAttribute* attr) { @@ -4021,7 +4029,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { bool has_offset_attr = false; bool has_align_attr = false; bool has_size_attr = false; - std::optional location; + type::StructMemberAttributes attributes; for (auto* attribute : member->attributes) { Mark(attribute); bool ok = Switch( @@ -4122,12 +4130,32 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { if (!value) { return false; } - location = value.Get(); + attributes.location = value.Get(); + return true; + }, + [&](const ast::BuiltinAttribute* attr) { + auto value = BuiltinAttribute(attr); + if (!value) { + return false; + } + attributes.builtin = value.Get(); + return true; + }, + [&](const ast::InterpolateAttribute* attr) { + auto value = InterpolateAttribute(attr); + if (!value) { + return false; + } + attributes.interpolation = value.Get(); + return true; + }, + [&](const ast::InvariantAttribute* attr) { + if (!InvariantAttribute(attr)) { + return false; + } + attributes.invariant = true; return true; }, - [&](const ast::BuiltinAttribute* attr) -> bool { return BuiltinAttribute(attr); }, - [&](const ast::InterpolateAttribute* attr) { return InterpolateAttribute(attr); }, - [&](const ast::InvariantAttribute* attr) { return InvariantAttribute(attr); }, [&](const ast::StrideAttribute* attr) { if (validator_.IsValidationEnabled( member->attributes, ast::DisabledValidation::kIgnoreStrideAttribute)) { @@ -4163,7 +4191,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { auto* sem_member = builder_->create( member, member->source, member->name->symbol, type, static_cast(sem_members.Length()), static_cast(offset), - static_cast(align), static_cast(size), location); + static_cast(align), static_cast(size), attributes); builder_->Sem().Add(member, sem_member); sem_members.Push(sem_member); diff --git a/src/tint/resolver/resolver.h b/src/tint/resolver/resolver.h index b26ad96fd1..7432f286ca 100644 --- a/src/tint/resolver/resolver.h +++ b/src/tint/resolver/resolver.h @@ -354,7 +354,8 @@ class Resolver { /// Resolves the `@interpolate` attribute @p attr /// @returns true on success, false on failure - bool InterpolateAttribute(const ast::InterpolateAttribute* attr); + utils::Result InterpolateAttribute( + const ast::InterpolateAttribute* attr); /// Resolves the internal attribute @p attr /// @returns true on success, false on failure diff --git a/src/tint/resolver/struct_pipeline_stage_use_test.cc b/src/tint/resolver/struct_pipeline_stage_use_test.cc index 4707650af9..529f1a64e6 100644 --- a/src/tint/resolver/struct_pipeline_stage_use_test.cc +++ b/src/tint/resolver/struct_pipeline_stage_use_test.cc @@ -187,7 +187,7 @@ TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderParamLocationSet) { auto* sem = TypeOf(s)->As(); ASSERT_NE(sem, nullptr); ASSERT_EQ(1u, sem->Members().Length()); - EXPECT_EQ(3u, sem->Members()[0]->Location()); + EXPECT_EQ(3u, sem->Members()[0]->Attributes().location); } TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderReturnTypeViaAlias) { @@ -217,7 +217,7 @@ TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderReturnTypeLocationSet) { auto* sem = TypeOf(s)->As(); ASSERT_NE(sem, nullptr); ASSERT_EQ(1u, sem->Members().Length()); - EXPECT_EQ(3u, sem->Members()[0]->Location()); + EXPECT_EQ(3u, sem->Members()[0]->Attributes().location); } } // namespace diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index c1ba335902..fafcfdf4c9 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc @@ -1215,7 +1215,7 @@ bool Validator::EntryPoint(const sem::Function* func, ast::PipelineStage stage) if (!validate_entry_point_attributes_inner( member->Declaration()->attributes, member->Type(), member->Source(), param_or_ret, - /*is_struct_member*/ true, member->Location())) { + /*is_struct_member*/ true, member->Attributes().location)) { AddNote("while analyzing entry point '" + decl->name->symbol.Name() + "'", decl->source); return false; @@ -2105,9 +2105,9 @@ bool Validator::Structure(const sem::Struct* str, ast::PipelineStage stage) cons }, [&](const ast::LocationAttribute* location) { has_location = true; - TINT_ASSERT(Resolver, member->Location().has_value()); - if (!LocationAttribute(location, member->Location().value(), member->Type(), - locations, stage, member->Source())) { + TINT_ASSERT(Resolver, member->Attributes().location.has_value()); + if (!LocationAttribute(location, member->Attributes().location.value(), + member->Type(), locations, stage, member->Source())) { return false; } return true; diff --git a/src/tint/sem/struct.cc b/src/tint/sem/struct.cc index 413231ab71..138671ec9e 100644 --- a/src/tint/sem/struct.cc +++ b/src/tint/sem/struct.cc @@ -40,8 +40,8 @@ StructMember::StructMember(const ast::StructMember* declaration, uint32_t offset, uint32_t align, uint32_t size, - std::optional location) - : Base(source, name, type, index, offset, align, size, location), declaration_(declaration) {} + const type::StructMemberAttributes& attributes) + : Base(source, name, type, index, offset, align, size, attributes), declaration_(declaration) {} StructMember::~StructMember() = default; diff --git a/src/tint/sem/struct.h b/src/tint/sem/struct.h index 2b437af102..7362f0f9c0 100644 --- a/src/tint/sem/struct.h +++ b/src/tint/sem/struct.h @@ -84,7 +84,7 @@ class StructMember final : public utils::Castable location); + const type::StructMemberAttributes& attributes); /// Destructor ~StructMember() override; diff --git a/src/tint/transform/canonicalize_entry_point_io.cc b/src/tint/transform/canonicalize_entry_point_io.cc index 840f8ffbc7..3bcd8d17cc 100644 --- a/src/tint/transform/canonicalize_entry_point_io.cc +++ b/src/tint/transform/canonicalize_entry_point_io.cc @@ -380,8 +380,8 @@ struct CanonicalizeEntryPointIO::State { auto attributes = CloneShaderIOAttributes(member->Declaration()->attributes, do_interpolate); - auto* input_expr = - AddInput(name, member->Type(), member->Location(), std::move(attributes)); + auto* input_expr = AddInput(name, member->Type(), member->Attributes().location, + std::move(attributes)); inner_struct_values.Push(input_expr); } @@ -410,8 +410,8 @@ struct CanonicalizeEntryPointIO::State { CloneShaderIOAttributes(member->Declaration()->attributes, do_interpolate); // Extract the original structure member. - AddOutput(name, member->Type(), member->Location(), std::move(attributes), - ctx.dst->MemberAccessor(original_result, name)); + AddOutput(name, member->Type(), member->Attributes().location, + std::move(attributes), ctx.dst->MemberAccessor(original_result, name)); } } else if (!inner_ret_type->Is()) { auto attributes = diff --git a/src/tint/transform/truncate_interstage_variables.cc b/src/tint/transform/truncate_interstage_variables.cc index 33c5cf7889..1fc0050e03 100644 --- a/src/tint/transform/truncate_interstage_variables.cc +++ b/src/tint/transform/truncate_interstage_variables.cc @@ -83,6 +83,8 @@ Transform::ApplyResult TruncateInterstageVariables::Apply(const Program* src, auto* func_sem = sem.Get(func_ast); auto* str = func_sem->ReturnType()->As(); + // This transform is run after CanonicalizeEntryPointIO transform, + // So it is guaranteed that entry point inputs are already grouped in a struct. if (TINT_UNLIKELY(!str)) { TINT_ICE(Transform, ctx.dst->Diagnostics()) << "Entrypoint function return type is non-struct.\n" @@ -91,20 +93,14 @@ Transform::ApplyResult TruncateInterstageVariables::Apply(const Program* src, continue; } - // This transform is run after CanonicalizeEntryPointIO transform, - // So it is guaranteed that entry point inputs are already grouped in a struct. - const ast::Struct* struct_ty = str->Declaration(); - // A prepass to check if any interstage variable locations in the entry point needs // truncating. If not we don't really need to handle this entry point. utils::Hashset omit_members; - for (auto* member : struct_ty->members) { - if (ast::GetAttribute(member->attributes)) { - auto* m = sem.Get(member); - uint32_t location = m->Location().value(); - if (!data->interstage_locations.test(location)) { - omit_members.Add(m); + for (auto* member : str->Members()) { + if (auto location = member->Attributes().location) { + if (!data->interstage_locations.test(location.value())) { + omit_members.Add(member); } } } diff --git a/src/tint/transform/vertex_pulling.cc b/src/tint/transform/vertex_pulling.cc index 064be002f8..5f704de842 100644 --- a/src/tint/transform/vertex_pulling.cc +++ b/src/tint/transform/vertex_pulling.cc @@ -826,8 +826,8 @@ struct VertexPulling::State { auto* sem = src->Sem().Get(member); info.type = sem->Type(); - TINT_ASSERT(Transform, sem->Location().has_value()); - location_info[sem->Location().value()] = info; + TINT_ASSERT(Transform, sem->Attributes().location.has_value()); + location_info[sem->Attributes().location.value()] = info; has_locations = true; } else { auto* builtin_attr = ast::GetAttribute(member->attributes); diff --git a/src/tint/type/struct.cc b/src/tint/type/struct.cc index e5c8d4ce6d..58d58e6bc4 100644 --- a/src/tint/type/struct.cc +++ b/src/tint/type/struct.cc @@ -179,7 +179,7 @@ StructMember::StructMember(tint::Source source, uint32_t offset, uint32_t align, uint32_t size, - std::optional location) + const StructMemberAttributes& attributes) : source_(source), name_(name), type_(type), @@ -187,7 +187,7 @@ StructMember::StructMember(tint::Source source, offset_(offset), align_(align), size_(size), - location_(location) {} + attributes_(attributes) {} StructMember::~StructMember() = default; @@ -195,7 +195,7 @@ StructMember* StructMember::Clone(CloneContext& ctx) const { auto sym = ctx.dst.st->Register(name_.Name()); auto* ty = type_->Clone(ctx); return ctx.dst.mgr->Get(source_, sym, ty, index_, offset_, align_, size_, - location_); + attributes_); } } // namespace tint::type diff --git a/src/tint/type/struct.h b/src/tint/type/struct.h index ea459f50ec..fc214ba9ee 100644 --- a/src/tint/type/struct.h +++ b/src/tint/type/struct.h @@ -22,6 +22,7 @@ #include #include "src/tint/builtin/address_space.h" +#include "src/tint/builtin/interpolation.h" #include "src/tint/symbol.h" #include "src/tint/type/node.h" #include "src/tint/type/type.h" @@ -163,6 +164,18 @@ class Struct : public utils::Castable { utils::Vector concrete_types_; }; +/// Attributes that can be applied to the StructMember +struct StructMemberAttributes { + /// The value of a `@location` attribute + std::optional location; + /// The value of a `@builtin` attribute + std::optional builtin; + /// The values of a `@interpolate` attribute + std::optional interpolation; + /// True if the member was annotated with `@invariant` + bool invariant = false; +}; + /// StructMember holds the type information for structure members. class StructMember : public utils::Castable { public: @@ -174,7 +187,7 @@ class StructMember : public utils::Castable { /// @param offset the byte offset from the base of the structure /// @param align the byte alignment of the member /// @param size the byte size of the member - /// @param location the location attribute, if present + /// @param attributes the optional attributes StructMember(tint::Source source, Symbol name, const type::Type* type, @@ -182,7 +195,7 @@ class StructMember : public utils::Castable { uint32_t offset, uint32_t align, uint32_t size, - std::optional location); + const StructMemberAttributes& attributes); /// Destructor ~StructMember() override; @@ -215,8 +228,8 @@ class StructMember : public utils::Castable { /// @returns byte size uint32_t Size() const { return size_; } - /// @returns the location, if set - std::optional Location() const { return location_; } + /// @returns the optional attributes + const StructMemberAttributes& Attributes() const { return attributes_; } /// @param ctx the clone context /// @returns a clone of this struct member @@ -231,7 +244,7 @@ class StructMember : public utils::Castable { const uint32_t offset_; const uint32_t align_; const uint32_t size_; - const std::optional location_; + const StructMemberAttributes attributes_; }; } // namespace tint::type diff --git a/src/tint/type/struct_test.cc b/src/tint/type/struct_test.cc index 7bfb43770d..d7f68a15b1 100644 --- a/src/tint/type/struct_test.cc +++ b/src/tint/type/struct_test.cc @@ -101,10 +101,8 @@ TEST_F(TypeStructTest, Location) { auto* sem = p.Sem().Get(st); ASSERT_EQ(2u, sem->Members().Length()); - EXPECT_TRUE(sem->Members()[0]->Location().has_value()); - EXPECT_EQ(sem->Members()[0]->Location().value(), 1u); - - EXPECT_FALSE(sem->Members()[1]->Location().has_value()); + EXPECT_EQ(sem->Members()[0]->Attributes().location, 1u); + EXPECT_FALSE(sem->Members()[1]->Attributes().location.has_value()); } TEST_F(TypeStructTest, IsConstructable) { @@ -207,12 +205,15 @@ TEST_F(TypeStructTest, HasFixedFootprint) { } TEST_F(TypeStructTest, Clone) { + type::StructMemberAttributes attrs_location_2; + attrs_location_2.location = 2; + auto* s = create( Source{}, Sym("my_struct"), utils::Vector{create(Source{}, Sym("b"), create(create(), 3u), - 0u, 0u, 16u, 12u, std::optional{2}), + 0u, 0u, 16u, 12u, attrs_location_2), create(Source{}, Sym("a"), create(), 1u, 16u, 4u, 4u, - std::optional())}, + type::StructMemberAttributes{})}, 4u /* align */, 8u /* size */, 16u /* size_no_padding */); ProgramID id; diff --git a/src/tint/type/type_test.cc b/src/tint/type/type_test.cc index 1565e10902..a09ab85ad6 100644 --- a/src/tint/type/type_test.cc +++ b/src/tint/type/type_test.cc @@ -56,7 +56,7 @@ struct TypeTest : public TestHelper { /* offset */ 0u, /* align */ 4u, /* size */ 4u, - /* location */ std::nullopt), + /* attributes */ type::StructMemberAttributes{}), }, /* align*/ 4u, /* size*/ 4u, @@ -72,7 +72,7 @@ struct TypeTest : public TestHelper { /* offset */ 0u, /* align */ 4u, /* size */ 4u, - /* location */ std::nullopt), + /* attributes */ type::StructMemberAttributes{}), }, /* align*/ 4u, /* size*/ 4u, @@ -88,7 +88,7 @@ struct TypeTest : public TestHelper { /* offset */ 0u, /* align */ 4u, /* size */ 4u, - /* location */ std::nullopt), + /* attributes */ type::StructMemberAttributes{}), }, /* align*/ 4u, /* size*/ 4u, diff --git a/src/tint/writer/glsl/generator_impl.h b/src/tint/writer/glsl/generator_impl.h index 0698d4f64d..060ea39630 100644 --- a/src/tint/writer/glsl/generator_impl.h +++ b/src/tint/writer/glsl/generator_impl.h @@ -399,7 +399,7 @@ class GeneratorImpl : public TextGenerator { /// Handles generating a 'var' declaration /// @param var the variable to generate void EmitVar(const ast::Var* var); - /// Handles generating a function-scope 'let' declaration + /// Handles generating a 'let' declaration /// @param let the variable to generate void EmitLet(const ast::Let* let); /// Handles generating a module-scope 'let' declaration diff --git a/src/tint/writer/hlsl/generator_impl.cc b/src/tint/writer/hlsl/generator_impl.cc index 5cce6a147c..1d19efe656 100644 --- a/src/tint/writer/hlsl/generator_impl.cc +++ b/src/tint/writer/hlsl/generator_impl.cc @@ -4216,73 +4216,48 @@ bool GeneratorImpl::EmitStructType(TextBuffer* b, const sem::Struct* str) { auto* ty = mem->Type(); auto out = line(b); std::string pre, post; - if (auto* decl = mem->Declaration()) { - for (auto* attr : decl->attributes) { - if (attr->Is()) { - auto& pipeline_stage_uses = str->PipelineStageUses(); - if (TINT_UNLIKELY(pipeline_stage_uses.size() != 1)) { - TINT_ICE(Writer, diagnostics_) << "invalid entry point IO struct uses"; - } - auto loc = mem->Location().value(); - if (pipeline_stage_uses.count(type::PipelineStageUsage::kVertexInput)) { - post += " : TEXCOORD" + std::to_string(loc); - } else if (pipeline_stage_uses.count( - type::PipelineStageUsage::kVertexOutput)) { - post += " : TEXCOORD" + std::to_string(loc); - } else if (pipeline_stage_uses.count( - type::PipelineStageUsage::kFragmentInput)) { - post += " : TEXCOORD" + std::to_string(loc); - } else if (TINT_LIKELY(pipeline_stage_uses.count( - type::PipelineStageUsage::kFragmentOutput))) { - post += " : SV_Target" + std::to_string(loc); - } else { - TINT_ICE(Writer, diagnostics_) << "invalid use of location attribute"; - } - } else if (auto* builtin_attr = attr->As()) { - auto builtin = program_->Sem().Get(builtin_attr)->Value(); - auto name = builtin_to_attribute(builtin); - if (name.empty()) { - diagnostics_.add_error(diag::System::Writer, "unsupported builtin"); - return false; - } - post += " : " + name; - } else if (auto* interpolate = attr->As()) { - auto& sem = program_->Sem(); - auto i_type = - sem.Get>( - interpolate->type) - ->Value(); + auto& attributes = mem->Attributes(); - auto i_smpl = builtin::InterpolationSampling::kUndefined; - if (interpolate->sampling) { - i_smpl = - sem.Get>( - interpolate->sampling) - ->Value(); - } - - auto mod = interpolation_to_modifiers(i_type, i_smpl); - if (mod.empty()) { - diagnostics_.add_error(diag::System::Writer, - "unsupported interpolation"); - return false; - } - pre += mod; - - } else if (attr->Is()) { - // Note: `precise` is not exactly the same as `invariant`, but is - // stricter and therefore provides the necessary guarantees. - // See discussion here: https://github.com/gpuweb/gpuweb/issues/893 - pre += "precise "; - } else if (TINT_UNLIKELY((!attr->IsAnyOf()))) { - TINT_ICE(Writer, diagnostics_) - << "unhandled struct member attribute: " << attr->Name(); - return false; - } + if (auto location = attributes.location) { + auto& pipeline_stage_uses = str->PipelineStageUses(); + if (TINT_UNLIKELY(pipeline_stage_uses.size() != 1)) { + TINT_ICE(Writer, diagnostics_) << "invalid entry point IO struct uses"; } + if (pipeline_stage_uses.count(type::PipelineStageUsage::kVertexInput)) { + post += " : TEXCOORD" + std::to_string(location.value()); + } else if (pipeline_stage_uses.count(type::PipelineStageUsage::kVertexOutput)) { + post += " : TEXCOORD" + std::to_string(location.value()); + } else if (pipeline_stage_uses.count(type::PipelineStageUsage::kFragmentInput)) { + post += " : TEXCOORD" + std::to_string(location.value()); + } else if (TINT_LIKELY(pipeline_stage_uses.count( + type::PipelineStageUsage::kFragmentOutput))) { + post += " : SV_Target" + std::to_string(location.value()); + } else { + TINT_ICE(Writer, diagnostics_) << "invalid use of location attribute"; + } + } + if (auto builtin = attributes.builtin) { + auto name = builtin_to_attribute(builtin.value()); + if (name.empty()) { + diagnostics_.add_error(diag::System::Writer, "unsupported builtin"); + return false; + } + post += " : " + name; + } + if (auto interpolation = attributes.interpolation) { + auto mod = interpolation_to_modifiers(interpolation->type, interpolation->sampling); + if (mod.empty()) { + diagnostics_.add_error(diag::System::Writer, "unsupported interpolation"); + return false; + } + pre += mod; + } + if (attributes.invariant) { + // Note: `precise` is not exactly the same as `invariant`, but is + // stricter and therefore provides the necessary guarantees. + // See discussion here: https://github.com/gpuweb/gpuweb/issues/893 + pre += "precise "; } out << pre; diff --git a/src/tint/writer/hlsl/generator_impl.h b/src/tint/writer/hlsl/generator_impl.h index 103e355d9e..9a330df6ed 100644 --- a/src/tint/writer/hlsl/generator_impl.h +++ b/src/tint/writer/hlsl/generator_impl.h @@ -470,7 +470,7 @@ class GeneratorImpl : public TextGenerator { /// @param var the variable to generate /// @returns true if the variable was emitted bool EmitVar(const ast::Var* var); - /// Handles generating a function-scope 'let' declaration + /// Handles generating a 'let' declaration /// @param let the variable to generate /// @returns true if the variable was emitted bool EmitLet(const ast::Let* let); diff --git a/src/tint/writer/msl/generator_impl.cc b/src/tint/writer/msl/generator_impl.cc index 0435918537..c0ba336b68 100644 --- a/src/tint/writer/msl/generator_impl.cc +++ b/src/tint/writer/msl/generator_impl.cc @@ -2852,88 +2852,51 @@ bool GeneratorImpl::EmitStructType(TextBuffer* b, const sem::Struct* str) { out << " " << mem_name; // Emit attributes - if (auto* decl = mem->Declaration()) { - for (auto* attr : decl->attributes) { - bool ok = Switch( - attr, - [&](const ast::BuiltinAttribute* builtin_attr) { - auto builtin = program_->Sem().Get(builtin_attr)->Value(); - auto name = builtin_to_attribute(builtin); - if (name.empty()) { - diagnostics_.add_error(diag::System::Writer, "unknown builtin"); - return false; - } - out << " [[" << name << "]]"; - return true; - }, - [&](const ast::LocationAttribute*) { - auto& pipeline_stage_uses = str->PipelineStageUses(); - if (TINT_UNLIKELY(pipeline_stage_uses.size() != 1)) { - TINT_ICE(Writer, diagnostics_) << "invalid entry point IO struct uses"; - return false; - } + auto& attributes = mem->Attributes(); - uint32_t loc = mem->Location().value(); - if (pipeline_stage_uses.count(type::PipelineStageUsage::kVertexInput)) { - out << " [[attribute(" + std::to_string(loc) + ")]]"; - } else if (pipeline_stage_uses.count( - type::PipelineStageUsage::kVertexOutput)) { - out << " [[user(locn" + std::to_string(loc) + ")]]"; - } else if (pipeline_stage_uses.count( - type::PipelineStageUsage::kFragmentInput)) { - out << " [[user(locn" + std::to_string(loc) + ")]]"; - } else if (TINT_LIKELY(pipeline_stage_uses.count( - type::PipelineStageUsage::kFragmentOutput))) { - out << " [[color(" + std::to_string(loc) + ")]]"; - } else { - TINT_ICE(Writer, diagnostics_) << "invalid use of location decoration"; - return false; - } - return true; - }, - [&](const ast::InterpolateAttribute* interpolate) { - auto& sem = program_->Sem(); - auto i_type = - sem.Get>( - interpolate->type) - ->Value(); - - auto i_smpl = builtin::InterpolationSampling::kUndefined; - if (interpolate->sampling) { - i_smpl = - sem.Get>( - interpolate->sampling) - ->Value(); - } - - auto name = interpolation_to_attribute(i_type, i_smpl); - if (name.empty()) { - diagnostics_.add_error(diag::System::Writer, - "unknown interpolation attribute"); - return false; - } - out << " [[" << name << "]]"; - return true; - }, - [&](const ast::InvariantAttribute*) { - if (invariant_define_name_.empty()) { - invariant_define_name_ = UniqueIdentifier("TINT_INVARIANT"); - } - out << " " << invariant_define_name_; - return true; - }, - [&](const ast::StructMemberOffsetAttribute*) { return true; }, - [&](const ast::StructMemberAlignAttribute*) { return true; }, - [&](const ast::StructMemberSizeAttribute*) { return true; }, - [&](Default) { - TINT_ICE(Writer, diagnostics_) - << "unhandled struct member attribute: " << attr->Name(); - return false; - }); - if (!ok) { - return false; - } + if (auto builtin = attributes.builtin) { + auto name = builtin_to_attribute(builtin.value()); + if (name.empty()) { + diagnostics_.add_error(diag::System::Writer, "unknown builtin"); + return false; } + out << " [[" << name << "]]"; + } + + if (auto location = attributes.location) { + auto& pipeline_stage_uses = str->PipelineStageUses(); + if (TINT_UNLIKELY(pipeline_stage_uses.size() != 1)) { + TINT_ICE(Writer, diagnostics_) << "invalid entry point IO struct uses"; + return false; + } + + if (pipeline_stage_uses.count(type::PipelineStageUsage::kVertexInput)) { + out << " [[attribute(" + std::to_string(location.value()) + ")]]"; + } else if (pipeline_stage_uses.count(type::PipelineStageUsage::kVertexOutput)) { + out << " [[user(locn" + std::to_string(location.value()) + ")]]"; + } else if (pipeline_stage_uses.count(type::PipelineStageUsage::kFragmentInput)) { + out << " [[user(locn" + std::to_string(location.value()) + ")]]"; + } else if (TINT_LIKELY( + pipeline_stage_uses.count(type::PipelineStageUsage::kFragmentOutput))) { + out << " [[color(" + std::to_string(location.value()) + ")]]"; + } else { + TINT_ICE(Writer, diagnostics_) << "invalid use of location decoration"; + return false; + } + } + + if (auto interpolation = attributes.interpolation) { + auto name = interpolation_to_attribute(interpolation->type, interpolation->sampling); + if (name.empty()) { + diagnostics_.add_error(diag::System::Writer, "unknown interpolation attribute"); + return false; + } + out << " [[" << name << "]]"; + } + + if (attributes.invariant) { + invariant_define_name_ = UniqueIdentifier("TINT_INVARIANT"); + out << " " << invariant_define_name_; } out << ";"; diff --git a/src/tint/writer/msl/generator_impl.h b/src/tint/writer/msl/generator_impl.h index 56992424d6..db5f310815 100644 --- a/src/tint/writer/msl/generator_impl.h +++ b/src/tint/writer/msl/generator_impl.h @@ -350,7 +350,7 @@ class GeneratorImpl : public TextGenerator { /// @param var the variable to generate /// @returns true if the variable was emitted bool EmitVar(const ast::Var* var); - /// Handles generating a function-scope 'let' declaration + /// Handles generating a 'let' declaration /// @param let the variable to generate /// @returns true if the variable was emitted bool EmitLet(const ast::Let* let);