From dc4e6c184495adc206cd453703a36189bc020f98 Mon Sep 17 00:00:00 2001 From: Antonio Maiorano Date: Fri, 14 May 2021 17:51:13 +0000 Subject: [PATCH] Remove sem::AccessControl In preparation for implementing https://github.com/gpuweb/gpuweb/issues/1604, this change removes the sem::AccessControl node. Instead, the ast::AccessControl::Access enum is now on the sem::StorageTexture class, as well as on sem::Variable. For sem::Variable, the field is set when the variable's type is either a storage buffer or a storage texture. Bug: tint:802 Change-Id: Id479af36b401d067b015027923f4e715f5f69f25 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51020 Reviewed-by: Ben Clayton Commit-Queue: Antonio Maiorano --- src/BUILD.gn | 10 +- src/CMakeLists.txt | 3 - src/ast/access_control.cc | 10 ++ src/ast/access_control.h | 5 +- src/ast/intrinsic_texture_helper_test.cc | 3 +- src/ast/variable.cc | 2 +- src/ast/variable.h | 6 +- src/block_allocator.h | 2 +- src/inspector/inspector.cc | 15 +-- src/inspector/inspector_test.cc | 55 +++----- src/intrinsic_table.cc | 121 +++++++---------- src/intrinsic_table_test.cc | 46 ++++--- src/program_builder.h | 39 +++--- src/reader/spirv/parser_impl.cc | 1 - src/reader/spirv/parser_type_test.cc | 12 +- src/reader/wgsl/parser_impl.cc | 1 - .../parser_impl_variable_ident_decl_test.cc | 1 - src/resolver/assignment_validation_test.cc | 5 +- src/resolver/decoration_validation_test.cc | 2 +- .../host_shareable_validation_test.cc | 11 +- src/resolver/intrinsic_test.cc | 8 +- src/resolver/is_host_shareable_test.cc | 11 -- src/resolver/is_storeable_test.cc | 11 -- .../pipeline_overridable_constant_test.cc | 2 +- src/resolver/resolver.cc | 123 ++++++++++++------ src/resolver/resolver.h | 7 +- src/resolver/resolver_test.cc | 7 +- src/resolver/resolver_test_helper.h | 7 - src/resolver/storage_class_validation_test.cc | 11 +- src/resolver/struct_storage_class_use_test.cc | 2 +- src/resolver/type_validation_test.cc | 24 ++-- src/resolver/validation_test.cc | 1 - src/sem/access_control_type.cc | 70 ---------- src/sem/access_control_type.h | 65 --------- src/sem/access_control_type_test.cc | 80 ------------ src/sem/bool_type_test.cc | 1 - src/sem/depth_texture_type_test.cc | 1 - src/sem/external_texture_type_test.cc | 1 - src/sem/f32_type_test.cc | 1 - src/sem/i32_type_test.cc | 1 - src/sem/matrix_type_test.cc | 1 - src/sem/multisampled_texture_type_test.cc | 1 - src/sem/pointer_type_test.cc | 1 - src/sem/sampled_texture_type_test.cc | 1 - src/sem/sampler_type_test.cc | 1 - src/sem/sem_array_test.cc | 1 - src/sem/sem_struct_test.cc | 1 - src/sem/storage_texture_type.cc | 12 +- src/sem/storage_texture_type.h | 7 + src/sem/storage_texture_type_test.cc | 27 ++-- src/sem/type.cc | 7 +- src/sem/u32_type_test.cc | 1 - src/sem/variable.cc | 5 +- src/sem/variable.h | 9 +- src/sem/vector_type_test.cc | 1 - src/transform/binding_remapper.cc | 8 +- src/transform/decompose_storage_access.cc | 45 ++++--- src/transform/transform.cc | 4 - src/transform/transform_test.cc | 13 -- src/transform/vertex_pulling.cc | 2 +- src/typepair.h | 1 - src/writer/hlsl/generator_impl.cc | 78 ++++++----- src/writer/hlsl/generator_impl.h | 2 + .../hlsl/generator_impl_function_test.cc | 13 +- .../generator_impl_member_accessor_test.cc | 3 +- .../hlsl/generator_impl_sanitizer_test.cc | 3 +- src/writer/hlsl/generator_impl_type_test.cc | 70 +++++----- src/writer/msl/generator_impl.cc | 46 ++----- .../msl/generator_impl_function_test.cc | 11 +- src/writer/msl/generator_impl_type_test.cc | 9 +- src/writer/spirv/builder.cc | 96 +++++++------- src/writer/spirv/builder.h | 3 - src/writer/spirv/builder_function_test.cc | 2 +- .../spirv/builder_global_variable_test.cc | 43 +++--- src/writer/spirv/builder_intrinsic_test.cc | 4 +- src/writer/spirv/builder_type_test.cc | 18 +-- .../wgsl/generator_impl_function_test.cc | 3 +- .../wgsl/generator_impl_global_decl_test.cc | 1 - src/writer/wgsl/generator_impl_type_test.cc | 7 +- test/BUILD.gn | 3 +- 80 files changed, 531 insertions(+), 817 deletions(-) delete mode 100644 src/sem/access_control_type.cc delete mode 100644 src/sem/access_control_type.h delete mode 100644 src/sem/access_control_type_test.cc diff --git a/src/BUILD.gn b/src/BUILD.gn index 844a7038f3..1e2a90b658 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -298,10 +298,10 @@ libtint_source_set("libtint_core_all_src") { "ast/continue_statement.h", "ast/decoration.cc", "ast/decoration.h", - "ast/disable_validation_decoration.cc", - "ast/disable_validation_decoration.h", "ast/depth_texture.cc", "ast/depth_texture.h", + "ast/disable_validation_decoration.cc", + "ast/disable_validation_decoration.h", "ast/discard_statement.cc", "ast/discard_statement.h", "ast/else_statement.cc", @@ -447,8 +447,6 @@ libtint_source_set("libtint_core_all_src") { "resolver/resolver.cc", "resolver/resolver.h", "scope_stack.h", - "sem/access_control_type.cc", - "sem/access_control_type.h", "sem/array.h", "sem/binding_point.h", "sem/bool_type.cc", @@ -590,12 +588,12 @@ libtint_source_set("libtint_spv_reader_src") { "reader/spirv/function.h", "reader/spirv/namer.cc", "reader/spirv/namer.h", - "reader/spirv/parser_type.cc", - "reader/spirv/parser_type.h", "reader/spirv/parser.cc", "reader/spirv/parser.h", "reader/spirv/parser_impl.cc", "reader/spirv/parser_impl.h", + "reader/spirv/parser_type.cc", + "reader/spirv/parser_type.h", "reader/spirv/usage.cc", "reader/spirv/usage.h", ] diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6e5f3abac6..7dd8be96d2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -285,8 +285,6 @@ set(TINT_LIB_SRCS transform/transform.h transform/vertex_pulling.cc transform/vertex_pulling.h - sem/access_control_type.cc - sem/access_control_type.h sem/bool_type.cc sem/bool_type.h sem/depth_texture_type.cc @@ -567,7 +565,6 @@ if(${TINT_BUILD_TESTS}) symbol_test.cc transform/transform_test.cc test_main.cc - sem/access_control_type_test.cc sem/bool_type_test.cc sem/depth_texture_type_test.cc sem/external_texture_type_test.cc diff --git a/src/ast/access_control.cc b/src/ast/access_control.cc index 635eeb66b5..52e3532c6c 100644 --- a/src/ast/access_control.cc +++ b/src/ast/access_control.cc @@ -37,6 +37,9 @@ AccessControl::~AccessControl() = default; std::string AccessControl::type_name() const { std::string name = "__access_control_"; switch (access_) { + case ast::AccessControl::kInvalid: + name += "invalid"; + break; case ast::AccessControl::kReadOnly: name += "read_only"; break; @@ -54,6 +57,9 @@ std::string AccessControl::FriendlyName(const SymbolTable& symbols) const { std::ostringstream out; out << "[[access("; switch (access_) { + case ast::AccessControl::kInvalid: + out << "invalid"; + break; case ast::AccessControl::kReadOnly: out << "read"; break; @@ -77,6 +83,10 @@ AccessControl* AccessControl::Clone(CloneContext* ctx) const { std::ostream& operator<<(std::ostream& out, AccessControl::Access access) { switch (access) { + case ast::AccessControl::kInvalid: { + out << "invalid"; + break; + } case ast::AccessControl::kReadOnly: { out << "read_only"; break; diff --git a/src/ast/access_control.h b/src/ast/access_control.h index 08c6db1750..23ce43dfb8 100644 --- a/src/ast/access_control.h +++ b/src/ast/access_control.h @@ -28,8 +28,11 @@ class AccessControl : public Castable { public: /// The access control settings enum Access { + /// Invalid + // TODO(crbug.com/tint/805): Remove this. + kInvalid = -1, /// Read only - kReadOnly = 0, + kReadOnly, /// Write only kWriteOnly, /// Read write diff --git a/src/ast/intrinsic_texture_helper_test.cc b/src/ast/intrinsic_texture_helper_test.cc index 8837ecd247..f9430d4fa4 100644 --- a/src/ast/intrinsic_texture_helper_test.cc +++ b/src/ast/intrinsic_texture_helper_test.cc @@ -14,7 +14,6 @@ #include "src/ast/intrinsic_texture_helper_test.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/sampled_texture_type.h" @@ -173,7 +172,7 @@ ast::Variable* TextureOverloadCase::buildTextureVariable( case ast::intrinsic::test::TextureKind::kStorage: { auto st = b->ty.storage_texture(texture_dimension, image_format); - auto ac = b->ty.access(access_control, st); + auto* ac = b->ty.access(access_control, st); return b->Global("texture", ac, ast::StorageClass::kNone, nullptr, decos); } } diff --git a/src/ast/variable.cc b/src/ast/variable.cc index 6bfe36f553..974607c6a3 100644 --- a/src/ast/variable.cc +++ b/src/ast/variable.cc @@ -27,7 +27,7 @@ Variable::Variable(ProgramID program_id, const Source& source, const Symbol& sym, StorageClass declared_storage_class, - ast::Type* type, + const ast::Type* type, bool is_const, Expression* constructor, DecorationList decorations) diff --git a/src/ast/variable.h b/src/ast/variable.h index d91fea24ec..0151d7404f 100644 --- a/src/ast/variable.h +++ b/src/ast/variable.h @@ -109,7 +109,7 @@ class Variable : public Castable { const Source& source, const Symbol& sym, StorageClass declared_storage_class, - ast::Type* type, + const ast::Type* type, bool is_const, Expression* constructor, DecorationList decorations); @@ -122,7 +122,7 @@ class Variable : public Castable { const Symbol& symbol() const { return symbol_; } /// @returns the variable type - ast::Type* type() const { return type_; } + ast::Type* type() const { return const_cast(type_); } /// @returns the declared storage class StorageClass declared_storage_class() const { @@ -177,7 +177,7 @@ class Variable : public Castable { Symbol const symbol_; // The value type if a const or formal paramter, and the store type if a var - ast::Type* const type_; + ast::Type const* const type_; bool const is_const_; Expression* const constructor_; DecorationList const decorations_; diff --git a/src/block_allocator.h b/src/block_allocator.h index 915d4b07c7..44de75c52b 100644 --- a/src/block_allocator.h +++ b/src/block_allocator.h @@ -153,7 +153,7 @@ class BlockAllocator { std::is_same::value || std::is_base_of::value, "TYPE does not derive from T"); auto uptr = std::make_unique(std::forward(args)...); - auto ptr = uptr.get(); + auto* ptr = uptr.get(); objects_.emplace_back(std::move(uptr)); return ptr; } diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc index 13809a18d8..a6c9ec25b8 100644 --- a/src/inspector/inspector.cc +++ b/src/inspector/inspector.cc @@ -23,7 +23,6 @@ #include "src/ast/scalar_constructor_expression.h" #include "src/ast/sint_literal.h" #include "src/ast/uint_literal.h" -#include "src/sem/access_control_type.h" #include "src/sem/array.h" #include "src/sem/f32_type.h" #include "src/sem/function.h" @@ -606,12 +605,11 @@ std::vector Inspector::GetStorageBufferResourceBindingsImpl( auto* var = rsv.first; auto binding_info = rsv.second; - auto* ac_type = var->Type()->As(); - if (ac_type == nullptr) { + if (var->AccessControl() == ast::AccessControl::kInvalid) { continue; } - if (read_only != ac_type->IsReadOnly()) { + if (read_only != (var->AccessControl() == ast::AccessControl::kReadOnly)) { continue; } @@ -691,12 +689,10 @@ std::vector Inspector::GetStorageTextureResourceBindingsImpl( auto* var = ref.first; auto binding_info = ref.second; - auto* ac_type = var->Type()->As(); - if (ac_type == nullptr) { - continue; - } + auto* texture_type = var->Type()->As(); - if (read_only != ac_type->IsReadOnly()) { + if (read_only != + (texture_type->access_control() == ast::AccessControl::kReadOnly)) { continue; } @@ -707,7 +703,6 @@ std::vector Inspector::GetStorageTextureResourceBindingsImpl( entry.bind_group = binding_info.group->value(); entry.binding = binding_info.binding->value(); - auto* texture_type = var->Type()->UnwrapAccess()->As(); entry.dim = TypeTextureDimensionToResourceBindingTextureDimension( texture_type->dim()); diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc index 72d071eab9..39c98c61a2 100644 --- a/src/inspector/inspector_test.cc +++ b/src/inspector/inspector_test.cc @@ -548,33 +548,20 @@ class InspectorHelper : public ProgramBuilder { return nullptr; } - /// Generates appropriate types for a StorageTexture - /// @param dim the texture dimension of the storage texture - /// @param format the image format of the storage texture - /// @returns the storage texture type and subtype - std::tuple MakeStorageTextureTypes( - ast::TextureDimension dim, - ast::ImageFormat format) { - auto tex = ty.storage_texture(dim, format); - return {tex, {tex.ast->type(), tex.sem->type()}}; - } /// Generates appropriate types for a Read-Only StorageTexture /// @param dim the texture dimension of the storage texture /// @param format the image format of the storage texture /// @param read_only should the access type be read only, otherwise write only /// @returns the storage texture type, subtype & access control type - std::tuple - MakeStorageTextureTypes(ast::TextureDimension dim, - ast::ImageFormat format, - bool read_only) { - typ::StorageTexture texture_type; - typ::Type subtype; - std::tie(texture_type, subtype) = MakeStorageTextureTypes(dim, format); - auto access_control = ty.access(read_only ? ast::AccessControl::kReadOnly - : ast::AccessControl::kWriteOnly, - texture_type); - return {texture_type, subtype, access_control}; + typ::Type MakeStorageTextureTypes(ast::TextureDimension dim, + ast::ImageFormat format, + bool read_only) { + auto ac = read_only ? ast::AccessControl::kReadOnly + : ast::AccessControl::kWriteOnly; + auto tex = ty.storage_texture(dim, format); + + return {ty.access(ac, tex.ast), tex.sem}; } /// Adds a storage texture variable to the program @@ -1682,20 +1669,14 @@ TEST_F(InspectorGetResourceBindingsTest, Simple) { MakeComparisonSamplerReferenceBodyFunction( "cs_func", "cs_texture", "cs_var", "cs_coords", "cs_depth", ty.f32(), {}); - typ::StorageTexture st_type; - typ::Type st_subtype; - typ::AccessControl st_ac; - std::tie(st_type, st_subtype, st_ac) = MakeStorageTextureTypes( - ast::TextureDimension::k2d, ast::ImageFormat::kR32Uint, false); - AddStorageTexture("st_var", st_ac, 4, 0); + auto st_type = MakeStorageTextureTypes(ast::TextureDimension::k2d, + ast::ImageFormat::kR32Uint, false); + AddStorageTexture("st_var", st_type, 4, 0); MakeStorageTextureBodyFunction("st_func", "st_var", ty.vec2(), {}); - typ::StorageTexture rost_type; - typ::Type rost_subtype; - typ::AccessControl rost_ac; - std::tie(rost_type, rost_subtype, rost_ac) = MakeStorageTextureTypes( - ast::TextureDimension::k2d, ast::ImageFormat::kR32Uint, true); - AddStorageTexture("rost_var", rost_ac, 4, 1); + auto rost_type = MakeStorageTextureTypes(ast::TextureDimension::k2d, + ast::ImageFormat::kR32Uint, true); + AddStorageTexture("rost_var", rost_type, 4, 1); MakeStorageTextureBodyFunction("rost_func", "rost_var", ty.vec2(), {}); MakeCallerBodyFunction("ep_func", @@ -2797,12 +2778,8 @@ TEST_P(InspectorGetStorageTextureResourceBindingsTestWithParam, Simple) { ResourceBinding::SampledKind expected_kind; std::tie(format, expected_format, expected_kind) = format_params; - typ::StorageTexture st_type; - typ::Type st_subtype; - typ::AccessControl ac; - std::tie(st_type, st_subtype, ac) = - MakeStorageTextureTypes(dim, format, read_only); - AddStorageTexture("st_var", ac, 0, 0); + auto st_type = MakeStorageTextureTypes(dim, format, read_only); + AddStorageTexture("st_var", st_type, 0, 0); typ::Type dim_type = nullptr; switch (dim) { diff --git a/src/intrinsic_table.cc b/src/intrinsic_table.cc index 16b90665ea..a638a1dbca 100644 --- a/src/intrinsic_table.cc +++ b/src/intrinsic_table.cc @@ -20,7 +20,6 @@ #include #include "src/program_builder.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/multisampled_texture_type.h" @@ -519,25 +518,25 @@ class StorageTextureBuilder : public Builder { public: explicit StorageTextureBuilder( ast::TextureDimension dimensions, + ast::AccessControl::Access access, OpenNumber texel_format, // a.k.a "image format" OpenType channel_format) // a.k.a "storage subtype" : dimensions_(dimensions), + access_(access), texel_format_(texel_format), channel_format_(channel_format) {} bool Match(MatchState& state, const sem::Type* ty) const override { - if (auto* ac = ty->As()) { - // If we have an storage texture argument that's got an access control - // type wrapped around it, accept it. Signatures that don't include an - // access control imply any access. Example: - // textureDimensions(t : texture_storage_1d) -> i32 - ty = ac->type(); - } - if (auto* tex = ty->As()) { if (MatchOpenNumber(state, texel_format_, static_cast(tex->image_format()))) { if (MatchOpenType(state, channel_format_, tex->type())) { + // AccessControl::kInvalid means match any + if (access_ != ast::AccessControl::kInvalid && + access_ != tex->access_control()) { + return false; + } + return tex->dim() == dimensions_; } } @@ -550,17 +549,27 @@ class StorageTextureBuilder : public Builder { static_cast(state.open_numbers.at(texel_format_)); auto* channel_format = state.open_types.at(channel_format_); return state.ty_mgr.Get( - dimensions_, texel_format, const_cast(channel_format)); + dimensions_, texel_format, access_, + const_cast(channel_format)); } std::string str() const override { std::stringstream ss; - ss << "texture_storage_" << dimensions_ << ""; + + ss << "texture_storage_" << dimensions_ << ""; + return ss.str(); } private: ast::TextureDimension const dimensions_; + ast::AccessControl::Access const access_; OpenNumber const texel_format_; OpenType const channel_format_; }; @@ -611,38 +620,6 @@ class SamplerBuilder : public Builder { ast::SamplerKind const kind_; }; -/// AccessControlBuilder is a Matcher / Builder for AccessControl types -class AccessControlBuilder : public Builder { - public: - explicit AccessControlBuilder(ast::AccessControl::Access access_control, - Builder* type) - : access_control_(access_control), type_(type) {} - - bool Match(MatchState& state, const sem::Type* ty) const override { - if (auto* ac = ty->As()) { - if (ac->access_control() == access_control_) { - return type_->Match(state, ty); - } - } - return false; - } - - sem::Type* Build(BuildState& state) const override { - auto* ty = type_->Build(state); - return state.ty_mgr.Get(access_control_, ty); - } - - std::string str() const override { - std::stringstream ss; - ss << "[[access(" << access_control_ << ")]] " << type_->str(); - return ss.str(); - } - - private: - ast::AccessControl::Access const access_control_; - Builder* const type_; -}; - /// Impl is the private implementation of the IntrinsicTable interface. class Impl : public IntrinsicTable { public: @@ -764,10 +741,11 @@ class Impl : public IntrinsicTable { /// @returns a Matcher / Builder that matches a storage texture of the given /// format with the given dimensions Builder* storage_texture(ast::TextureDimension dimensions, + ast::AccessControl::Access access, OpenNumber texel_format, OpenType channel_format) { return matcher_allocator_.Create( - dimensions, texel_format, channel_format); + dimensions, access, texel_format, channel_format); } /// @returns a Matcher / Builder that matches an external texture @@ -780,13 +758,6 @@ class Impl : public IntrinsicTable { return matcher_allocator_.Create(kind); } - /// @returns a Matcher / Builder that matches an access control type - Builder* access_control(ast::AccessControl::Access access_control, - Builder* type) { - return matcher_allocator_.Create(access_control, - type); - } - /// Registers an overload with the given intrinsic type, return type Matcher / /// Builder, and parameter Matcher / Builders. /// This overload of Register does not constrain any OpenTypes. @@ -1108,30 +1079,32 @@ Impl::Impl() { auto* tex_depth_cube = depth_texture(Dim::kCube); auto* tex_depth_cube_array = depth_texture(Dim::kCubeArray); auto* tex_external = external_texture(); - auto* tex_storage_1d_FT = - storage_texture(Dim::k1d, OpenNumber::F, OpenType::T); - auto* tex_storage_2d_FT = - storage_texture(Dim::k2d, OpenNumber::F, OpenType::T); - auto* tex_storage_2d_array_FT = - storage_texture(Dim::k2dArray, OpenNumber::F, OpenType::T); - auto* tex_storage_3d_FT = - storage_texture(Dim::k3d, OpenNumber::F, OpenType::T); - auto* tex_storage_ro_1d_FT = - access_control(ast::AccessControl::kReadOnly, tex_storage_1d_FT); - auto* tex_storage_ro_2d_FT = - access_control(ast::AccessControl::kReadOnly, tex_storage_2d_FT); - auto* tex_storage_ro_2d_array_FT = - access_control(ast::AccessControl::kReadOnly, tex_storage_2d_array_FT); - auto* tex_storage_ro_3d_FT = - access_control(ast::AccessControl::kReadOnly, tex_storage_3d_FT); - auto* tex_storage_wo_1d_FT = - access_control(ast::AccessControl::kWriteOnly, tex_storage_1d_FT); - auto* tex_storage_wo_2d_FT = - access_control(ast::AccessControl::kWriteOnly, tex_storage_2d_FT); + auto* tex_storage_1d_FT = storage_texture( + Dim::k1d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); + auto* tex_storage_2d_FT = storage_texture( + Dim::k2d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); + auto* tex_storage_2d_array_FT = storage_texture( + Dim::k2dArray, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); + auto* tex_storage_3d_FT = storage_texture( + Dim::k3d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); + auto* tex_storage_ro_1d_FT = storage_texture( + Dim::k1d, ast::AccessControl::kReadOnly, OpenNumber::F, OpenType::T); + auto* tex_storage_ro_2d_FT = storage_texture( + Dim::k2d, ast::AccessControl::kReadOnly, OpenNumber::F, OpenType::T); + auto* tex_storage_ro_2d_array_FT = storage_texture( + Dim::k2dArray, ast::AccessControl::kReadOnly, OpenNumber::F, OpenType::T); + auto* tex_storage_ro_3d_FT = storage_texture( + Dim::k3d, ast::AccessControl::kReadOnly, OpenNumber::F, OpenType::T); + auto* tex_storage_wo_1d_FT = storage_texture( + Dim::k1d, ast::AccessControl::kWriteOnly, OpenNumber::F, OpenType::T); + auto* tex_storage_wo_2d_FT = storage_texture( + Dim::k2d, ast::AccessControl::kWriteOnly, OpenNumber::F, OpenType::T); auto* tex_storage_wo_2d_array_FT = - access_control(ast::AccessControl::kWriteOnly, tex_storage_2d_array_FT); - auto* tex_storage_wo_3d_FT = - access_control(ast::AccessControl::kWriteOnly, tex_storage_3d_FT); + storage_texture(Dim::k2dArray, ast::AccessControl::kWriteOnly, + OpenNumber::F, OpenType::T); + auto* tex_storage_wo_3d_FT = storage_texture( + Dim::k3d, ast::AccessControl::kWriteOnly, OpenNumber::F, OpenType::T); + auto* sampler = this->sampler(ast::SamplerKind::kSampler); auto* sampler_comparison = this->sampler(ast::SamplerKind::kComparisonSampler); diff --git a/src/intrinsic_table_test.cc b/src/intrinsic_table_test.cc index b0e465e2d5..5bd2f5d5e6 100644 --- a/src/intrinsic_table_test.cc +++ b/src/intrinsic_table_test.cc @@ -16,7 +16,6 @@ #include "gmock/gmock.h" #include "src/program_builder.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/multisampled_texture_type.h" @@ -302,34 +301,39 @@ TEST_F(IntrinsicTableTest, MatchExternalTexture) { } TEST_F(IntrinsicTableTest, MatchROStorageTexture) { - auto tex = ty.storage_texture(ast::TextureDimension::k2d, - ast::ImageFormat::kR16Float); - auto tex_ac = ty.access(ast::AccessControl::kReadOnly, tex); + auto* subtype = + sem::StorageTexture::SubtypeFor(ast::ImageFormat::kR16Float, Types()); + auto* tex = create( + ast::TextureDimension::k2d, ast::ImageFormat::kR16Float, + ast::AccessControl::kReadOnly, subtype); + auto result = table->Lookup(*this, IntrinsicType::kTextureLoad, - {tex_ac, ty.vec2()}, Source{}); + {tex, ty.vec2()}, Source{}); ASSERT_NE(result.intrinsic, nullptr); ASSERT_EQ(result.diagnostics.str(), ""); EXPECT_THAT(result.intrinsic->Type(), IntrinsicType::kTextureLoad); EXPECT_THAT(result.intrinsic->ReturnType(), ty.vec4()); EXPECT_THAT( result.intrinsic->Parameters(), - ElementsAre(Parameter{tex_ac, Parameter::Usage::kTexture}, + ElementsAre(Parameter{tex, Parameter::Usage::kTexture}, Parameter{ty.vec2(), Parameter::Usage::kCoords})); } TEST_F(IntrinsicTableTest, MatchWOStorageTexture) { - auto tex = ty.storage_texture(ast::TextureDimension::k2d, - ast::ImageFormat::kR16Float); - auto tex_ac = ty.access(ast::AccessControl::kWriteOnly, tex); - auto result = - table->Lookup(*this, IntrinsicType::kTextureStore, - {tex_ac, ty.vec2(), ty.vec4()}, Source{}); + auto* subtype = + sem::StorageTexture::SubtypeFor(ast::ImageFormat::kR16Float, Types()); + auto* tex = create( + ast::TextureDimension::k2d, ast::ImageFormat::kR16Float, + ast::AccessControl::kWriteOnly, subtype); + + auto result = table->Lookup(*this, IntrinsicType::kTextureStore, + {tex, ty.vec2(), ty.vec4()}, Source{}); ASSERT_NE(result.intrinsic, nullptr); ASSERT_EQ(result.diagnostics.str(), ""); EXPECT_THAT(result.intrinsic->Type(), IntrinsicType::kTextureStore); EXPECT_THAT(result.intrinsic->ReturnType(), ty.void_()); EXPECT_THAT(result.intrinsic->Parameters(), - ElementsAre(Parameter{tex_ac, Parameter::Usage::kTexture}, + ElementsAre(Parameter{tex, Parameter::Usage::kTexture}, Parameter{ty.vec2(), Parameter::Usage::kCoords}, Parameter{ty.vec4(), Parameter::Usage::kValue})); } @@ -440,10 +444,10 @@ TEST_F(IntrinsicTableTest, OverloadOrderByNumberOfParameters) { textureDimensions(texture : texture_depth_2d_array) -> vec2 textureDimensions(texture : texture_depth_cube) -> vec3 textureDimensions(texture : texture_depth_cube_array) -> vec3 - textureDimensions(texture : texture_storage_1d) -> i32 - textureDimensions(texture : texture_storage_2d) -> vec2 - textureDimensions(texture : texture_storage_2d_array) -> vec2 - textureDimensions(texture : texture_storage_3d) -> vec3 + textureDimensions(texture : texture_storage_1d) -> i32 + textureDimensions(texture : texture_storage_2d) -> vec2 + textureDimensions(texture : texture_storage_2d_array) -> vec2 + textureDimensions(texture : texture_storage_3d) -> vec3 textureDimensions(texture : texture_external) -> vec2 )"); } @@ -478,10 +482,10 @@ TEST_F(IntrinsicTableTest, OverloadOrderByMatchingParameter) { textureDimensions(texture : texture_depth_2d_array) -> vec2 textureDimensions(texture : texture_depth_cube) -> vec3 textureDimensions(texture : texture_depth_cube_array) -> vec3 - textureDimensions(texture : texture_storage_1d) -> i32 - textureDimensions(texture : texture_storage_2d) -> vec2 - textureDimensions(texture : texture_storage_2d_array) -> vec2 - textureDimensions(texture : texture_storage_3d) -> vec3 + textureDimensions(texture : texture_storage_1d) -> i32 + textureDimensions(texture : texture_storage_2d) -> vec2 + textureDimensions(texture : texture_storage_2d_array) -> vec2 + textureDimensions(texture : texture_storage_3d) -> vec3 textureDimensions(texture : texture_external) -> vec2 )"); } diff --git a/src/program_builder.h b/src/program_builder.h index 833cb33124..42375a9293 100644 --- a/src/program_builder.h +++ b/src/program_builder.h @@ -60,7 +60,6 @@ #include "src/ast/void.h" #include "src/program.h" #include "src/program_id.h" -#include "src/sem/access_control_type.h" #include "src/sem/array.h" #include "src/sem/bool_type.h" #include "src/sem/depth_texture_type.h" @@ -704,13 +703,10 @@ class ProgramBuilder { /// @param access the access control /// @param type the inner type /// @returns the access control qualifier type - typ::AccessControl access(ast::AccessControl::Access access, - typ::Type type) const { - type = MaybeCreateTypename(type); - return {type.ast ? builder->create(access, type) - : nullptr, - type.sem ? builder->create(access, type) - : nullptr}; + ast::AccessControl* access(ast::AccessControl::Access access, + const ast::Type* type) const { + type = MaybeCreateTypename(type).ast; + return type ? builder->create(access, type) : nullptr; } /// Creates an access control qualifier type @@ -718,15 +714,12 @@ class ProgramBuilder { /// @param access the access control /// @param type the inner type /// @returns the access control qualifier type - typ::AccessControl access(const Source& source, - ast::AccessControl::Access access, - typ::Type type) const { - type = MaybeCreateTypename(type); - return {type.ast - ? builder->create(source, access, type) - : nullptr, - type.sem ? builder->create(access, type) - : nullptr}; + ast::AccessControl* access(const Source& source, + ast::AccessControl::Access access, + const ast::Type* type) const { + type = MaybeCreateTypename(type).ast; + return type ? builder->create(source, access, type) + : nullptr; } /// @param type the type of the pointer @@ -855,7 +848,8 @@ class ProgramBuilder { auto* sem_subtype = sem::StorageTexture::SubtypeFor(format, builder->Types()); return {builder->create(dims, format, ast_subtype), - builder->create(dims, format, sem_subtype)}; + builder->create( + dims, format, ast::AccessControl::kInvalid, sem_subtype)}; } /// @param source the Source of the node @@ -870,7 +864,8 @@ class ProgramBuilder { sem::StorageTexture::SubtypeFor(format, builder->Types()); return {builder->create(source, dims, format, ast_subtype), - builder->create(dims, format, sem_subtype)}; + builder->create( + dims, format, ast::AccessControl::kInvalid, sem_subtype)}; } /// @param source the Source of the node @@ -1189,7 +1184,7 @@ class ProgramBuilder { /// @returns a `ast::Variable` with the given name, storage and type template ast::Variable* Var(NAME&& name, - ast::Type* type, + const ast::Type* type, ast::StorageClass storage = ast::StorageClass::kNone, ast::Expression* constructor = nullptr, ast::DecorationList decorations = {}) { @@ -1208,7 +1203,7 @@ class ProgramBuilder { template ast::Variable* Var(const Source& source, NAME&& name, - ast::Type* type, + const ast::Type* type, ast::StorageClass storage = ast::StorageClass::kNone, ast::Expression* constructor = nullptr, ast::DecorationList decorations = {}) { @@ -1290,7 +1285,7 @@ class ProgramBuilder { /// global variable with the ast::Module. template ast::Variable* Global(NAME&& name, - ast::Type* type, + const ast::Type* type, ast::StorageClass storage, ast::Expression* constructor = nullptr, ast::DecorationList decorations = {}) { diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 068d07764d..30d833cf5b 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -25,7 +25,6 @@ #include "src/ast/struct_block_decoration.h" #include "src/ast/type_name.h" #include "src/reader/spirv/function.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/sampled_texture_type.h" diff --git a/src/reader/spirv/parser_type_test.cc b/src/reader/spirv/parser_type_test.cc index 523e736190..11458eebb0 100644 --- a/src/reader/spirv/parser_type_test.cc +++ b/src/reader/spirv/parser_type_test.cc @@ -35,8 +35,8 @@ TEST(SpvParserTypeTest, SameArgumentsGivesSamePointer) { EXPECT_EQ(ty.Vector(ty.I32(), 3), ty.Vector(ty.I32(), 3)); EXPECT_EQ(ty.Matrix(ty.I32(), 3, 2), ty.Matrix(ty.I32(), 3, 2)); EXPECT_EQ(ty.Array(ty.I32(), 3, 2), ty.Array(ty.I32(), 3, 2)); - EXPECT_EQ(ty.AccessControl(ty.I32(), ast::AccessControl::Access::kReadOnly), - ty.AccessControl(ty.I32(), ast::AccessControl::Access::kReadOnly)); + EXPECT_EQ(ty.AccessControl(ty.I32(), ast::AccessControl::kReadOnly), + ty.AccessControl(ty.I32(), ast::AccessControl::kReadOnly)); EXPECT_EQ(ty.Alias(sym, ty.I32()), ty.Alias(sym, ty.I32())); EXPECT_EQ(ty.Struct(sym, {ty.I32()}), ty.Struct(sym, {ty.I32()})); EXPECT_EQ(ty.Sampler(ast::SamplerKind::kSampler), @@ -70,10 +70,10 @@ TEST(SpvParserTypeTest, DifferentArgumentsGivesDifferentPointer) { EXPECT_NE(ty.Array(ty.I32(), 3, 2), ty.Array(ty.U32(), 3, 2)); EXPECT_NE(ty.Array(ty.I32(), 3, 2), ty.Array(ty.I32(), 2, 2)); EXPECT_NE(ty.Array(ty.I32(), 3, 2), ty.Array(ty.I32(), 3, 3)); - EXPECT_NE(ty.AccessControl(ty.I32(), ast::AccessControl::Access::kReadOnly), - ty.AccessControl(ty.U32(), ast::AccessControl::Access::kReadOnly)); - EXPECT_NE(ty.AccessControl(ty.I32(), ast::AccessControl::Access::kReadOnly), - ty.AccessControl(ty.I32(), ast::AccessControl::Access::kWriteOnly)); + EXPECT_NE(ty.AccessControl(ty.I32(), ast::AccessControl::kReadOnly), + ty.AccessControl(ty.U32(), ast::AccessControl::kReadOnly)); + EXPECT_NE(ty.AccessControl(ty.I32(), ast::AccessControl::kReadOnly), + ty.AccessControl(ty.I32(), ast::AccessControl::kWriteOnly)); EXPECT_NE(ty.Alias(sym_a, ty.I32()), ty.Alias(sym_b, ty.I32())); EXPECT_NE(ty.Struct(sym_a, {ty.I32()}), ty.Struct(sym_b, {ty.I32()})); EXPECT_NE(ty.Sampler(ast::SamplerKind::kSampler), diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index ae8c14df49..88618fef32 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -37,7 +37,6 @@ #include "src/ast/vector.h" #include "src/ast/workgroup_decoration.h" #include "src/reader/wgsl/lexer.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/multisampled_texture_type.h" diff --git a/src/reader/wgsl/parser_impl_variable_ident_decl_test.cc b/src/reader/wgsl/parser_impl_variable_ident_decl_test.cc index 32cf335d1a..696991daab 100644 --- a/src/reader/wgsl/parser_impl_variable_ident_decl_test.cc +++ b/src/reader/wgsl/parser_impl_variable_ident_decl_test.cc @@ -14,7 +14,6 @@ #include "src/ast/struct_block_decoration.h" #include "src/reader/wgsl/parser_impl_test_helper.h" -#include "src/sem/access_control_type.h" namespace tint { namespace reader { diff --git a/src/resolver/assignment_validation_test.cc b/src/resolver/assignment_validation_test.cc index a1fc4ec429..1c5e69661a 100644 --- a/src/resolver/assignment_validation_test.cc +++ b/src/resolver/assignment_validation_test.cc @@ -16,7 +16,6 @@ #include "gmock/gmock.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/storage_texture_type.h" namespace tint { @@ -260,8 +259,8 @@ TEST_F(ResolverAssignmentValidationTest, AssignFromPointer_Fail) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), "12:34 error v-000x: invalid assignment: right-hand-side is not " - "storable: ptr>"); + "storable: ptr>"); } } // namespace diff --git a/src/resolver/decoration_validation_test.cc b/src/resolver/decoration_validation_test.cc index adcaeabbc6..85a26ea838 100644 --- a/src/resolver/decoration_validation_test.cc +++ b/src/resolver/decoration_validation_test.cc @@ -523,7 +523,7 @@ TEST_F(ResourceDecorationTest, UniformBufferMissingBinding) { TEST_F(ResourceDecorationTest, StorageBufferMissingBinding) { auto* s = Structure("S", {Member("x", ty.i32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global(Source{{12, 34}}, "G", ac, ast::StorageClass::kStorage); EXPECT_FALSE(r()->Resolve()); diff --git a/src/resolver/host_shareable_validation_test.cc b/src/resolver/host_shareable_validation_test.cc index 09ce2c75f0..baa43a5b36 100644 --- a/src/resolver/host_shareable_validation_test.cc +++ b/src/resolver/host_shareable_validation_test.cc @@ -17,7 +17,6 @@ #include "gmock/gmock.h" #include "src/ast/struct_block_decoration.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/struct.h" namespace tint { @@ -29,7 +28,7 @@ using ResolverHostShareableValidationTest = ResolverTest; TEST_F(ResolverHostShareableValidationTest, BoolMember) { auto* s = Structure("S", {Member(Source{{12, 34}}, "x", ty.bool_())}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); Global(Source{{56, 78}}, "g", a, ast::StorageClass::kStorage, nullptr, { create(0), @@ -48,7 +47,7 @@ TEST_F(ResolverHostShareableValidationTest, BoolMember) { TEST_F(ResolverHostShareableValidationTest, BoolVectorMember) { auto* s = Structure("S", {Member(Source{{12, 34}}, "x", ty.vec3())}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); Global(Source{{56, 78}}, "g", a, ast::StorageClass::kStorage, nullptr, { create(0), @@ -69,7 +68,7 @@ TEST_F(ResolverHostShareableValidationTest, Aliases) { AST().AddConstructedType(a1); auto* s = Structure("S", {Member(Source{{12, 34}}, "x", a1)}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); auto* a2 = ty.alias("a2", ac); AST().AddConstructedType(a2); Global(Source{{56, 78}}, "g", a2, ast::StorageClass::kStorage, nullptr, @@ -94,7 +93,7 @@ TEST_F(ResolverHostShareableValidationTest, NestedStructures) { auto* s = Structure("S", {Member(Source{{7, 8}}, "m", i3)}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); Global(Source{{9, 10}}, "g", a, ast::StorageClass::kStorage, nullptr, { create(0), @@ -136,7 +135,7 @@ TEST_F(ResolverHostShareableValidationTest, NoError) { auto* s = Structure("S", {Member(Source{{7, 8}}, "m", i3)}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); Global(Source{{9, 10}}, "g", a, ast::StorageClass::kStorage, nullptr, { create(0), diff --git a/src/resolver/intrinsic_test.cc b/src/resolver/intrinsic_test.cc index caaa2c9d66..af7323dd36 100644 --- a/src/resolver/intrinsic_test.cc +++ b/src/resolver/intrinsic_test.cc @@ -30,7 +30,6 @@ #include "src/ast/unary_op_expression.h" #include "src/ast/variable_decl_statement.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/call.h" #include "src/sem/function.h" #include "src/sem/member_accessor_expression.h" @@ -241,7 +240,7 @@ class ResolverIntrinsicTest_TextureOperation } void add_call_param(std::string name, - typ::Type type, + const ast::Type* type, ast::ExpressionList* call_params) { if (type->UnwrapAll()->is_handle()) { Global(name, type, ast::StorageClass::kNone, nullptr, @@ -276,7 +275,8 @@ TEST_P(ResolverIntrinsicTest_StorageTextureOperation, TextureLoadRo) { auto coords_type = GetCoordsType(dim, ty.i32()); auto texture_type = ty.storage_texture(dim, format); - auto ro_texture_type = ty.access(ast::AccessControl::kReadOnly, texture_type); + auto* ro_texture_type = + ty.access(ast::AccessControl::kReadOnly, texture_type); ast::ExpressionList call_params; @@ -769,7 +769,7 @@ TEST_F(ResolverIntrinsicDataTest, ArrayLength_Vector) { auto* ary = ty.array(); auto* str = Structure("S", {Member("x", ary)}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, str); + auto* ac = ty.access(ast::AccessControl::kReadOnly, str); Global("a", ac, ast::StorageClass::kStorage, nullptr, { create(0), diff --git a/src/resolver/is_host_shareable_test.cc b/src/resolver/is_host_shareable_test.cc index cb27133dda..fef601b9fc 100644 --- a/src/resolver/is_host_shareable_test.cc +++ b/src/resolver/is_host_shareable_test.cc @@ -16,7 +16,6 @@ #include "gmock/gmock.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" namespace tint { namespace resolver { @@ -79,16 +78,6 @@ TEST_F(ResolverIsHostShareable, Pointer) { r()->IsHostShareable(ty.pointer(ast::StorageClass::kPrivate))); } -TEST_F(ResolverIsHostShareable, AccessControlVoid) { - EXPECT_FALSE(r()->IsHostShareable( - ty.access(ast::AccessControl::kReadOnly, ty.void_()))); -} - -TEST_F(ResolverIsHostShareable, AccessControlI32) { - EXPECT_TRUE( - r()->IsHostShareable(ty.access(ast::AccessControl::kReadOnly, ty.i32()))); -} - TEST_F(ResolverIsHostShareable, ArraySizedOfHostShareable) { auto* arr = create(create(), 5, 4, 20, 4, true); EXPECT_TRUE(r()->IsHostShareable(arr)); diff --git a/src/resolver/is_storeable_test.cc b/src/resolver/is_storeable_test.cc index 35362d6915..2b79ce49be 100644 --- a/src/resolver/is_storeable_test.cc +++ b/src/resolver/is_storeable_test.cc @@ -16,7 +16,6 @@ #include "gmock/gmock.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" namespace tint { namespace resolver { @@ -63,16 +62,6 @@ TEST_F(ResolverIsStorableTest, Pointer) { EXPECT_FALSE(r()->IsStorable(ty.pointer(ast::StorageClass::kPrivate))); } -TEST_F(ResolverIsStorableTest, AccessControlVoid) { - EXPECT_FALSE(r()->IsStorable( - create(ast::AccessControl::kReadOnly, ty.void_()))); -} - -TEST_F(ResolverIsStorableTest, AccessControlI32) { - EXPECT_TRUE(r()->IsStorable( - create(ast::AccessControl::kReadOnly, ty.i32()))); -} - TEST_F(ResolverIsStorableTest, ArraySizedOfStorable) { auto* arr = create(create(), 5, 4, 20, 4, true); EXPECT_TRUE(r()->IsStorable(arr)); diff --git a/src/resolver/pipeline_overridable_constant_test.cc b/src/resolver/pipeline_overridable_constant_test.cc index 48a345697c..9fdfd8fc30 100644 --- a/src/resolver/pipeline_overridable_constant_test.cc +++ b/src/resolver/pipeline_overridable_constant_test.cc @@ -81,7 +81,7 @@ TEST_F(ResolverPipelineOverridableConstantTest, WithAndWithoutIds) { for (auto* var : variables) { auto* sem = Sem().Get(var); ASSERT_NE(sem, nullptr); - constant_ids.push_back(sem->ConstantId()); + constant_ids.push_back(static_cast(sem->ConstantId())); } EXPECT_THAT(constant_ids, UnorderedElementsAre(0u, 3u, 2u, 4u, 5u, 1u)); } diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 328cb15fef..e21e22f9e7 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -46,7 +46,6 @@ #include "src/ast/variable_decl_statement.h" #include "src/ast/vector.h" #include "src/ast/workgroup_decoration.h" -#include "src/sem/access_control_type.h" #include "src/sem/array.h" #include "src/sem/call.h" #include "src/sem/depth_texture_type.h" @@ -341,12 +340,18 @@ sem::Type* Resolver::Type(const ast::Type* ty) { return builder_->create(); } if (auto* t = ty->As()) { + auto added = name_to_ast_type_.emplace(t->name(), t->type()).second; + // TODO(crbug.com/tint/803): Remove this. + if (!added) { + return nullptr; + } return Type(t->type()); } if (auto* t = ty->As()) { + ScopedAssignment sa(curent_access_control_, t); + if (auto* el = Type(t->type())) { - return builder_->create(t->access_control(), - const_cast(el)); + return el; } return nullptr; } @@ -400,8 +405,18 @@ sem::Type* Resolver::Type(const ast::Type* ty) { } if (auto* t = ty->As()) { if (auto* el = Type(t->type())) { + auto ac = ast::AccessControl::kInvalid; + if (curent_access_control_) { + ac = curent_access_control_->access_control(); + } else { + // TODO(amaiorano): move error about missing access control on storage + // textures here, instead of when variables declared. That way, we'd + // get the error on the alias line (for + // alias>). + } + return builder_->create( - t->dim(), t->image_format(), const_cast(el)); + t->dim(), t->image_format(), ac, const_cast(el)); } return nullptr; } @@ -483,8 +498,30 @@ Resolver::VariableInfo* Resolver::Variable(ast::Variable* var, return nullptr; } - auto* info = - variable_infos_.Create(var, const_cast(type), type_name); + // TODO(crbug.com/tint/802): Temporary while ast::AccessControl exits. + auto find_first_access_control = + [this](ast::Type* ty) -> ast::AccessControl* { + ast::AccessControl* ac = ty->As(); + if (ac) { + return ac; + } + while (auto* tn = ty->As()) { + ty = name_to_ast_type_[tn->name()]; + ac = ty->As(); + if (ac) { + return ac; + } + } + return nullptr; + }; + + auto access_control = ast::AccessControl::kInvalid; + if (auto* ac = find_first_access_control(var->type())) { + access_control = ac->access_control(); + } + + auto* info = variable_infos_.Create(var, const_cast(type), + type_name, access_control); variable_to_info_.emplace(var, info); return info; @@ -630,8 +667,10 @@ bool Resolver::ValidateGlobalVariable(const VariableInfo* info) { // Its store type must be a host-shareable structure type with block // attribute, satisfying the storage class constraints. - auto* access = info->type->As(); - auto* str = access ? access->type()->As() : nullptr; + auto* str = info->access_control != ast::AccessControl::kInvalid + ? info->type->As() + : nullptr; + if (!str) { diagnostics_.add_error( "variables declared in the storage class must be of an " @@ -716,38 +755,36 @@ bool Resolver::ValidateVariable(const VariableInfo* info) { } } - if (type->As()) { - diagnostics_.add_error("Storage Textures must have access control.", - var->source()); - return false; - } + if (auto* storage_tex = type->As()) { + if (storage_tex->access_control() == ast::AccessControl::kInvalid) { + diagnostics_.add_error("Storage Textures must have access control.", + var->source()); + return false; + } - if (auto* ac = type->As()) { - if (auto* r = ac->type()->As()) { - if (ac->IsReadWrite()) { - diagnostics_.add_error( - "Storage Textures only support Read-Only and Write-Only access " - "control.", - var->source()); - return false; - } + if (info->access_control == ast::AccessControl::kReadWrite) { + diagnostics_.add_error( + "Storage Textures only support Read-Only and Write-Only access " + "control.", + var->source()); + return false; + } - if (!IsValidStorageTextureDimension(r->dim())) { - diagnostics_.add_error( - "Cube dimensions for storage textures are not " - "supported.", - var->source()); - return false; - } + if (!IsValidStorageTextureDimension(storage_tex->dim())) { + diagnostics_.add_error( + "Cube dimensions for storage textures are not " + "supported.", + var->source()); + return false; + } - if (!IsValidStorageTextureImageFormat(r->image_format())) { - diagnostics_.add_error( - "image format must be one of the texel formats specified for " - "storage textues in " - "https://gpuweb.github.io/gpuweb/wgsl/#texel-formats", - var->source()); - return false; - } + if (!IsValidStorageTextureImageFormat(storage_tex->image_format())) { + diagnostics_.add_error( + "image format must be one of the texel formats specified for " + "storage textues in " + "https://gpuweb.github.io/gpuweb/wgsl/#texel-formats", + var->source()); + return false; } } @@ -2289,7 +2326,7 @@ void Resolver::CreateSemanticNodes() const { // Create a pipeline overridable constant. uint16_t constant_id; if (override_deco->HasValue()) { - constant_id = override_deco->value(); + constant_id = static_cast(override_deco->value()); } else { // No ID was specified, so allocate the next available ID. constant_id = next_constant_id; @@ -2306,8 +2343,8 @@ void Resolver::CreateSemanticNodes() const { sem_var = builder_->create(var, info->type, constant_id); } else { - sem_var = - builder_->create(var, info->type, info->storage_class); + sem_var = builder_->create( + var, info->type, info->storage_class, info->access_control); } std::vector users; @@ -2969,11 +3006,13 @@ void Resolver::Mark(const ast::Node* node) { Resolver::VariableInfo::VariableInfo(const ast::Variable* decl, sem::Type* ctype, - const std::string& tn) + const std::string& tn, + ast::AccessControl::Access ac) : declaration(decl), type(ctype), type_name(tn), - storage_class(decl->declared_storage_class()) { + storage_class(decl->declared_storage_class()), + access_control(ac) { if (storage_class == ast::StorageClass::kNone && type->UnwrapAll()->is_handle()) { // https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index 1f367f1288..821814036d 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h @@ -90,13 +90,15 @@ class Resolver { struct VariableInfo { VariableInfo(const ast::Variable* decl, sem::Type* type, - const std::string& type_name); + const std::string& type_name, + ast::AccessControl::Access ac); ~VariableInfo(); ast::Variable const* const declaration; sem::Type* type; std::string const type_name; ast::StorageClass storage_class; + ast::AccessControl::Access const access_control; std::vector users; sem::BindingPoint binding_point; }; @@ -348,8 +350,11 @@ class Resolver { std::unordered_map named_types_; std::unordered_set marked_; std::unordered_map constant_ids_; + std::unordered_map name_to_ast_type_; + FunctionInfo* current_function_ = nullptr; sem::Statement* current_statement_ = nullptr; + const ast::AccessControl* curent_access_control_ = nullptr; BlockAllocator variable_infos_; BlockAllocator function_infos_; }; diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc index 3b426468ee..2c00171fcb 100644 --- a/src/resolver/resolver_test.cc +++ b/src/resolver/resolver_test.cc @@ -33,7 +33,6 @@ #include "src/ast/unary_op_expression.h" #include "src/ast/variable_decl_statement.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/call.h" #include "src/sem/function.h" #include "src/sem/member_accessor_expression.h" @@ -763,7 +762,7 @@ TEST_F(ResolverTest, Function_Parameters) { TEST_F(ResolverTest, Function_RegisterInputOutputVariables) { auto* s = Structure("S", {Member("m", ty.u32())}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); auto* in_var = Global("in_var", ty.f32(), ast::StorageClass::kInput); auto* out_var = Global("out_var", ty.f32(), ast::StorageClass::kOutput); @@ -802,7 +801,7 @@ TEST_F(ResolverTest, Function_RegisterInputOutputVariables) { TEST_F(ResolverTest, Function_RegisterInputOutputVariables_SubFunction) { auto* s = Structure("S", {Member("m", ty.u32())}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); auto* in_var = Global("in_var", ty.f32(), ast::StorageClass::kInput); auto* out_var = Global("out_var", ty.f32(), ast::StorageClass::kOutput); @@ -1475,7 +1474,7 @@ TEST_F(ResolverTest, StorageClass_SetForSampler) { TEST_F(ResolverTest, StorageClass_SetForTexture) { auto t = ty.sampled_texture(ast::TextureDimension::k1d, ty.f32()); - auto ac = ty.access(ast::AccessControl::Access::kReadOnly, t); + auto* ac = ty.access(ast::AccessControl::kReadOnly, t); auto* var = Global("var", ac, ast::StorageClass::kNone, nullptr, { create(0), diff --git a/src/resolver/resolver_test_helper.h b/src/resolver/resolver_test_helper.h index cd9f5a64a3..46752566bc 100644 --- a/src/resolver/resolver_test_helper.h +++ b/src/resolver/resolver_test_helper.h @@ -255,13 +255,6 @@ sem::Type* sem_mat4x4(const ProgramBuilder::TypesBuilder& ty) { return ty.builder->create(column_type, 4u); } -template -sem::Type* sem_access(const ProgramBuilder::TypesBuilder& ty) { - auto* type = create_type(ty); - return ty.builder->create(ast::AccessControl::kReadOnly, - type); -} - } // namespace resolver } // namespace tint diff --git a/src/resolver/storage_class_validation_test.cc b/src/resolver/storage_class_validation_test.cc index 0db641021e..15e35945a0 100644 --- a/src/resolver/storage_class_validation_test.cc +++ b/src/resolver/storage_class_validation_test.cc @@ -17,7 +17,6 @@ #include "gmock/gmock.h" #include "src/ast/struct_block_decoration.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/struct.h" namespace tint { @@ -70,7 +69,7 @@ TEST_F(ResolverStorageClassValidationTest, StorageBufferArray) { // var g : [[access(read)]] array; auto* s = Structure("S", {Member("a", ty.f32())}); auto* a = ty.array(s, 3); - auto ac = ty.access(ast::AccessControl::kReadOnly, a); + auto* ac = ty.access(ast::AccessControl::kReadOnly, a); Global(Source{{56, 78}}, "g", ac, ast::StorageClass::kStorage, nullptr, { create(0), @@ -122,7 +121,7 @@ TEST_F(ResolverStorageClassValidationTest, StorageBufferNoBlockDecoration) { // struct S { x : i32 }; // var g : [[access(read)]] S; auto* s = Structure(Source{{12, 34}}, "S", {Member("x", ty.i32())}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); Global(Source{{56, 78}}, "g", a, ast::StorageClass::kStorage, nullptr, { create(0), @@ -142,7 +141,7 @@ TEST_F(ResolverStorageClassValidationTest, StorageBufferNoError_Basic) { // var g : [[access(read)]] S; auto* s = Structure("S", {Member(Source{{12, 34}}, "x", ty.i32())}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); Global(Source{{56, 78}}, "g", a, ast::StorageClass::kStorage, nullptr, { create(0), @@ -161,7 +160,7 @@ TEST_F(ResolverStorageClassValidationTest, StorageBufferNoError_Aliases) { {create()}); auto* a1 = ty.alias("a1", s); AST().AddConstructedType(a1); - auto ac = ty.access(ast::AccessControl::kReadOnly, a1); + auto* ac = ty.access(ast::AccessControl::kReadOnly, a1); auto* a2 = ty.alias("a2", ac); AST().AddConstructedType(a2); Global(Source{{56, 78}}, "g", a2, ast::StorageClass::kStorage, nullptr, @@ -211,7 +210,7 @@ TEST_F(ResolverStorageClassValidationTest, UniformBufferArray) { // var g : [[access(read)]] array; auto* s = Structure("S", {Member("a", ty.f32())}); auto* a = ty.array(s, 3); - auto ac = ty.access(ast::AccessControl::kReadOnly, a); + auto* ac = ty.access(ast::AccessControl::kReadOnly, a); Global(Source{{56, 78}}, "g", ac, ast::StorageClass::kUniform, nullptr, { create(0), diff --git a/src/resolver/struct_storage_class_use_test.cc b/src/resolver/struct_storage_class_use_test.cc index 1003eba04a..674e84b407 100644 --- a/src/resolver/struct_storage_class_use_test.cc +++ b/src/resolver/struct_storage_class_use_test.cc @@ -172,7 +172,7 @@ TEST_F(ResolverStorageClassUseTest, StructReachableViaLocalArray) { TEST_F(ResolverStorageClassUseTest, StructMultipleStorageClassUses) { auto* s = Structure("S", {Member("a", ty.f32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("x", s, ast::StorageClass::kUniform, nullptr, { create(0), diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc index 0c1a01afdc..beec7d54bd 100644 --- a/src/resolver/type_validation_test.cc +++ b/src/resolver/type_validation_test.cc @@ -18,7 +18,6 @@ #include "src/ast/struct_block_decoration.h" #include "src/resolver/resolver.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/storage_texture_type.h" @@ -501,11 +500,11 @@ static constexpr Params cases[] = { Params{ast_alias>>>>, sem_mat3x3}, - Params{ast_alias>>, sem_access}, + Params{ast_alias>>, sem_bool}, Params{ast_alias>>>>, - sem_access>>}, + sem_vec3}, Params{ast_alias>>>>, - sem_access>>}, + sem_mat3x3}, }; using CanonicalTest = ResolverTestWithParam; @@ -633,7 +632,7 @@ TEST_P(StorageTextureDimensionTest, All) { auto& params = GetParam(); auto st = ty.storage_texture(params.dim, ast::ImageFormat::kR32Uint); - auto ac = ty.access(ast::AccessControl::kReadOnly, st); + auto* ac = ty.access(ast::AccessControl::kReadOnly, st); Global("a", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ @@ -706,7 +705,7 @@ TEST_P(StorageTextureFormatTest, All) { // var d : [[access(read)]] texture_storage_3d<*>; auto st_a = ty.storage_texture(ast::TextureDimension::k1d, params.format); - auto ac_a = ty.access(ast::AccessControl::kReadOnly, st_a); + auto* ac_a = ty.access(ast::AccessControl::kReadOnly, st_a); Global("a", ac_a, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), @@ -714,7 +713,7 @@ TEST_P(StorageTextureFormatTest, All) { }); auto st_b = ty.storage_texture(ast::TextureDimension::k2d, params.format); - auto ac_b = ty.access(ast::AccessControl::kReadOnly, st_b); + auto* ac_b = ty.access(ast::AccessControl::kReadOnly, st_b); Global("b", ac_b, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), @@ -723,7 +722,7 @@ TEST_P(StorageTextureFormatTest, All) { auto st_c = ty.storage_texture(ast::TextureDimension::k2dArray, params.format); - auto ac_c = ty.access(ast::AccessControl::kReadOnly, st_c); + auto* ac_c = ty.access(ast::AccessControl::kReadOnly, st_c); Global("c", ac_c, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), @@ -731,7 +730,7 @@ TEST_P(StorageTextureFormatTest, All) { }); auto st_d = ty.storage_texture(ast::TextureDimension::k3d, params.format); - auto ac_d = ty.access(ast::AccessControl::kReadOnly, st_d); + auto* ac_d = ty.access(ast::AccessControl::kReadOnly, st_d); Global("d", ac_d, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), @@ -772,9 +771,8 @@ TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) { auto st = ty.storage_texture(ast::TextureDimension::k1d, ast::ImageFormat::kR32Uint); - auto ac = ty.access(ast::AccessControl::kReadWrite, st); - Global("a", ac, ast::StorageClass::kNone, nullptr, + Global("a", st, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -789,7 +787,7 @@ TEST_F(StorageTextureAccessControlTest, ReadOnlyAccessControl_Pass) { auto st = ty.storage_texture(ast::TextureDimension::k1d, ast::ImageFormat::kR32Uint); - auto ac = ty.access(ast::AccessControl::kReadOnly, st); + auto* ac = ty.access(ast::AccessControl::kReadOnly, st); Global("a", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ @@ -806,7 +804,7 @@ TEST_F(StorageTextureAccessControlTest, WriteOnlyAccessControl_Pass) { auto st = ty.storage_texture(ast::TextureDimension::k1d, ast::ImageFormat::kR32Uint); - auto ac = ty.access(ast::AccessControl::kWriteOnly, st); + auto* ac = ty.access(ast::AccessControl::kWriteOnly, st); Global("a", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc index 87a9608080..9440f1f560 100644 --- a/src/resolver/validation_test.cc +++ b/src/resolver/validation_test.cc @@ -29,7 +29,6 @@ #include "src/ast/unary_op_expression.h" #include "src/ast/variable_decl_statement.h" #include "src/resolver/resolver_test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/call.h" #include "src/sem/function.h" #include "src/sem/member_accessor_expression.h" diff --git a/src/sem/access_control_type.cc b/src/sem/access_control_type.cc deleted file mode 100644 index 8e94d6094b..0000000000 --- a/src/sem/access_control_type.cc +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright 2020 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. - -#include "src/sem/access_control_type.h" - -#include "src/program_builder.h" - -TINT_INSTANTIATE_TYPEINFO(tint::sem::AccessControl); - -namespace tint { -namespace sem { - -AccessControl::AccessControl(ast::AccessControl::Access access, - const Type* subtype) - : access_(access), subtype_(subtype) { - TINT_ASSERT(subtype_); - TINT_ASSERT(!subtype_->Is()); -} - -AccessControl::AccessControl(AccessControl&&) = default; - -AccessControl::~AccessControl() = default; - -std::string AccessControl::type_name() const { - std::string name = "__access_control_"; - switch (access_) { - case ast::AccessControl::kReadOnly: - name += "read_only"; - break; - case ast::AccessControl::kWriteOnly: - name += "write_only"; - break; - case ast::AccessControl::kReadWrite: - name += "read_write"; - break; - } - return name + subtype_->type_name(); -} - -std::string AccessControl::FriendlyName(const SymbolTable& symbols) const { - std::ostringstream out; - out << "[[access("; - switch (access_) { - case ast::AccessControl::kReadOnly: - out << "read"; - break; - case ast::AccessControl::kWriteOnly: - out << "write"; - break; - case ast::AccessControl::kReadWrite: - out << "read_write"; - break; - } - out << ")]] " << subtype_->FriendlyName(symbols); - return out.str(); -} - -} // namespace sem -} // namespace tint diff --git a/src/sem/access_control_type.h b/src/sem/access_control_type.h deleted file mode 100644 index 8f49ee023d..0000000000 --- a/src/sem/access_control_type.h +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2020 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_SEM_ACCESS_CONTROL_TYPE_H_ -#define SRC_SEM_ACCESS_CONTROL_TYPE_H_ - -#include - -#include "src/ast/access_control.h" -#include "src/sem/type.h" - -namespace tint { -namespace sem { - -/// An access control type. Holds an access setting and pointer to another type. -class AccessControl : public Castable { - public: - /// Constructor - /// @param access the access control setting - /// @param subtype the access controlled type - AccessControl(ast::AccessControl::Access access, const Type* subtype); - /// Move constructor - AccessControl(AccessControl&&); - ~AccessControl() override; - - /// @returns true if the access control is read only - bool IsReadOnly() const { return access_ == ast::AccessControl::kReadOnly; } - /// @returns true if the access control is write only - bool IsWriteOnly() const { return access_ == ast::AccessControl::kWriteOnly; } - /// @returns true if the access control is read/write - bool IsReadWrite() const { return access_ == ast::AccessControl::kReadWrite; } - - /// @returns the access control value - ast::AccessControl::Access access_control() const { return access_; } - /// @returns the subtype type - Type* type() const { return const_cast(subtype_); } - - /// @returns the name for this type - std::string type_name() const override; - - /// @param symbols the program's symbol table - /// @returns the name for this type that closely resembles how it would be - /// declared in WGSL. - std::string FriendlyName(const SymbolTable& symbols) const override; - - private: - ast::AccessControl::Access const access_; - const Type* const subtype_; -}; - -} // namespace sem -} // namespace tint - -#endif // SRC_SEM_ACCESS_CONTROL_TYPE_H_ diff --git a/src/sem/access_control_type_test.cc b/src/sem/access_control_type_test.cc deleted file mode 100644 index 0f738fcc17..0000000000 --- a/src/sem/access_control_type_test.cc +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright 2020 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. - -#include "src/sem/access_control_type.h" - -#include "src/sem/test_helper.h" -#include "src/sem/texture_type.h" - -namespace tint { -namespace sem { -namespace { - -using AccessControlTest = TestHelper; - -TEST_F(AccessControlTest, Create) { - U32 u32; - AccessControl a{ast::AccessControl::kReadWrite, &u32}; - EXPECT_TRUE(a.IsReadWrite()); - EXPECT_EQ(a.type(), &u32); -} - -TEST_F(AccessControlTest, AccessRead) { - I32 i32; - AccessControl at{ast::AccessControl::kReadOnly, &i32}; - EXPECT_TRUE(at.IsReadOnly()); - EXPECT_FALSE(at.IsWriteOnly()); - EXPECT_FALSE(at.IsReadWrite()); - - EXPECT_EQ(at.type_name(), "__access_control_read_only__i32"); -} - -TEST_F(AccessControlTest, AccessWrite) { - I32 i32; - AccessControl at{ast::AccessControl::kWriteOnly, &i32}; - EXPECT_FALSE(at.IsReadOnly()); - EXPECT_TRUE(at.IsWriteOnly()); - EXPECT_FALSE(at.IsReadWrite()); - - EXPECT_EQ(at.type_name(), "__access_control_write_only__i32"); -} - -TEST_F(AccessControlTest, AccessReadWrite) { - I32 i32; - AccessControl at{ast::AccessControl::kReadWrite, &i32}; - EXPECT_FALSE(at.IsReadOnly()); - EXPECT_FALSE(at.IsWriteOnly()); - EXPECT_TRUE(at.IsReadWrite()); - - EXPECT_EQ(at.type_name(), "__access_control_read_write__i32"); -} - -TEST_F(AccessControlTest, FriendlyNameReadOnly) { - AccessControl at{ast::AccessControl::kReadOnly, ty.i32()}; - EXPECT_EQ(at.FriendlyName(Symbols()), "[[access(read)]] i32"); -} - -TEST_F(AccessControlTest, FriendlyNameWriteOnly) { - AccessControl at{ast::AccessControl::kWriteOnly, ty.i32()}; - EXPECT_EQ(at.FriendlyName(Symbols()), "[[access(write)]] i32"); -} - -TEST_F(AccessControlTest, FriendlyNameReadWrite) { - AccessControl at{ast::AccessControl::kReadWrite, ty.i32()}; - EXPECT_EQ(at.FriendlyName(Symbols()), "[[access(read_write)]] i32"); -} - -} // namespace -} // namespace sem -} // namespace tint diff --git a/src/sem/bool_type_test.cc b/src/sem/bool_type_test.cc index bf0e1c8b35..4d08fda2a6 100644 --- a/src/sem/bool_type_test.cc +++ b/src/sem/bool_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/depth_texture_type_test.cc b/src/sem/depth_texture_type_test.cc index bd4829d369..c04507ec71 100644 --- a/src/sem/depth_texture_type_test.cc +++ b/src/sem/depth_texture_type_test.cc @@ -16,7 +16,6 @@ #include "src/sem/test_helper.h" -#include "src/sem/access_control_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/sampled_texture_type.h" #include "src/sem/storage_texture_type.h" diff --git a/src/sem/external_texture_type_test.cc b/src/sem/external_texture_type_test.cc index c570675ed8..ca15ea5178 100644 --- a/src/sem/external_texture_type_test.cc +++ b/src/sem/external_texture_type_test.cc @@ -14,7 +14,6 @@ #include "src/sem/external_texture_type.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/sampled_texture_type.h" diff --git a/src/sem/f32_type_test.cc b/src/sem/f32_type_test.cc index 660705c129..88fe3db461 100644 --- a/src/sem/f32_type_test.cc +++ b/src/sem/f32_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/i32_type_test.cc b/src/sem/i32_type_test.cc index 1578ee8a60..02ae0b456b 100644 --- a/src/sem/i32_type_test.cc +++ b/src/sem/i32_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/matrix_type_test.cc b/src/sem/matrix_type_test.cc index 19033eff9e..6a55b87f5a 100644 --- a/src/sem/matrix_type_test.cc +++ b/src/sem/matrix_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/multisampled_texture_type_test.cc b/src/sem/multisampled_texture_type_test.cc index ee454b8bfb..3b442463a3 100644 --- a/src/sem/multisampled_texture_type_test.cc +++ b/src/sem/multisampled_texture_type_test.cc @@ -14,7 +14,6 @@ #include "src/sem/multisampled_texture_type.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/sampled_texture_type.h" diff --git a/src/sem/pointer_type_test.cc b/src/sem/pointer_type_test.cc index afa3a0aeff..a45eb576d7 100644 --- a/src/sem/pointer_type_test.cc +++ b/src/sem/pointer_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/sampled_texture_type_test.cc b/src/sem/sampled_texture_type_test.cc index 89ec80f7e4..222dd7019e 100644 --- a/src/sem/sampled_texture_type_test.cc +++ b/src/sem/sampled_texture_type_test.cc @@ -14,7 +14,6 @@ #include "src/sem/sampled_texture_type.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/storage_texture_type.h" diff --git a/src/sem/sampler_type_test.cc b/src/sem/sampler_type_test.cc index 1673edab75..f27ac2e0ee 100644 --- a/src/sem/sampler_type_test.cc +++ b/src/sem/sampler_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/sem_array_test.cc b/src/sem/sem_array_test.cc index 515069e576..f0d2239349 100644 --- a/src/sem/sem_array_test.cc +++ b/src/sem/sem_array_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/sem_struct_test.cc b/src/sem/sem_struct_test.cc index 051b8237a4..b974ee765e 100644 --- a/src/sem/sem_struct_test.cc +++ b/src/sem/sem_struct_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/struct.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/storage_texture_type.cc b/src/sem/storage_texture_type.cc index c99f932fe7..ab6e270691 100644 --- a/src/sem/storage_texture_type.cc +++ b/src/sem/storage_texture_type.cc @@ -23,8 +23,12 @@ namespace sem { StorageTexture::StorageTexture(ast::TextureDimension dim, ast::ImageFormat format, + ast::AccessControl::Access access_control, sem::Type* subtype) - : Base(dim), image_format_(format), subtype_(subtype) {} + : Base(dim), + image_format_(format), + access_control_(access_control), + subtype_(subtype) {} StorageTexture::StorageTexture(StorageTexture&&) = default; @@ -32,13 +36,15 @@ StorageTexture::~StorageTexture() = default; std::string StorageTexture::type_name() const { std::ostringstream out; - out << "__storage_texture_" << dim() << "_" << image_format_; + out << "__storage_texture_" << dim() << "_" << image_format_ << "_" + << access_control_; return out.str(); } std::string StorageTexture::FriendlyName(const SymbolTable&) const { std::ostringstream out; - out << "texture_storage_" << dim() << "<" << image_format_ << ">"; + out << "texture_storage_" << dim() << "<" << image_format_ << ", " + << access_control_ << ">"; return out.str(); } diff --git a/src/sem/storage_texture_type.h b/src/sem/storage_texture_type.h index 5ca3206ce6..3a36743bb3 100644 --- a/src/sem/storage_texture_type.h +++ b/src/sem/storage_texture_type.h @@ -17,6 +17,7 @@ #include +#include "src/ast/access_control.h" #include "src/ast/storage_texture.h" #include "src/sem/texture_type.h" @@ -31,9 +32,11 @@ class StorageTexture : public Castable { /// Constructor /// @param dim the dimensionality of the texture /// @param format the image format of the texture + /// @param access_control the access control type of the texture /// @param subtype the storage subtype. Use SubtypeFor() to calculate this. StorageTexture(ast::TextureDimension dim, ast::ImageFormat format, + ast::AccessControl::Access access_control, sem::Type* subtype); /// Move constructor @@ -46,6 +49,9 @@ class StorageTexture : public Castable { /// @returns the image format ast::ImageFormat image_format() const { return image_format_; } + /// @returns the access control + ast::AccessControl::Access access_control() const { return access_control_; } + /// @returns the name for this type std::string type_name() const override; @@ -61,6 +67,7 @@ class StorageTexture : public Castable { private: ast::ImageFormat const image_format_; + ast::AccessControl::Access const access_control_; Type* const subtype_; }; diff --git a/src/sem/storage_texture_type_test.cc b/src/sem/storage_texture_type_test.cc index 35294c9f7b..80a078fab2 100644 --- a/src/sem/storage_texture_type_test.cc +++ b/src/sem/storage_texture_type_test.cc @@ -14,7 +14,6 @@ #include "src/sem/storage_texture_type.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/external_texture_type.h" #include "src/sem/sampled_texture_type.h" @@ -30,7 +29,8 @@ TEST_F(StorageTextureTest, Dim) { auto* subtype = StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types()); auto* s = create(ast::TextureDimension::k2dArray, - ast::ImageFormat::kRgba32Float, subtype); + ast::ImageFormat::kRgba32Float, + ast::AccessControl::kReadWrite, subtype); EXPECT_EQ(s->dim(), ast::TextureDimension::k2dArray); } @@ -38,7 +38,8 @@ TEST_F(StorageTextureTest, Format) { auto* subtype = StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types()); auto* s = create(ast::TextureDimension::k2dArray, - ast::ImageFormat::kRgba32Float, subtype); + ast::ImageFormat::kRgba32Float, + ast::AccessControl::kReadWrite, subtype); EXPECT_EQ(s->image_format(), ast::ImageFormat::kRgba32Float); } @@ -46,24 +47,28 @@ TEST_F(StorageTextureTest, TypeName) { auto* subtype = StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types()); auto* s = create(ast::TextureDimension::k2dArray, - ast::ImageFormat::kRgba32Float, subtype); - EXPECT_EQ(s->type_name(), "__storage_texture_2d_array_rgba32float"); + ast::ImageFormat::kRgba32Float, + ast::AccessControl::kReadWrite, subtype); + EXPECT_EQ(s->type_name(), + "__storage_texture_2d_array_rgba32float_read_write"); } TEST_F(StorageTextureTest, FriendlyName) { auto* subtype = StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types()); auto* s = create(ast::TextureDimension::k2dArray, - ast::ImageFormat::kRgba32Float, subtype); + ast::ImageFormat::kRgba32Float, + ast::AccessControl::kReadWrite, subtype); EXPECT_EQ(s->FriendlyName(Symbols()), - "texture_storage_2d_array"); + "texture_storage_2d_array"); } TEST_F(StorageTextureTest, F32) { auto* subtype = sem::StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types()); Type* s = create(ast::TextureDimension::k2dArray, - ast::ImageFormat::kRgba32Float, subtype); + ast::ImageFormat::kRgba32Float, + ast::AccessControl::kReadWrite, subtype); auto program = Build(); @@ -77,7 +82,8 @@ TEST_F(StorageTextureTest, U32) { auto* subtype = sem::StorageTexture::SubtypeFor(ast::ImageFormat::kRg32Uint, Types()); Type* s = create(ast::TextureDimension::k2dArray, - ast::ImageFormat::kRg32Uint, subtype); + ast::ImageFormat::kRg32Uint, + ast::AccessControl::kReadWrite, subtype); auto program = Build(); @@ -91,7 +97,8 @@ TEST_F(StorageTextureTest, I32) { auto* subtype = sem::StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Sint, Types()); Type* s = create(ast::TextureDimension::k2dArray, - ast::ImageFormat::kRgba32Sint, subtype); + ast::ImageFormat::kRgba32Sint, + ast::AccessControl::kReadWrite, subtype); auto program = Build(); diff --git a/src/sem/type.cc b/src/sem/type.cc index cd0f5c4a5e..7e80396c8f 100644 --- a/src/sem/type.cc +++ b/src/sem/type.cc @@ -14,7 +14,6 @@ #include "src/sem/type.h" -#include "src/sem/access_control_type.h" #include "src/sem/bool_type.h" #include "src/sem/f32_type.h" #include "src/sem/i32_type.h" @@ -45,10 +44,8 @@ const Type* Type::UnwrapPtr() const { } const Type* Type::UnwrapAccess() const { + // TODO(amaiorano): Delete this function auto* type = this; - while (auto* access = type->As()) { - type = access->type(); - } return type; } @@ -57,8 +54,6 @@ const Type* Type::UnwrapAll() const { while (true) { if (auto* ptr = type->As()) { type = ptr->type(); - } else if (auto* access = type->As()) { - type = access->type(); } else { break; } diff --git a/src/sem/u32_type_test.cc b/src/sem/u32_type_test.cc index 7859294903..17663dc8a9 100644 --- a/src/sem/u32_type_test.cc +++ b/src/sem/u32_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/sem/variable.cc b/src/sem/variable.cc index 83210cc1bf..3700c06959 100644 --- a/src/sem/variable.cc +++ b/src/sem/variable.cc @@ -25,10 +25,12 @@ namespace sem { Variable::Variable(const ast::Variable* declaration, const sem::Type* type, - ast::StorageClass storage_class) + ast::StorageClass storage_class, + ast::AccessControl::Access access_control) : declaration_(declaration), type_(type), storage_class_(storage_class), + access_control_(access_control), is_pipeline_constant_(false) {} Variable::Variable(const ast::Variable* declaration, @@ -37,6 +39,7 @@ Variable::Variable(const ast::Variable* declaration, : declaration_(declaration), type_(type), storage_class_(ast::StorageClass::kNone), + access_control_(ast::AccessControl::kInvalid), is_pipeline_constant_(true), constant_id_(constant_id) {} diff --git a/src/sem/variable.h b/src/sem/variable.h index ba6118fc38..49f23400e2 100644 --- a/src/sem/variable.h +++ b/src/sem/variable.h @@ -17,6 +17,7 @@ #include +#include "src/ast/access_control.h" #include "src/ast/storage_class.h" #include "src/sem/expression.h" @@ -41,9 +42,11 @@ class Variable : public Castable { /// @param declaration the AST declaration node /// @param type the variable type /// @param storage_class the variable storage class + /// @param access_control the variable access control type Variable(const ast::Variable* declaration, const sem::Type* type, - ast::StorageClass storage_class); + ast::StorageClass storage_class, + ast::AccessControl::Access access_control); /// Constructor for overridable pipeline constants /// @param declaration the AST declaration node @@ -65,6 +68,9 @@ class Variable : public Castable { /// @returns the storage class for the variable ast::StorageClass StorageClass() const { return storage_class_; } + /// @returns the access control for the variable + ast::AccessControl::Access AccessControl() const { return access_control_; } + /// @returns the expressions that use the variable const std::vector& Users() const { return users_; } @@ -81,6 +87,7 @@ class Variable : public Castable { const ast::Variable* const declaration_; const sem::Type* const type_; ast::StorageClass const storage_class_; + ast::AccessControl::Access const access_control_; std::vector users_; const bool is_pipeline_constant_; const uint16_t constant_id_ = 0; diff --git a/src/sem/vector_type_test.cc b/src/sem/vector_type_test.cc index 3ff6904986..2b2c48710c 100644 --- a/src/sem/vector_type_test.cc +++ b/src/sem/vector_type_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "src/sem/access_control_type.h" #include "src/sem/test_helper.h" #include "src/sem/texture_type.h" diff --git a/src/transform/binding_remapper.cc b/src/transform/binding_remapper.cc index 04e7739850..8a1c42cb47 100644 --- a/src/transform/binding_remapper.cc +++ b/src/transform/binding_remapper.cc @@ -19,7 +19,6 @@ #include "src/ast/disable_validation_decoration.h" #include "src/program_builder.h" -#include "src/sem/access_control_type.h" #include "src/sem/function.h" #include "src/sem/variable.h" @@ -115,12 +114,7 @@ Output BindingRemapper::Run(const Program* in, const DataMap& datamap) { if (ac_it != remappings->access_controls.end()) { ast::AccessControl::Access ac = ac_it->second; auto* ty = in->Sem().Get(var)->Type(); - ast::Type* inner_ty = nullptr; - if (auto* old_ac = ty->As()) { - inner_ty = CreateASTTypeFor(&ctx, old_ac->type()); - } else { - inner_ty = CreateASTTypeFor(&ctx, ty); - } + ast::Type* inner_ty = CreateASTTypeFor(&ctx, ty); auto* new_ty = ctx.dst->create(ac, inner_ty); auto* new_var = ctx.dst->create( ctx.Clone(var->source()), ctx.Clone(var->symbol()), diff --git a/src/transform/decompose_storage_access.cc b/src/transform/decompose_storage_access.cc index 75d98f51c0..eb4cdbc0b6 100644 --- a/src/transform/decompose_storage_access.cc +++ b/src/transform/decompose_storage_access.cc @@ -26,7 +26,6 @@ #include "src/ast/scalar_constructor_expression.h" #include "src/ast/type_name.h" #include "src/program_builder.h" -#include "src/sem/access_control_type.h" #include "src/sem/array.h" #include "src/sem/call.h" #include "src/sem/member_accessor_expression.h" @@ -338,10 +337,6 @@ const ast::NamedType* ConstructedTypeOf(const sem::Type* ty) { ty = ptr->type(); continue; } - if (auto* access = ty->As()) { - ty = access->type(); - continue; - } if (auto* str = ty->As()) { return str->Declaration(); } @@ -364,6 +359,17 @@ struct Store { StorageBufferAccess target; // The target for the write }; +ast::Type* MaybeCreateASTAccessControl(CloneContext* ctx, + const sem::VariableUser* var_user, + ast::Type* ty) { + if (var_user && + var_user->Variable()->AccessControl() != ast::AccessControl::kInvalid) { + return ctx->dst->create( + var_user->Variable()->AccessControl(), ty); + } + return ty; +} + } // namespace /// State holds the current transform state @@ -415,13 +421,17 @@ struct DecomposeStorageAccess::State { /// @param insert_after the user-declared type to insert the function after /// @param buf_ty the storage buffer type /// @param el_ty the storage buffer element type + /// @param var_user the variable user /// @return the name of the function that performs the load Symbol LoadFunc(CloneContext& ctx, const ast::NamedType* insert_after, const sem::Type* buf_ty, - const sem::Type* el_ty) { + const sem::Type* el_ty, + const sem::VariableUser* var_user) { return utils::GetOrCreate(load_funcs, TypePair{buf_ty, el_ty}, [&] { auto* buf_ast_ty = CreateASTTypeFor(&ctx, buf_ty); + buf_ast_ty = MaybeCreateASTAccessControl(&ctx, var_user, buf_ast_ty); + ast::VariableList params = { // Note: The buffer parameter requires the kStorage StorageClass in // order for HLSL to emit this as a ByteAddressBuffer. @@ -446,7 +456,7 @@ struct DecomposeStorageAccess::State { ast::ExpressionList values; if (auto* mat_ty = el_ty->As()) { auto* vec_ty = mat_ty->ColumnType(); - Symbol load = LoadFunc(ctx, insert_after, buf_ty, vec_ty); + Symbol load = LoadFunc(ctx, insert_after, buf_ty, vec_ty, var_user); for (uint32_t i = 0; i < mat_ty->columns(); i++) { auto* offset = ctx.dst->Add("offset", i * MatrixColumnStride(mat_ty)); @@ -456,14 +466,14 @@ struct DecomposeStorageAccess::State { for (auto* member : str->Members()) { auto* offset = ctx.dst->Add("offset", member->Offset()); Symbol load = LoadFunc(ctx, insert_after, buf_ty, - member->Type()->UnwrapAll()); + member->Type()->UnwrapAll(), var_user); values.emplace_back(ctx.dst->Call(load, "buffer", offset)); } } else if (auto* arr = el_ty->As()) { for (uint32_t i = 0; i < arr->Count(); i++) { auto* offset = ctx.dst->Add("offset", arr->Stride() * i); Symbol load = LoadFunc(ctx, insert_after, buf_ty, - arr->ElemType()->UnwrapAll()); + arr->ElemType()->UnwrapAll(), var_user); values.emplace_back(ctx.dst->Call(load, "buffer", offset)); } } @@ -487,13 +497,16 @@ struct DecomposeStorageAccess::State { /// @param insert_after the user-declared type to insert the function after /// @param buf_ty the storage buffer type /// @param el_ty the storage buffer element type + /// @param var_user the variable user /// @return the name of the function that performs the store Symbol StoreFunc(CloneContext& ctx, const ast::NamedType* insert_after, const sem::Type* buf_ty, - const sem::Type* el_ty) { + const sem::Type* el_ty, + const sem::VariableUser* var_user) { return utils::GetOrCreate(store_funcs, TypePair{buf_ty, el_ty}, [&] { auto* buf_ast_ty = CreateASTTypeFor(&ctx, buf_ty); + buf_ast_ty = MaybeCreateASTAccessControl(&ctx, var_user, buf_ast_ty); auto* el_ast_ty = CreateASTTypeFor(&ctx, el_ty); ast::VariableList params{ // Note: The buffer parameter requires the kStorage StorageClass in @@ -519,7 +532,7 @@ struct DecomposeStorageAccess::State { ast::StatementList body; if (auto* mat_ty = el_ty->As()) { auto* vec_ty = mat_ty->ColumnType(); - Symbol store = StoreFunc(ctx, insert_after, buf_ty, vec_ty); + Symbol store = StoreFunc(ctx, insert_after, buf_ty, vec_ty, var_user); for (uint32_t i = 0; i < mat_ty->columns(); i++) { auto* offset = ctx.dst->Add("offset", i * MatrixColumnStride(mat_ty)); @@ -533,7 +546,7 @@ struct DecomposeStorageAccess::State { auto* access = ctx.dst->MemberAccessor( "value", ctx.Clone(member->Declaration()->symbol())); Symbol store = StoreFunc(ctx, insert_after, buf_ty, - member->Type()->UnwrapAll()); + member->Type()->UnwrapAll(), var_user); auto* call = ctx.dst->Call(store, "buffer", offset, access); body.emplace_back(ctx.dst->create(call)); } @@ -542,7 +555,7 @@ struct DecomposeStorageAccess::State { auto* offset = ctx.dst->Add("offset", arr->Stride() * i); auto* access = ctx.dst->IndexAccessor("value", ctx.dst->Expr(i)); Symbol store = StoreFunc(ctx, insert_after, buf_ty, - arr->ElemType()->UnwrapAll()); + arr->ElemType()->UnwrapAll(), var_user); auto* call = ctx.dst->Call(store, "buffer", offset, access); body.emplace_back(ctx.dst->create(call)); } @@ -760,7 +773,8 @@ Output DecomposeStorageAccess::Run(const Program* in, const DataMap&) { auto* buf_ty = access.var->Type()->UnwrapPtr(); auto* el_ty = access.type->UnwrapAll(); auto* insert_after = ConstructedTypeOf(access.var->Type()); - Symbol func = state.LoadFunc(ctx, insert_after, buf_ty, el_ty); + Symbol func = state.LoadFunc(ctx, insert_after, buf_ty, el_ty, + access.var->As()); auto* load = ctx.dst->Call(func, ctx.Clone(buf), offset); @@ -775,7 +789,8 @@ Output DecomposeStorageAccess::Run(const Program* in, const DataMap&) { auto* el_ty = store.target.type->UnwrapAll(); auto* value = store.assignment->rhs(); auto* insert_after = ConstructedTypeOf(store.target.var->Type()); - Symbol func = state.StoreFunc(ctx, insert_after, buf_ty, el_ty); + Symbol func = state.StoreFunc(ctx, insert_after, buf_ty, el_ty, + store.target.var->As()); auto* call = ctx.dst->Call(func, ctx.Clone(buf), offset, ctx.Clone(value)); diff --git a/src/transform/transform.cc b/src/transform/transform.cc index b82a70681a..d7ead87424 100644 --- a/src/transform/transform.cc +++ b/src/transform/transform.cc @@ -103,10 +103,6 @@ ast::Type* Transform::CreateASTTypeFor(CloneContext* ctx, const sem::Type* ty) { } return ctx->dst->create(el, a->Count(), std::move(decos)); } - if (auto* ac = ty->As()) { - auto* el = CreateASTTypeFor(ctx, ac->type()); - return ctx->dst->create(ac->access_control(), el); - } if (auto* s = ty->As()) { return ctx->dst->create( ctx->Clone(s->Declaration()->name())); diff --git a/src/transform/transform_test.cc b/src/transform/transform_test.cc index 5d30d0f736..0e05161714 100644 --- a/src/transform/transform_test.cc +++ b/src/transform/transform_test.cc @@ -102,19 +102,6 @@ TEST_F(CreateASTTypeForTest, ArrayNonImplicitStride) { ->stride(), 32u); } -TEST_F(CreateASTTypeForTest, AccessControl) { - auto* ac = create([](ProgramBuilder& b) { - auto* decl = b.Structure("S", {}, {}); - auto* str = - b.create(decl, sem::StructMemberList{}, 4 /* align */, - 4 /* size */, 4 /* size_no_padding */); - return b.create(ast::AccessControl::kReadOnly, str); - }); - ASSERT_TRUE(ac->Is()); - EXPECT_EQ(ac->As()->access_control(), - ast::AccessControl::kReadOnly); - EXPECT_TRUE(ac->As()->type()->Is()); -} TEST_F(CreateASTTypeForTest, Struct) { auto* str = create([](ProgramBuilder& b) { diff --git a/src/transform/vertex_pulling.cc b/src/transform/vertex_pulling.cc index df33fa3be4..73287b95d8 100644 --- a/src/transform/vertex_pulling.cc +++ b/src/transform/vertex_pulling.cc @@ -214,7 +214,7 @@ struct State { ctx.dst->create(), }); for (uint32_t i = 0; i < cfg.vertex_state.size(); ++i) { - auto access = + auto* access = ctx.dst->ty.access(ast::AccessControl::kReadOnly, struct_type); // The decorated variable with struct type ctx.dst->Global( diff --git a/src/typepair.h b/src/typepair.h index bc3e98bce3..bcbf4c6d57 100644 --- a/src/typepair.h +++ b/src/typepair.h @@ -237,7 +237,6 @@ bool operator!=(std::nullptr_t, const TypePair& rhs) { using Type = TypePair; -using AccessControl = TypePair; using Array = TypePair; using Bool = TypePair; using DepthTexture = TypePair; diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index d76f4f153b..7b7ce20cc4 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -22,7 +22,6 @@ #include "src/ast/internal_decoration.h" #include "src/ast/override_decoration.h" #include "src/ast/variable_decl_statement.h" -#include "src/sem/access_control_type.h" #include "src/sem/array.h" #include "src/sem/call.h" #include "src/sem/depth_texture_type.h" @@ -247,7 +246,8 @@ bool GeneratorImpl::EmitBitcast(std::ostream& pre, } out << "as"; - if (!EmitType(out, type, ast::StorageClass::kNone, "")) { + if (!EmitType(out, type, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) { return false; } out << "("; @@ -1313,7 +1313,8 @@ bool GeneratorImpl::EmitTypeConstructor(std::ostream& pre, if (brackets) { out << "{"; } else { - if (!EmitType(out, type, ast::StorageClass::kNone, "")) { + if (!EmitType(out, type, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) { return false; } out << "("; @@ -1571,7 +1572,8 @@ bool GeneratorImpl::EmitFunctionInternal(std::ostream& out, Symbol ep_sym) { auto* func = builder_.Sem().Get(func_ast); - if (!EmitType(out, func->ReturnType(), ast::StorageClass::kNone, "")) { + if (!EmitType(out, func->ReturnType(), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) { return false; } @@ -1629,7 +1631,7 @@ bool GeneratorImpl::EmitFunctionInternal(std::ostream& out, // buffers. These functions have a storage buffer parameter with // StorageClass::kStorage. This is required to correctly translate the // parameter to [RW]ByteAddressBuffer. - if (!EmitType(out, type, v->StorageClass(), + if (!EmitType(out, type, v->StorageClass(), v->AccessControl(), builder_.Symbols().NameFor(v->Declaration()->symbol()))) { return false; } @@ -1716,7 +1718,7 @@ bool GeneratorImpl::EmitEntryPointData( increment_indent(); make_indent(out); - if (!EmitType(out, type, var->StorageClass(), "")) { + if (!EmitType(out, type, var->StorageClass(), var->AccessControl(), "")) { return false; } out << " " << builder_.Symbols().NameFor(decl->symbol()) << ";" @@ -1741,18 +1743,21 @@ bool GeneratorImpl::EmitEntryPointData( continue; // Global already emitted } - auto* access = var->Type()->As(); - if (access == nullptr) { + if (var->AccessControl() == ast::AccessControl::kInvalid) { diagnostics_.add_error("access control type required for storage buffer"); return false; } - if (!EmitType(out, var->Type(), ast::StorageClass::kStorage, "")) { + if (!EmitType(out, var->Type(), ast::StorageClass::kStorage, + var->AccessControl(), "")) { return false; } out << " " << builder_.Symbols().NameFor(decl->symbol()) - << RegisterAndSpace(access->IsReadOnly() ? 't' : 'u', binding_point) + << RegisterAndSpace( + var->AccessControl() == ast::AccessControl::kReadOnly ? 't' + : 'u', + binding_point) << ";" << std::endl; emitted_storagebuffer = true; } @@ -1779,7 +1784,7 @@ bool GeneratorImpl::EmitEntryPointData( auto* type = sem->Type(); make_indent(out); - if (!EmitType(out, type, sem->StorageClass(), + if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), builder_.Symbols().NameFor(var->symbol()))) { return false; } @@ -1830,7 +1835,7 @@ bool GeneratorImpl::EmitEntryPointData( auto* type = sem->Type(); make_indent(out); - if (!EmitType(out, type, sem->StorageClass(), + if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), builder_.Symbols().NameFor(var->symbol()))) { return false; } @@ -1898,7 +1903,8 @@ bool GeneratorImpl::EmitEntryPointData( } auto name = builder_.Symbols().NameFor(decl->symbol()); - if (!EmitType(out, var->Type(), var->StorageClass(), name)) { + if (!EmitType(out, var->Type(), var->StorageClass(), var->AccessControl(), + name)) { return false; } if (!var->Type()->Is()) { @@ -1909,11 +1915,9 @@ bool GeneratorImpl::EmitEntryPointData( if (unwrapped_type->Is()) { register_space = "t"; - if (unwrapped_type->Is()) { - if (auto* ac = var->Type()->As()) { - if (!ac->IsReadOnly()) { - register_space = "u"; - } + if (auto* storage_tex = unwrapped_type->As()) { + if (storage_tex->access_control() != ast::AccessControl::kReadOnly) { + register_space = "u"; } } } else if (unwrapped_type->Is()) { @@ -2030,7 +2034,7 @@ bool GeneratorImpl::EmitEntryPointFunction(std::ostream& out, } first = false; - if (!EmitType(out, type, sem->StorageClass(), "")) { + if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), "")) { return false; } @@ -2098,7 +2102,8 @@ bool GeneratorImpl::EmitZeroValue(std::ostream& out, sem::Type* type) { } else if (type->Is()) { out << "0u"; } else if (auto* vec = type->As()) { - if (!EmitType(out, type, ast::StorageClass::kNone, "")) { + if (!EmitType(out, type, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) { return false; } ScopedParen sp(out); @@ -2111,7 +2116,8 @@ bool GeneratorImpl::EmitZeroValue(std::ostream& out, sem::Type* type) { } } } else if (auto* mat = type->As()) { - if (!EmitType(out, type, ast::StorageClass::kNone, "")) { + if (!EmitType(out, type, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) { return false; } ScopedParen sp(out); @@ -2368,19 +2374,10 @@ bool GeneratorImpl::EmitSwitch(std::ostream& out, ast::SwitchStatement* stmt) { bool GeneratorImpl::EmitType(std::ostream& out, const sem::Type* type, ast::StorageClass storage_class, + ast::AccessControl::Access access_control, const std::string& name) { - auto* access = type->As(); - if (access) { - type = access->type(); - } - if (storage_class == ast::StorageClass::kStorage) { - if (access == nullptr) { - diagnostics_.add_error("access control type required for storage buffer"); - return false; - } - - if (!access->IsReadOnly()) { + if (access_control != ast::AccessControl::kReadOnly) { out << "RW"; } out << "ByteAddressBuffer"; @@ -2400,7 +2397,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, sizes.push_back(arr->Count()); base_type = arr->ElemType(); } - if (!EmitType(out, base_type, storage_class, "")) { + if (!EmitType(out, base_type, storage_class, access_control, "")) { return false; } if (!name.empty()) { @@ -2416,7 +2413,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, } else if (type->Is()) { out << "int"; } else if (auto* mat = type->As()) { - if (!EmitType(out, mat->type(), storage_class, "")) { + if (!EmitType(out, mat->type(), storage_class, access_control, "")) { return false; } // Note: HLSL's matrices are declared as NxM, where N is the number of @@ -2446,7 +2443,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, auto* sampled = tex->As(); if (storage) { - if (access && !access->IsReadOnly()) { + if (access_control != ast::AccessControl::kReadOnly) { out << "RW"; } } @@ -2512,7 +2509,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, out << "uint" << size; } else { out << "vector<"; - if (!EmitType(out, vec->type(), storage_class, "")) { + if (!EmitType(out, vec->type(), storage_class, access_control, "")) { return false; } out << ", " << size << ">"; @@ -2548,7 +2545,8 @@ bool GeneratorImpl::EmitStructType(std::ostream& out, // https://bugs.chromium.org/p/tint/issues/detail?id=184 auto mem_name = builder_.Symbols().NameFor(mem->Declaration()->symbol()); - if (!EmitType(out, mem->Type(), ast::StorageClass::kNone, mem_name)) { + if (!EmitType(out, mem->Type(), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, mem_name)) { return false; } // Array member name will be output with the type @@ -2650,7 +2648,7 @@ bool GeneratorImpl::EmitVariable(std::ostream& out, } auto* sem = builder_.Sem().Get(var); auto* type = sem->Type(); - if (!EmitType(out, type, sem->StorageClass(), + if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), builder_.Symbols().NameFor(var->symbol()))) { return false; } @@ -2703,7 +2701,7 @@ bool GeneratorImpl::EmitProgramConstVariable(std::ostream& out, } out << "#endif" << std::endl; out << "static const "; - if (!EmitType(out, type, sem->StorageClass(), + if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), builder_.Symbols().NameFor(var->symbol()))) { return false; } @@ -2712,7 +2710,7 @@ bool GeneratorImpl::EmitProgramConstVariable(std::ostream& out, out << "#undef WGSL_SPEC_CONSTANT_" << const_id << std::endl; } else { out << "static const "; - if (!EmitType(out, type, sem->StorageClass(), + if (!EmitType(out, type, sem->StorageClass(), sem->AccessControl(), builder_.Symbols().NameFor(var->symbol()))) { return false; } diff --git a/src/writer/hlsl/generator_impl.h b/src/writer/hlsl/generator_impl.h index b473ec084c..2fb4fedf1a 100644 --- a/src/writer/hlsl/generator_impl.h +++ b/src/writer/hlsl/generator_impl.h @@ -287,11 +287,13 @@ class GeneratorImpl : public TextGenerator { /// @param out the output stream /// @param type the type to generate /// @param storage_class the storage class of the variable + /// @param access_control the access control type of the variable /// @param name the name of the variable, only used for array emission /// @returns true if the type is emitted bool EmitType(std::ostream& out, const sem::Type* type, ast::StorageClass storage_class, + ast::AccessControl::Access access_control, const std::string& name); /// Handles generating a structure declaration /// @param out the output stream diff --git a/src/writer/hlsl/generator_impl_function_test.cc b/src/writer/hlsl/generator_impl_function_test.cc index cad1739229..68f55c7257 100644 --- a/src/writer/hlsl/generator_impl_function_test.cc +++ b/src/writer/hlsl/generator_impl_function_test.cc @@ -17,7 +17,6 @@ #include "src/ast/struct_block_decoration.h" #include "src/ast/variable_decl_statement.h" #include "src/ast/workgroup_decoration.h" -#include "src/sem/access_control_type.h" #include "src/writer/hlsl/test_helper.h" using ::testing::HasSubstr; @@ -408,7 +407,7 @@ TEST_F(HlslGeneratorImplTest_Function, }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, { @@ -454,7 +453,7 @@ TEST_F(HlslGeneratorImplTest_Function, }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, { @@ -500,7 +499,7 @@ TEST_F(HlslGeneratorImplTest_Function, }, {create()}); - auto ac = ty.access(ast::AccessControl::kWriteOnly, s); + auto* ac = ty.access(ast::AccessControl::kWriteOnly, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, { @@ -543,7 +542,7 @@ TEST_F(HlslGeneratorImplTest_Function, }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, { @@ -794,7 +793,7 @@ TEST_F(HlslGeneratorImplTest_Function, Emit_Decoration_Called_By_EntryPoint_With_StorageBuffer) { auto* s = Structure("S", {Member("x", ty.f32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, { create(0), @@ -984,7 +983,7 @@ TEST_F(HlslGeneratorImplTest_Function, auto* s = Structure("Data", {Member("d", ty.f32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("data", ac, ast::StorageClass::kStorage, nullptr, { diff --git a/src/writer/hlsl/generator_impl_member_accessor_test.cc b/src/writer/hlsl/generator_impl_member_accessor_test.cc index 92e13753a5..8f0d658eb5 100644 --- a/src/writer/hlsl/generator_impl_member_accessor_test.cc +++ b/src/writer/hlsl/generator_impl_member_accessor_test.cc @@ -15,7 +15,6 @@ #include "gmock/gmock.h" #include "src/ast/stage_decoration.h" #include "src/ast/struct_block_decoration.h" -#include "src/sem/access_control_type.h" #include "src/writer/hlsl/test_helper.h" namespace tint { @@ -99,7 +98,7 @@ class HlslGeneratorImplTest_MemberAccessorBase : public BASE { auto* s = b.Structure("Data", members, {b.create()}); - auto ac_ty = b.ty.access(ast::AccessControl::kReadWrite, s); + auto* ac_ty = b.ty.access(ast::AccessControl::kReadWrite, s); b.Global("data", ac_ty, ast::StorageClass::kStorage, nullptr, ast::DecorationList{ diff --git a/src/writer/hlsl/generator_impl_sanitizer_test.cc b/src/writer/hlsl/generator_impl_sanitizer_test.cc index 107eded940..6a16f4ca79 100644 --- a/src/writer/hlsl/generator_impl_sanitizer_test.cc +++ b/src/writer/hlsl/generator_impl_sanitizer_test.cc @@ -15,7 +15,6 @@ #include "src/ast/stage_decoration.h" #include "src/ast/struct_block_decoration.h" #include "src/ast/variable_decl_statement.h" -#include "src/sem/access_control_type.h" #include "src/writer/hlsl/test_helper.h" namespace tint { @@ -34,7 +33,7 @@ TEST_F(HlslSanitizerTest, ArrayLength) { { create(), }); - auto ac_ty = ty.access(ast::AccessControl::kReadOnly, sb_ty); + auto* ac_ty = ty.access(ast::AccessControl::kReadOnly, sb_ty); Global("sb", ac_ty, ast::StorageClass::kStorage, nullptr, ast::DecorationList{ diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc index e3252f0899..288b6049c1 100644 --- a/src/writer/hlsl/generator_impl_type_test.cc +++ b/src/writer/hlsl/generator_impl_type_test.cc @@ -16,7 +16,6 @@ #include "src/ast/call_statement.h" #include "src/ast/stage_decoration.h" #include "src/ast/struct_block_decoration.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/sampled_texture_type.h" @@ -38,8 +37,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Array) { GeneratorImpl& gen = Build(); - ASSERT_TRUE( - gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, "ary")) + ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "ary")) << gen.error(); EXPECT_EQ(result(), "bool ary[4]"); } @@ -50,8 +49,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_ArrayOfArray) { GeneratorImpl& gen = Build(); - ASSERT_TRUE( - gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, "ary")) + ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "ary")) << gen.error(); EXPECT_EQ(result(), "bool ary[5][4]"); } @@ -64,8 +63,8 @@ TEST_F(HlslGeneratorImplTest_Type, GeneratorImpl& gen = Build(); - ASSERT_TRUE( - gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, "ary")) + ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "ary")) << gen.error(); EXPECT_EQ(result(), "bool ary[5][4][1]"); } @@ -76,8 +75,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_ArrayOfArrayOfArray) { GeneratorImpl& gen = Build(); - ASSERT_TRUE( - gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, "ary")) + ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "ary")) << gen.error(); EXPECT_EQ(result(), "bool ary[6][5][4]"); } @@ -88,8 +87,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Array_WithoutName) { GeneratorImpl& gen = Build(); - ASSERT_TRUE( - gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "bool[4]"); } @@ -100,8 +99,8 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_RuntimeArray) { GeneratorImpl& gen = Build(); - ASSERT_TRUE( - gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, "ary")) + ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "ary")) << gen.error(); EXPECT_EQ(result(), "bool ary[]"); } @@ -111,7 +110,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Bool) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, bool_, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, bool_, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "bool"); } @@ -121,7 +121,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_F32) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, f32, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, f32, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "float"); } @@ -131,7 +132,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_I32) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, i32, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, i32, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "int"); } @@ -141,7 +143,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Matrix) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, mat2x3, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, mat2x3, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "float2x3"); } @@ -152,7 +155,8 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Pointer) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, &p, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, &p, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "float*"); } @@ -206,7 +210,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Struct) { GeneratorImpl& gen = Build(); auto* sem_s = program->TypeOf(s)->As(); - ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "S"); } @@ -225,7 +230,8 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Struct_InjectPadding) { GeneratorImpl& gen = Build(); auto* sem_s = program->TypeOf(s)->As(); - ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(gen.result(), R"(struct S { int a; @@ -280,7 +286,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_U32) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, u32, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, u32, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "uint"); } @@ -290,7 +297,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Vector) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, vec3, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, vec3, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "float3"); } @@ -300,7 +308,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Void) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, void_, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, void_, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "void"); } @@ -310,7 +319,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitSampler) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, &sampler, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, &sampler, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "SamplerState"); } @@ -320,7 +330,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitSamplerComparison) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, &sampler, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, &sampler, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "SamplerComparisonState"); } @@ -515,7 +526,8 @@ TEST_F(HlslGeneratorImplTest_Type, EmitMultisampledTexture) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(out, &s, ast::StorageClass::kNone, "")) + ASSERT_TRUE(gen.EmitType(out, &s, ast::StorageClass::kNone, + ast::AccessControl::kInvalid, "")) << gen.error(); EXPECT_EQ(result(), "Texture2DMS"); } @@ -536,9 +548,9 @@ TEST_P(HlslStorageTexturesTest, Emit) { auto params = GetParam(); auto t = ty.storage_texture(params.dim, params.imgfmt); - auto ac = ty.access(params.ro ? ast::AccessControl::kReadOnly - : ast::AccessControl::kWriteOnly, - t); + auto* ac = ty.access(params.ro ? ast::AccessControl::kReadOnly + : ast::AccessControl::kWriteOnly, + t); Global("tex", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index 09961271db..79296e5585 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -30,7 +30,6 @@ #include "src/ast/uint_literal.h" #include "src/ast/variable_decl_statement.h" #include "src/ast/void.h" -#include "src/sem/access_control_type.h" #include "src/sem/array.h" #include "src/sem/bool_type.h" #include "src/sem/call.h" @@ -1296,18 +1295,12 @@ bool GeneratorImpl::EmitFunctionInternal(ast::Function* func, } first = false; - auto* ac = var->Type()->As(); - if (ac == nullptr) { - diagnostics_.add_error( - "invalid type for storage buffer, expected access control"); - return false; - } - if (ac->IsReadOnly()) { + if (var->AccessControl() == ast::AccessControl::kReadOnly) { out_ << "const "; } out_ << "device "; - if (!EmitType(ac->type(), "")) { + if (!EmitType(var->Type(), "")) { return false; } out_ << "& " << program_->Symbols().NameFor(var->Declaration()->symbol()); @@ -1524,18 +1517,12 @@ bool GeneratorImpl::EmitEntryPointFunction(ast::Function* func) { auto* binding = data.second.binding; // auto* set = data.second.set; - auto* ac = var->Type()->As(); - if (ac == nullptr) { - diagnostics_.add_error( - "invalid type for storage buffer, expected access control"); - return false; - } - if (ac->IsReadOnly()) { + if (var->AccessControl() == ast::AccessControl::kReadOnly) { out_ << "const "; } out_ << "device "; - if (!EmitType(ac->type(), "")) { + if (!EmitType(var->Type(), "")) { return false; } out_ << "& " << program_->Symbols().NameFor(var->Declaration()->symbol()) @@ -1892,20 +1879,6 @@ bool GeneratorImpl::EmitSwitch(ast::SwitchStatement* stmt) { } bool GeneratorImpl::EmitType(const sem::Type* type, const std::string& name) { - std::string access_str = ""; - if (auto* ac = type->As()) { - if (ac->access_control() == ast::AccessControl::kReadOnly) { - access_str = "read"; - } else if (ac->access_control() == ast::AccessControl::kWriteOnly) { - access_str = "write"; - } else { - diagnostics_.add_error("Invalid access control for storage texture"); - return false; - } - - type = ac->type(); - } - if (auto* ary = type->As()) { const sem::Type* base_type = ary; std::vector sizes; @@ -1989,7 +1962,16 @@ bool GeneratorImpl::EmitType(const sem::Type* type, const std::string& name) { if (!EmitType(storage->type(), "")) { return false; } - out_ << ", access::" << access_str; + + std::string access_str; + if (storage->access_control() == ast::AccessControl::kReadOnly) { + out_ << ", access::read"; + } else if (storage->access_control() == ast::AccessControl::kWriteOnly) { + out_ << ", access::write"; + } else { + diagnostics_.add_error("Invalid access control for storage texture"); + return false; + } } else if (auto* ms = tex->As()) { if (!EmitType(ms->type(), "")) { return false; diff --git a/src/writer/msl/generator_impl_function_test.cc b/src/writer/msl/generator_impl_function_test.cc index aab822693a..dafc9ca2ee 100644 --- a/src/writer/msl/generator_impl_function_test.cc +++ b/src/writer/msl/generator_impl_function_test.cc @@ -15,7 +15,6 @@ #include "src/ast/stage_decoration.h" #include "src/ast/struct_block_decoration.h" #include "src/ast/variable_decl_statement.h" -#include "src/sem/access_control_type.h" #include "src/writer/msl/test_helper.h" namespace tint { @@ -307,7 +306,7 @@ TEST_F(MslGeneratorImplTest, }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, {create(0), create(1)}); @@ -352,7 +351,7 @@ TEST_F(MslGeneratorImplTest, }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, {create(0), create(1)}); @@ -608,7 +607,7 @@ TEST_F(MslGeneratorImplTest, }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, {create(0), create(1)}); @@ -664,7 +663,7 @@ TEST_F(MslGeneratorImplTest, }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("coord", ac, ast::StorageClass::kStorage, nullptr, {create(0), create(1)}); @@ -799,7 +798,7 @@ TEST_F(MslGeneratorImplTest, auto* s = Structure("Data", {Member("d", ty.f32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("data", ac, ast::StorageClass::kStorage, nullptr, {create(0), create(0)}); diff --git a/src/writer/msl/generator_impl_type_test.cc b/src/writer/msl/generator_impl_type_test.cc index b80e3e5ebf..7e5ea8a082 100644 --- a/src/writer/msl/generator_impl_type_test.cc +++ b/src/writer/msl/generator_impl_type_test.cc @@ -15,7 +15,6 @@ #include #include "src/ast/struct_block_decoration.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/sampled_texture_type.h" @@ -762,9 +761,9 @@ TEST_P(MslStorageTexturesTest, Emit) { auto params = GetParam(); auto s = ty.storage_texture(params.dim, ast::ImageFormat::kR32Float); - auto ac = ty.access(params.ro ? ast::AccessControl::kReadOnly - : ast::AccessControl::kWriteOnly, - s); + auto* ac = ty.access(params.ro ? ast::AccessControl::kReadOnly + : ast::AccessControl::kWriteOnly, + s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { create(0), @@ -773,7 +772,7 @@ TEST_P(MslStorageTexturesTest, Emit) { GeneratorImpl& gen = Build(); - ASSERT_TRUE(gen.EmitType(ac, "")) << gen.error(); + ASSERT_TRUE(gen.EmitType(program->TypeOf(ac), "")) << gen.error(); EXPECT_EQ(gen.result(), params.result); } INSTANTIATE_TEST_SUITE_P( diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 2fd180a7a6..d3ca53c02d 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -728,34 +728,35 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { OperandList ops = {Operand::Int(type_id), result, Operand::Int(ConvertStorageClass(sc))}; - // Unwrap after emitting the access control as unwrap all removes access - // control types. - auto* type_no_ac = sem->Type()->UnwrapAll(); + auto* type = sem->Type(); + if (var->has_constructor()) { ops.push_back(Operand::Int(init_id)); - } else if (type_no_ac->Is()) { - if (auto* ac = sem->Type()->As()) { - switch (ac->access_control()) { - case ast::AccessControl::kWriteOnly: - push_annot( - spv::Op::OpDecorate, - {Operand::Int(var_id), Operand::Int(SpvDecorationNonReadable)}); - break; - case ast::AccessControl::kReadOnly: - push_annot( - spv::Op::OpDecorate, - {Operand::Int(var_id), Operand::Int(SpvDecorationNonWritable)}); - break; - case ast::AccessControl::kReadWrite: - break; - } + } else if (sem->AccessControl() != ast::AccessControl::kInvalid) { + // type is a sem::Struct or a sem::StorageTexture + switch (sem->AccessControl()) { + case ast::AccessControl::kInvalid: + TINT_ICE(builder_.Diagnostics()) << "missing access control"; + break; + case ast::AccessControl::kWriteOnly: + push_annot( + spv::Op::OpDecorate, + {Operand::Int(var_id), Operand::Int(SpvDecorationNonReadable)}); + break; + case ast::AccessControl::kReadOnly: + push_annot( + spv::Op::OpDecorate, + {Operand::Int(var_id), Operand::Int(SpvDecorationNonWritable)}); + break; + case ast::AccessControl::kReadWrite: + break; } - } else if (!type_no_ac->Is()) { + } else if (!type->Is()) { // If we don't have a constructor and we're an Output or Private variable, // then WGSL requires that we zero-initialize. if (sem->StorageClass() == ast::StorageClass::kPrivate || sem->StorageClass() == ast::StorageClass::kOutput) { - init_id = GenerateConstantNullIfNeeded(type_no_ac); + init_id = GenerateConstantNullIfNeeded(type); if (init_id == 0) { return 0; } @@ -2931,12 +2932,6 @@ uint32_t Builder::GenerateTypeIfNeeded(const sem::Type* type) { return 0; } - if (auto* ac = type->As()) { - if (!ac->type()->UnwrapAccess()->Is()) { - return GenerateTypeIfNeeded(ac->type()); - } - } - auto val = type_name_to_id_.find(type->type_name()); if (val != type_name_to_id_.end()) { return val->second; @@ -2944,14 +2939,7 @@ uint32_t Builder::GenerateTypeIfNeeded(const sem::Type* type) { auto result = result_op(); auto id = result.to_i(); - if (auto* ac = type->As()) { - // The non-struct case was handled above. - auto* subtype = ac->UnwrapAccess(); - if (!GenerateStructType(subtype->As(), ac->access_control(), - result)) { - return 0; - } - } else if (auto* arr = type->As()) { + if (auto* arr = type->As()) { if (!GenerateArrayType(arr, result)) { return 0; } @@ -2970,7 +2958,7 @@ uint32_t Builder::GenerateTypeIfNeeded(const sem::Type* type) { return 0; } } else if (auto* str = type->As()) { - if (!GenerateStructType(str, ast::AccessControl::kReadWrite, result)) { + if (!GenerateStructType(str, result)) { return 0; } } else if (type->Is()) { @@ -2985,6 +2973,28 @@ uint32_t Builder::GenerateTypeIfNeeded(const sem::Type* type) { if (!GenerateTextureType(tex, result)) { return 0; } + + if (auto* st = tex->As()) { + // Register all three access types of StorageTexture names. In SPIR-V, we + // must output a single type, while the variable is annotated with the + // access type. Doing this ensures we de-dupe. + type_name_to_id_[builder_ + .create( + st->dim(), st->image_format(), + ast::AccessControl::kReadOnly, st->type()) + ->type_name()] = id; + type_name_to_id_[builder_ + .create( + st->dim(), st->image_format(), + ast::AccessControl::kWriteOnly, st->type()) + ->type_name()] = id; + type_name_to_id_[builder_ + .create( + st->dim(), st->image_format(), + ast::AccessControl::kReadWrite, st->type()) + ->type_name()] = id; + } + } else if (type->Is()) { push_type(spv::Op::OpTypeSampler, {result}); @@ -3139,7 +3149,6 @@ bool Builder::GeneratePointerType(const sem::Pointer* ptr, } bool Builder::GenerateStructType(const sem::Struct* struct_type, - ast::AccessControl::Access access_control, const Operand& result) { auto struct_id = result.to_i(); auto* impl = struct_type->Declaration(); @@ -3165,19 +3174,6 @@ bool Builder::GenerateStructType(const sem::Struct* struct_type, return false; } - // We're attaching the access control to the members of the struct instead - // of to the variable. The reason we do this is that WGSL models the - // access as part of the type. If we attach to the variable, it's no - // longer part of the type in the SPIR-V backend, but part of the - // variable. This differs from the modeling and other backends. Attaching - // to the struct members means the access control stays part of the type - // where it logically makes the most sense. - if (access_control == ast::AccessControl::kReadOnly) { - push_annot(spv::Op::OpMemberDecorate, - {Operand::Int(struct_id), Operand::Int(i), - Operand::Int(SpvDecorationNonWritable)}); - } - ops.push_back(Operand::Int(mem_id)); } diff --git a/src/writer/spirv/builder.h b/src/writer/spirv/builder.h index 7163e275e5..53c917b8f6 100644 --- a/src/writer/spirv/builder.h +++ b/src/writer/spirv/builder.h @@ -34,7 +34,6 @@ #include "src/ast/variable_decl_statement.h" #include "src/program_builder.h" #include "src/scope_stack.h" -#include "src/sem/access_control_type.h" #include "src/sem/storage_texture_type.h" #include "src/writer/spirv/function.h" #include "src/writer/spirv/scalar_constant.h" @@ -446,11 +445,9 @@ class Builder { bool GeneratePointerType(const sem::Pointer* ptr, const Operand& result); /// Generates a vector type declaration /// @param struct_type the vector to generate - /// @param access_control the access controls to assign to the struct /// @param result the result operand /// @returns true if the vector was successfully generated bool GenerateStructType(const sem::Struct* struct_type, - ast::AccessControl::Access access_control, const Operand& result); /// Generates a struct member /// @param struct_id the id of the parent structure diff --git a/src/writer/spirv/builder_function_test.cc b/src/writer/spirv/builder_function_test.cc index a2da8e5b86..a3efcf79e6 100644 --- a/src/writer/spirv/builder_function_test.cc +++ b/src/writer/spirv/builder_function_test.cc @@ -204,7 +204,7 @@ TEST_F(BuilderTest, Emit_Multiple_EntryPoint_With_Same_ModuleVar) { auto* s = Structure("Data", {Member("d", ty.f32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("data", ac, ast::StorageClass::kStorage, nullptr, ast::DecorationList{ diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc index d3db49ff72..fc1ee17cda 100644 --- a/src/writer/spirv/builder_global_variable_test.cc +++ b/src/writer/spirv/builder_global_variable_test.cc @@ -408,7 +408,7 @@ TEST_F(BuilderTest, GlobalVar_DeclReadOnly) { Member("b", ty.i32()), }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, A); + auto* ac = ty.access(ast::AccessControl::kReadOnly, A); auto* var = Global("b", ac, ast::StorageClass::kStorage, nullptr, { @@ -422,9 +422,8 @@ TEST_F(BuilderTest, GlobalVar_DeclReadOnly) { EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %3 Block OpMemberDecorate %3 0 Offset 0 -OpMemberDecorate %3 0 NonWritable OpMemberDecorate %3 1 Offset 4 -OpMemberDecorate %3 1 NonWritable +OpDecorate %1 NonWritable OpDecorate %1 Binding 0 OpDecorate %1 DescriptorSet 0 )"); @@ -451,7 +450,7 @@ TEST_F(BuilderTest, GlobalVar_TypeAliasDeclReadOnly) { {create()}); auto* B = ty.alias("B", A); AST().AddConstructedType(B); - auto ac = ty.access(ast::AccessControl::kReadOnly, B); + auto* ac = ty.access(ast::AccessControl::kReadOnly, B); auto* var = Global("b", ac, ast::StorageClass::kStorage, nullptr, { create(0), @@ -464,7 +463,7 @@ TEST_F(BuilderTest, GlobalVar_TypeAliasDeclReadOnly) { EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %3 Block OpMemberDecorate %3 0 Offset 0 -OpMemberDecorate %3 0 NonWritable +OpDecorate %1 NonWritable OpDecorate %1 Binding 0 OpDecorate %1 DescriptorSet 0 )"); @@ -488,7 +487,7 @@ TEST_F(BuilderTest, GlobalVar_TypeAliasAssignReadOnly) { auto* A = Structure("A", {Member("a", ty.i32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, A); + auto* ac = ty.access(ast::AccessControl::kReadOnly, A); auto* B = ty.alias("B", ac); AST().AddConstructedType(B); auto* var = Global("b", B, ast::StorageClass::kStorage, nullptr, @@ -503,7 +502,7 @@ TEST_F(BuilderTest, GlobalVar_TypeAliasAssignReadOnly) { EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %3 Block OpMemberDecorate %3 0 Offset 0 -OpMemberDecorate %3 0 NonWritable +OpDecorate %1 NonWritable OpDecorate %1 Binding 0 OpDecorate %1 DescriptorSet 0 )"); @@ -527,8 +526,8 @@ TEST_F(BuilderTest, GlobalVar_TwoVarDeclReadOnly) { auto* A = Structure("A", {Member("a", ty.i32())}, {create()}); - auto read = ty.access(ast::AccessControl::kReadOnly, A); - auto rw = ty.access(ast::AccessControl::kReadWrite, A); + auto* read = ty.access(ast::AccessControl::kReadOnly, A); + auto* rw = ty.access(ast::AccessControl::kReadWrite, A); auto* var_b = Global("b", read, ast::StorageClass::kStorage, nullptr, { @@ -549,28 +548,22 @@ TEST_F(BuilderTest, GlobalVar_TwoVarDeclReadOnly) { EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %3 Block OpMemberDecorate %3 0 Offset 0 -OpMemberDecorate %3 0 NonWritable +OpDecorate %1 NonWritable OpDecorate %1 DescriptorSet 0 OpDecorate %1 Binding 0 -OpDecorate %7 Block -OpMemberDecorate %7 0 Offset 0 OpDecorate %5 DescriptorSet 1 OpDecorate %5 Binding 0 )"); EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %3 "A" OpMemberName %3 0 "a" OpName %1 "b" -OpName %7 "A" -OpMemberName %7 0 "a" OpName %5 "c" )"); EXPECT_EQ(DumpInstructions(b.types()), R"(%4 = OpTypeInt 32 1 %3 = OpTypeStruct %4 %2 = OpTypePointer StorageBuffer %3 %1 = OpVariable %2 StorageBuffer -%7 = OpTypeStruct %4 -%6 = OpTypePointer StorageBuffer %7 -%5 = OpVariable %6 StorageBuffer +%5 = OpVariable %2 StorageBuffer )"); } @@ -580,7 +573,7 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageReadOnly) { auto type = ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Uint); - auto ac = ty.access(ast::AccessControl::kReadOnly, type); + auto* ac = ty.access(ast::AccessControl::kReadOnly, type); auto* var_a = Global("a", ac, ast::StorageClass::kNone, nullptr, { @@ -609,7 +602,7 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageWriteOnly) { auto type = ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Uint); - auto ac = ty.access(ast::AccessControl::kWriteOnly, type); + auto* ac = ty.access(ast::AccessControl::kWriteOnly, type); auto* var_a = Global("a", ac, ast::StorageClass::kNone, nullptr, { @@ -638,18 +631,18 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageWithDifferentAccess) { // var a : [[access(read)]] texture_storage_2d; // var b : [[access(write)]] texture_storage_2d; - auto type_a = ty.access(ast::AccessControl::kReadOnly, - ty.storage_texture(ast::TextureDimension::k2d, - ast::ImageFormat::kR32Uint)); + auto* type_a = ty.access(ast::AccessControl::kReadOnly, + ty.storage_texture(ast::TextureDimension::k2d, + ast::ImageFormat::kR32Uint)); auto* var_a = Global("a", type_a, ast::StorageClass::kNone, nullptr, { create(0), create(0), }); - auto type_b = ty.access(ast::AccessControl::kWriteOnly, - ty.storage_texture(ast::TextureDimension::k2d, - ast::ImageFormat::kR32Uint)); + auto* type_b = ty.access(ast::AccessControl::kWriteOnly, + ty.storage_texture(ast::TextureDimension::k2d, + ast::ImageFormat::kR32Uint)); auto* var_b = Global("b", type_b, ast::StorageClass::kNone, nullptr, { create(1), diff --git a/src/writer/spirv/builder_intrinsic_test.cc b/src/writer/spirv/builder_intrinsic_test.cc index 9b9f4db6db..bca5f4af93 100644 --- a/src/writer/spirv/builder_intrinsic_test.cc +++ b/src/writer/spirv/builder_intrinsic_test.cc @@ -1389,7 +1389,7 @@ OpFunctionEnd TEST_F(IntrinsicBuilderTest, Call_ArrayLength) { auto* s = Structure("my_struct", {Member(0, "a", ty.array(4))}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("b", ac, ast::StorageClass::kStorage, nullptr, ast::DecorationList{ create(1), @@ -1439,7 +1439,7 @@ TEST_F(IntrinsicBuilderTest, Call_ArrayLength_OtherMembersInStruct) { Member(4, "a", ty.array(4)), }, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("b", ac, ast::StorageClass::kStorage, nullptr, ast::DecorationList{ create(1), diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc index fdb2d767d6..bccd360da3 100644 --- a/src/writer/spirv/builder_type_test.cc +++ b/src/writer/spirv/builder_type_test.cc @@ -30,7 +30,7 @@ TEST_F(BuilderTest_Type, GenerateRuntimeArray) { auto* ary = ty.array(ty.i32(), 0); auto* str = Structure("S", {Member("x", ary)}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, str); + auto* ac = ty.access(ast::AccessControl::kReadOnly, str); Global("a", ac, ast::StorageClass::kStorage, nullptr, { create(0), @@ -52,7 +52,7 @@ TEST_F(BuilderTest_Type, ReturnsGeneratedRuntimeArray) { auto* ary = ty.array(ty.i32(), 0); auto* str = Structure("S", {Member("x", ary)}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadOnly, str); + auto* ac = ty.access(ast::AccessControl::kReadOnly, str); Global("a", ac, ast::StorageClass::kStorage, nullptr, { create(0), @@ -822,7 +822,7 @@ TEST_F(BuilderTest_Type, SampledTexture_Generate_CubeArray) { TEST_F(BuilderTest_Type, StorageTexture_Generate_1d) { auto s = ty.storage_texture(ast::TextureDimension::k1d, ast::ImageFormat::kR32Float); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { @@ -842,7 +842,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_1d) { TEST_F(BuilderTest_Type, StorageTexture_Generate_2d) { auto s = ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Float); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { @@ -862,7 +862,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_2d) { TEST_F(BuilderTest_Type, StorageTexture_Generate_2dArray) { auto s = ty.storage_texture(ast::TextureDimension::k2dArray, ast::ImageFormat::kR32Float); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { @@ -882,7 +882,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_2dArray) { TEST_F(BuilderTest_Type, StorageTexture_Generate_3d) { auto s = ty.storage_texture(ast::TextureDimension::k3d, ast::ImageFormat::kR32Float); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { @@ -903,7 +903,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_SampledTypeFloat_Format_r32float) { auto s = ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Float); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { @@ -924,7 +924,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_SampledTypeSint_Format_r32sint) { auto s = ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Sint); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { @@ -945,7 +945,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_SampledTypeUint_Format_r32uint) { auto s = ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Uint); - auto ac = ty.access(ast::AccessControl::kReadOnly, s); + auto* ac = ty.access(ast::AccessControl::kReadOnly, s); Global("test_var", ac, ast::StorageClass::kNone, nullptr, { diff --git a/src/writer/wgsl/generator_impl_function_test.cc b/src/writer/wgsl/generator_impl_function_test.cc index f9e79d2865..16b1c0dd5b 100644 --- a/src/writer/wgsl/generator_impl_function_test.cc +++ b/src/writer/wgsl/generator_impl_function_test.cc @@ -16,7 +16,6 @@ #include "src/ast/struct_block_decoration.h" #include "src/ast/variable_decl_statement.h" #include "src/ast/workgroup_decoration.h" -#include "src/sem/access_control_type.h" #include "src/writer/wgsl/test_helper.h" namespace tint { @@ -204,7 +203,7 @@ TEST_F(WgslGeneratorImplTest, auto* s = Structure("Data", {Member("d", ty.f32())}, {create()}); - auto ac = ty.access(ast::AccessControl::kReadWrite, s); + auto* ac = ty.access(ast::AccessControl::kReadWrite, s); Global("data", ac, ast::StorageClass::kStorage, nullptr, ast::DecorationList{ diff --git a/src/writer/wgsl/generator_impl_global_decl_test.cc b/src/writer/wgsl/generator_impl_global_decl_test.cc index f8eea59b59..db555b198c 100644 --- a/src/writer/wgsl/generator_impl_global_decl_test.cc +++ b/src/writer/wgsl/generator_impl_global_decl_test.cc @@ -14,7 +14,6 @@ #include "src/ast/stage_decoration.h" #include "src/ast/variable_decl_statement.h" -#include "src/sem/access_control_type.h" #include "src/sem/sampled_texture_type.h" #include "src/writer/wgsl/test_helper.h" diff --git a/src/writer/wgsl/generator_impl_type_test.cc b/src/writer/wgsl/generator_impl_type_test.cc index d4d76911f4..f0005c1706 100644 --- a/src/writer/wgsl/generator_impl_type_test.cc +++ b/src/writer/wgsl/generator_impl_type_test.cc @@ -13,7 +13,6 @@ // limitations under the License. #include "src/ast/struct_block_decoration.h" -#include "src/sem/access_control_type.h" #include "src/sem/depth_texture_type.h" #include "src/sem/multisampled_texture_type.h" #include "src/sem/sampled_texture_type.h" @@ -50,7 +49,7 @@ TEST_F(WgslGeneratorImplTest, EmitType_AccessControl_Read) { auto* s = Structure("S", {Member("a", ty.i32())}, {create()}); - auto a = ty.access(ast::AccessControl::kReadOnly, s); + auto* a = ty.access(ast::AccessControl::kReadOnly, s); AST().AddConstructedType(ty.alias("make_type_reachable", a)); GeneratorImpl& gen = Build(); @@ -63,7 +62,7 @@ TEST_F(WgslGeneratorImplTest, EmitType_AccessControl_ReadWrite) { auto* s = Structure("S", {Member("a", ty.i32())}, {create()}); - auto a = ty.access(ast::AccessControl::kReadWrite, s); + auto* a = ty.access(ast::AccessControl::kReadWrite, s); AST().AddConstructedType(ty.alias("make_type_reachable", a)); GeneratorImpl& gen = Build(); @@ -430,7 +429,7 @@ TEST_P(WgslGenerator_StorageTextureTest, EmitType_StorageTexture) { auto param = GetParam(); auto t = ty.storage_texture(param.dim, param.fmt); - auto ac = ty.access(param.access, t); + auto* ac = ty.access(param.access, t); GeneratorImpl& gen = Build(); diff --git a/test/BUILD.gn b/test/BUILD.gn index f5c3440f34..88b158ad29 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -269,7 +269,6 @@ tint_unittests_source_set("tint_unittests_core_src") { "../src/resolver/type_validation_test.cc", "../src/resolver/validation_test.cc", "../src/scope_stack_test.cc", - "../src/sem/access_control_type_test.cc", "../src/sem/bool_type_test.cc", "../src/sem/depth_texture_type_test.cc", "../src/sem/external_texture_type_test.cc", @@ -349,8 +348,8 @@ tint_unittests_source_set("tint_unittests_spv_reader_src") { "../src/reader/spirv/parser_impl_test_helper.cc", "../src/reader/spirv/parser_impl_test_helper.h", "../src/reader/spirv/parser_impl_user_name_test.cc", - "../src/reader/spirv/parser_type_test.cc", "../src/reader/spirv/parser_test.cc", + "../src/reader/spirv/parser_type_test.cc", "../src/reader/spirv/spirv_tools_helpers_test.cc", "../src/reader/spirv/spirv_tools_helpers_test.h", "../src/reader/spirv/usage_test.cc",