From 97c9e0ba9f975054cede16f0709da9182a4fc2a0 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 6 Sep 2022 14:41:16 +0000 Subject: [PATCH] Store struct member location into the sem. This CL adds the `@location` of a Struct Member into the `StructMember` sem object as a `std::optional`. The resolver then populates the location value if an attribute is found during resolution. This will provide a place to store the evaluated expression value for a location in the future. Bug: tint:1633 Change-Id: I6f696968dddf95af1f933d96cdb4a7630badac2e Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101062 Kokoro: Kokoro Reviewed-by: Ben Clayton Commit-Queue: Dan Sinclair --- src/tint/resolver/inferred_type_test.cc | 10 +++---- src/tint/resolver/intrinsic_table.cc | 3 +- src/tint/resolver/resolver.cc | 11 ++++---- .../struct_pipeline_stage_use_test.cc | 28 +++++++++++++++++++ src/tint/sem/sem_struct_test.cc | 18 ++++++++++++ src/tint/sem/struct.cc | 6 ++-- src/tint/sem/struct.h | 9 +++++- src/tint/sem/type_test.cc | 3 +- 8 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/tint/resolver/inferred_type_test.cc b/src/tint/resolver/inferred_type_test.cc index 83ca0bd752..047ac78591 100644 --- a/src/tint/resolver/inferred_type_test.cc +++ b/src/tint/resolver/inferred_type_test.cc @@ -149,11 +149,11 @@ TEST_F(ResolverInferredTypeTest, InferStruct_Pass) { auto* member = Member("x", ty.i32()); auto* str = Structure("S", utils::Vector{member}); - auto* expected_type = - create(str, str->name, - sem::StructMemberList{create( - member, member->symbol, create(), 0u, 0u, 0u, 4u)}, - 0u, 4u, 4u); + auto* expected_type = create( + str, str->name, + sem::StructMemberList{create(member, member->symbol, create(), + 0u, 0u, 0u, 4u, std::nullopt)}, + 0u, 4u, 4u); auto* ctor_expr = Construct(ty.Of(str)); diff --git a/src/tint/resolver/intrinsic_table.cc b/src/tint/resolver/intrinsic_table.cc index 86b44a43a7..4db3cb4423 100644 --- a/src/tint/resolver/intrinsic_table.cc +++ b/src/tint/resolver/intrinsic_table.cc @@ -793,7 +793,8 @@ const sem::Struct* build_struct(MatchState& state, /* index */ static_cast(members.size()), /* offset */ offset, /* align */ align, - /* size */ size)); + /* size */ size, + /* location */ std::nullopt)); offset += size; } uint32_t size_without_padding = offset; diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 1599334d0d..5d51dd5f0f 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -2747,6 +2747,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; for (auto* attr : member->attributes) { Mark(attr); if (auto* o = attr->As()) { @@ -2760,11 +2761,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { align = 1; has_offset_attr = true; } else if (auto* a = attr->As()) { - const auto* expr = Expression(a->align); - if (!expr) { - return nullptr; - } - auto* materialized = Materialize(expr); + auto* materialized = Materialize(Expression(a->align)); if (!materialized) { return nullptr; } @@ -2790,6 +2787,8 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { } size = s->size; has_size_attr = true; + } else if (auto* l = attr->As()) { + location = l->value; } } @@ -2811,7 +2810,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { auto* sem_member = builder_->create( member, member->symbol, type, static_cast(sem_members.size()), static_cast(offset), static_cast(align), - static_cast(size)); + static_cast(size), location); builder_->Sem().Add(member, sem_member); sem_members.emplace_back(sem_member); diff --git a/src/tint/resolver/struct_pipeline_stage_use_test.cc b/src/tint/resolver/struct_pipeline_stage_use_test.cc index acf30f6bf5..c8e77ea904 100644 --- a/src/tint/resolver/struct_pipeline_stage_use_test.cc +++ b/src/tint/resolver/struct_pipeline_stage_use_test.cc @@ -174,6 +174,20 @@ TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderParamViaAlias) { UnorderedElementsAre(sem::PipelineStageUsage::kFragmentInput)); } +TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderParamLocationSet) { + auto* s = Structure("S", utils::Vector{Member("a", ty.f32(), utils::Vector{Location(3)})}); + + Func("main", utils::Vector{Param("param", ty.Of(s))}, ty.void_(), utils::Empty, + utils::Vector{Stage(ast::PipelineStage::kFragment)}); + + ASSERT_TRUE(r()->Resolve()) << r()->error(); + + auto* sem = TypeOf(s)->As(); + ASSERT_NE(sem, nullptr); + ASSERT_EQ(1u, sem->Members().size()); + EXPECT_EQ(3u, sem->Members()[0]->Location()); +} + TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderReturnTypeViaAlias) { auto* s = Structure("S", utils::Vector{Member("a", ty.f32(), utils::Vector{Location(0)})}); auto* s_alias = Alias("S_alias", ty.Of(s)); @@ -190,5 +204,19 @@ TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderReturnTypeViaAlias) { UnorderedElementsAre(sem::PipelineStageUsage::kFragmentOutput)); } +TEST_F(ResolverPipelineStageUseTest, StructUsedAsShaderReturnTypeLocationSet) { + auto* s = Structure("S", utils::Vector{Member("a", ty.f32(), utils::Vector{Location(3)})}); + + Func("main", utils::Empty, ty.Of(s), utils::Vector{Return(Construct(ty.Of(s), Expr(0_f)))}, + utils::Vector{Stage(ast::PipelineStage::kFragment)}); + + ASSERT_TRUE(r()->Resolve()) << r()->error(); + + auto* sem = TypeOf(s)->As(); + ASSERT_NE(sem, nullptr); + ASSERT_EQ(1u, sem->Members().size()); + EXPECT_EQ(3u, sem->Members()[0]->Location()); +} + } // namespace } // namespace tint::resolver diff --git a/src/tint/sem/sem_struct_test.cc b/src/tint/sem/sem_struct_test.cc index 3ea14e113d..453b8f7ddc 100644 --- a/src/tint/sem/sem_struct_test.cc +++ b/src/tint/sem/sem_struct_test.cc @@ -105,5 +105,23 @@ TEST_F(StructTest, Layout) { /* */ };)"); } +TEST_F(StructTest, Location) { + auto* st = Structure("st", utils::Vector{ + Member("a", ty.i32(), utils::Vector{Location(1u)}), + Member("b", ty.u32()), + }); + + auto p = Build(); + ASSERT_TRUE(p.IsValid()) << p.Diagnostics().str(); + + auto* sem = p.Sem().Get(st); + ASSERT_EQ(2u, sem->Members().size()); + + EXPECT_TRUE(sem->Members()[0]->Location().has_value()); + EXPECT_EQ(sem->Members()[0]->Location().value(), 1u); + + EXPECT_FALSE(sem->Members()[1]->Location().has_value()); +} + } // namespace } // namespace tint::sem diff --git a/src/tint/sem/struct.cc b/src/tint/sem/struct.cc index 0ace9db55f..7d457e84b2 100644 --- a/src/tint/sem/struct.cc +++ b/src/tint/sem/struct.cc @@ -161,14 +161,16 @@ StructMember::StructMember(const ast::StructMember* declaration, uint32_t index, uint32_t offset, uint32_t align, - uint32_t size) + uint32_t size, + std::optional location) : declaration_(declaration), name_(name), type_(type), index_(index), offset_(offset), align_(align), - size_(size) {} + size_(size), + location_(location) {} StructMember::~StructMember() = default; diff --git a/src/tint/sem/struct.h b/src/tint/sem/struct.h index 62b1008ce3..0f3213ae8d 100644 --- a/src/tint/sem/struct.h +++ b/src/tint/sem/struct.h @@ -17,6 +17,7 @@ #include +#include #include #include #include @@ -180,13 +181,15 @@ class StructMember final : public 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 StructMember(const ast::StructMember* declaration, Symbol name, const sem::Type* type, uint32_t index, uint32_t offset, uint32_t align, - uint32_t size); + uint32_t size, + std::optional location); /// Destructor ~StructMember() override; @@ -219,6 +222,9 @@ class StructMember final : public Castable { /// @returns byte size uint32_t Size() const { return size_; } + /// @returns the location, if set + std::optional Location() const { return location_; } + private: const ast::StructMember* const declaration_; const Symbol name_; @@ -228,6 +234,7 @@ class StructMember final : public Castable { const uint32_t offset_; const uint32_t align_; const uint32_t size_; + const std::optional location_; }; } // namespace tint::sem diff --git a/src/tint/sem/type_test.cc b/src/tint/sem/type_test.cc index 837579ef84..56dcbff050 100644 --- a/src/tint/sem/type_test.cc +++ b/src/tint/sem/type_test.cc @@ -54,7 +54,8 @@ struct TypeTest : public TestHelper { /* index */ 0u, /* offset */ 0u, /* align */ 4u, - /* size */ 4u), + /* size */ 4u, + /* location */ std::nullopt), }, /* align*/ 4u, /* size*/ 4u,