Remove access control from StorageTexture. This CL removes the access control value from the storage textures and, instead, wraps in an type::AccessControl. This matches the current spec where the access is an annotation on the type as opposed to part of the type. Bug: tint:286 Change-Id: Ia944ed8557fbf490d78db2a1b49c31d0aba08728 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37740 Auto-Submit: dan sinclair <dsinclair@chromium.org> Reviewed-by: Ben Clayton <bclayton@google.com> Commit-Queue: dan sinclair <dsinclair@chromium.org>
diff --git a/src/ast/intrinsic_texture_helper_test.cc b/src/ast/intrinsic_texture_helper_test.cc index 84a5f1f5..ba25bdc 100644 --- a/src/ast/intrinsic_texture_helper_test.cc +++ b/src/ast/intrinsic_texture_helper_test.cc
@@ -15,6 +15,7 @@ #include "src/ast/intrinsic_texture_helper_test.h" #include "src/ast/builder.h" +#include "src/ast/type/access_control_type.h" #include "src/ast/type/depth_texture_type.h" #include "src/ast/type/multisampled_texture_type.h" #include "src/ast/type/sampled_texture_type.h" @@ -176,10 +177,12 @@ nullptr, decos); case ast::intrinsic::test::TextureKind::kStorage: { - auto* st = b->create<ast::type::StorageTexture>( - texture_dimension, access_control, image_format); + auto* st = + b->create<ast::type::StorageTexture>(texture_dimension, image_format); st->set_type(datatype); - return b->Var("texture", ast::StorageClass::kUniformConstant, st, nullptr, + + auto* ac = b->create<ast::type::AccessControl>(access_control, st); + return b->Var("texture", ast::StorageClass::kUniformConstant, ac, nullptr, decos); } }
diff --git a/src/ast/intrinsic_texture_helper_test.h b/src/ast/intrinsic_texture_helper_test.h index 07a1eee..fae5a2a 100644 --- a/src/ast/intrinsic_texture_helper_test.h +++ b/src/ast/intrinsic_texture_helper_test.h
@@ -18,6 +18,7 @@ #include <functional> #include <vector> +#include "src/ast/access_control.h" #include "src/ast/builder.h" #include "src/ast/type/sampler_type.h" #include "src/ast/type/storage_texture_type.h"
diff --git a/src/ast/type/storage_texture_type.cc b/src/ast/type/storage_texture_type.cc index da44ac6..3dc432c 100644 --- a/src/ast/type/storage_texture_type.cc +++ b/src/ast/type/storage_texture_type.cc
@@ -142,10 +142,8 @@ return out; } -StorageTexture::StorageTexture(TextureDimension dim, - ast::AccessControl access, - ImageFormat format) - : Base(dim), access_(access), image_format_(format) {} +StorageTexture::StorageTexture(TextureDimension dim, ImageFormat format) + : Base(dim), image_format_(format) {} void StorageTexture::set_type(Type* const type) { type_ = type; @@ -161,13 +159,12 @@ std::string StorageTexture::type_name() const { std::ostringstream out; - out << "__storage_texture_" << access_ << "_" << dim() << "_" - << image_format_; + out << "__storage_texture_" << dim() << "_" << image_format_; return out.str(); } StorageTexture* StorageTexture::Clone(CloneContext* ctx) const { - return ctx->mod->create<StorageTexture>(dim(), access_, image_format_); + return ctx->mod->create<StorageTexture>(dim(), image_format_); } } // namespace type
diff --git a/src/ast/type/storage_texture_type.h b/src/ast/type/storage_texture_type.h index 33e1815..2723a0f 100644 --- a/src/ast/type/storage_texture_type.h +++ b/src/ast/type/storage_texture_type.h
@@ -17,7 +17,6 @@ #include <string> -#include "src/ast/access_control.h" #include "src/ast/type/texture_type.h" namespace tint { @@ -70,10 +69,8 @@ public: /// Constructor /// @param dim the dimensionality of the texture - /// @param access the access type for the texture /// @param format the image format of the texture StorageTexture(TextureDimension dim, - ast::AccessControl access, ImageFormat format); /// Move constructor @@ -86,9 +83,6 @@ /// @returns the subtype of the storage texture set with set_type Type* type() const; - /// @returns the storage access - ast::AccessControl access() const { return access_; } - /// @returns the image format ImageFormat image_format() const { return image_format_; } @@ -101,7 +95,6 @@ StorageTexture* Clone(CloneContext* ctx) const override; private: - ast::AccessControl const access_; ImageFormat const image_format_; Type* type_ = nullptr; // Semantic info
diff --git a/src/ast/type/storage_texture_type_test.cc b/src/ast/type/storage_texture_type_test.cc index c47078b..d05d53d 100644 --- a/src/ast/type/storage_texture_type_test.cc +++ b/src/ast/type/storage_texture_type_test.cc
@@ -40,8 +40,7 @@ using StorageTextureTest = TestHelper; TEST_F(StorageTextureTest, Is) { - StorageTexture s(TextureDimension::k2dArray, ast::AccessControl::kReadOnly, - ImageFormat::kRgba32Float); + StorageTexture s(TextureDimension::k2dArray, ImageFormat::kRgba32Float); Type* ty = &s; EXPECT_FALSE(ty->Is<AccessControl>()); EXPECT_FALSE(ty->Is<Alias>()); @@ -59,8 +58,7 @@ } TEST_F(StorageTextureTest, IsTexture) { - StorageTexture s(TextureDimension::k2dArray, ast::AccessControl::kReadOnly, - ImageFormat::kRgba32Float); + StorageTexture s(TextureDimension::k2dArray, ImageFormat::kRgba32Float); Texture* ty = &s; EXPECT_FALSE(ty->Is<DepthTexture>()); EXPECT_FALSE(ty->Is<SampledTexture>()); @@ -68,32 +66,22 @@ } TEST_F(StorageTextureTest, Dim) { - StorageTexture s(TextureDimension::k2dArray, ast::AccessControl::kReadOnly, - ImageFormat::kRgba32Float); + StorageTexture s(TextureDimension::k2dArray, ImageFormat::kRgba32Float); EXPECT_EQ(s.dim(), TextureDimension::k2dArray); } -TEST_F(StorageTextureTest, Access) { - StorageTexture s(TextureDimension::k2dArray, ast::AccessControl::kReadOnly, - ImageFormat::kRgba32Float); - EXPECT_EQ(s.access(), ast::AccessControl::kReadOnly); -} - TEST_F(StorageTextureTest, Format) { - StorageTexture s(TextureDimension::k2dArray, ast::AccessControl::kReadOnly, - ImageFormat::kRgba32Float); + StorageTexture s(TextureDimension::k2dArray, ImageFormat::kRgba32Float); EXPECT_EQ(s.image_format(), ImageFormat::kRgba32Float); } TEST_F(StorageTextureTest, TypeName) { - StorageTexture s(TextureDimension::k2dArray, ast::AccessControl::kReadOnly, - ImageFormat::kRgba32Float); - EXPECT_EQ(s.type_name(), "__storage_texture_read_only_2d_array_rgba32float"); + StorageTexture s(TextureDimension::k2dArray, ImageFormat::kRgba32Float); + EXPECT_EQ(s.type_name(), "__storage_texture_2d_array_rgba32float"); } TEST_F(StorageTextureTest, F32) { Type* s = mod->create<StorageTexture>(TextureDimension::k2dArray, - ast::AccessControl::kReadOnly, ImageFormat::kRgba32Float); TypeDeterminer td(mod); @@ -105,7 +93,6 @@ TEST_F(StorageTextureTest, U32) { Type* s = mod->create<StorageTexture>(TextureDimension::k2dArray, - ast::AccessControl::kReadOnly, ImageFormat::kRg32Uint); TypeDeterminer td(mod); @@ -117,7 +104,6 @@ TEST_F(StorageTextureTest, I32) { Type* s = mod->create<StorageTexture>(TextureDimension::k2dArray, - ast::AccessControl::kReadOnly, ImageFormat::kRgba32Sint); TypeDeterminer td(mod); @@ -128,8 +114,7 @@ } TEST_F(StorageTextureTest, MinBufferBindingSize) { - StorageTexture s(TextureDimension::k2dArray, ast::AccessControl::kReadOnly, - ImageFormat::kRgba32Sint); + StorageTexture s(TextureDimension::k2dArray, ImageFormat::kRgba32Sint); EXPECT_EQ(0u, s.MinBufferBindingSize(MemoryLayout::kUniformBuffer)); }
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 93e7cd8..696b305 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc
@@ -4050,7 +4050,7 @@ return Fail(); } ast::type::Texture* texture_type = - texture_ptr_type->type()->As<ast::type::Texture>(); + texture_ptr_type->type()->UnwrapAll()->As<ast::type::Texture>(); if (!texture_type) { return Fail(); } @@ -4325,12 +4325,14 @@ Fail(); return {}; } - if (!type || !type->type()->Is<ast::type::Texture>()) { + if (!type || !type->type()->UnwrapAll()->Is<ast::type::Texture>()) { Fail() << "invalid texture type for " << image->PrettyPrint(); return {}; } + + auto* unwrapped_type = type->type()->UnwrapAll(); ast::type::TextureDimension dim = - type->type()->As<ast::type::Texture>()->dim(); + unwrapped_type->As<ast::type::Texture>()->dim(); // Number of regular coordinates. uint32_t num_axes = 0; bool is_arrayed = false;
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index bcb7fd2..54af127 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc
@@ -1898,8 +1898,8 @@ if (format == ast::type::ImageFormat::kNone) { return nullptr; } - ast_store_type = - ast_module_.create<ast::type::StorageTexture>(dim, access, format); + ast_store_type = ast_module_.create<ast::type::AccessControl>( + access, ast_module_.create<ast::type::StorageTexture>(dim, format)); } } else { Fail() << "unsupported: UniformConstant variable is not a recognized "
diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc index f0294b6..281380c 100644 --- a/src/reader/spirv/parser_impl_handle_test.cc +++ b/src/reader/spirv/parser_impl_handle_test.cc
@@ -1184,7 +1184,7 @@ } x_10 uniform_constant - __storage_texture_read_only_1d_rg32float + __access_control_read_only__storage_texture_1d_rg32float })"}, DeclUnderspecifiedHandleCase{R"( OpDecorate %10 NonReadable @@ -1200,7 +1200,7 @@ } x_10 uniform_constant - __storage_texture_write_only_1d_rg32float + __access_control_write_only__storage_texture_1d_rg32float })"})); // Test emission of variables when we have image accesses in executable code. @@ -2564,13 +2564,13 @@ << "TEXTURE BUILTIN IS BAD " << module << assembly; } -INSTANTIATE_TEST_SUITE_P( - ImageWrite_OptionalParams, - SpvParserTest_ImageAccessTest, - ::testing::ValuesIn(std::vector<ImageAccessCase>{ - // OpImageWrite with no extra params - {"%float 2D 0 0 0 2 Rgba32f", "OpImageWrite %im %vi12 %vf1234", - R"( +INSTANTIATE_TEST_SUITE_P(ImageWrite_OptionalParams, + SpvParserTest_ImageAccessTest, + ::testing::ValuesIn(std::vector<ImageAccessCase>{ + // OpImageWrite with no extra params + {"%float 2D 0 0 0 2 Rgba32f", + "OpImageWrite %im %vi12 %vf1234", + R"( Variable{ Decorations{ SetDecoration{2} @@ -2578,9 +2578,9 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rgba32float + __access_control_write_only__storage_texture_2d_rgba32float })", - R"(Call[not set]{ + R"(Call[not set]{ Identifier[not set]{textureStore} ( Identifier[not set]{x_20} @@ -2606,7 +2606,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_r32float + __access_control_write_only__storage_texture_2d_r32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2626,7 +2626,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_r32float + __access_control_write_only__storage_texture_2d_r32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2649,7 +2649,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_r32float + __access_control_write_only__storage_texture_2d_r32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2672,7 +2672,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_r32float + __access_control_write_only__storage_texture_2d_r32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2695,7 +2695,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rg32float + __access_control_write_only__storage_texture_2d_rg32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2715,7 +2715,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rg32float + __access_control_write_only__storage_texture_2d_rg32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2738,7 +2738,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rg32float + __access_control_write_only__storage_texture_2d_rg32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2762,7 +2762,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rgba32float + __access_control_write_only__storage_texture_2d_rgba32float })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2928,7 +2928,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rgba32uint + __access_control_write_only__storage_texture_2d_rgba32uint })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2948,7 +2948,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rgba32uint + __access_control_write_only__storage_texture_2d_rgba32uint })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2970,7 +2970,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rgba32sint + __access_control_write_only__storage_texture_2d_rgba32sint })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -2992,7 +2992,7 @@ } x_20 uniform_constant - __storage_texture_write_only_2d_rgba32sint + __access_control_write_only__storage_texture_2d_rgba32sint })", R"(Call[not set]{ Identifier[not set]{textureStore} @@ -3003,22 +3003,22 @@ ) })"}})); -INSTANTIATE_TEST_SUITE_P( - ImageRead_OptionalParams, - SpvParserTest_ImageAccessTest, - ::testing::ValuesIn(std::vector<ImageAccessCase>{ - // OpImageRead with no extra params - {"%float 2D 0 0 0 2 Rgba32f", "%99 = OpImageRead %v4float %im %vi12", - R"(Variable{ +INSTANTIATE_TEST_SUITE_P(ImageRead_OptionalParams, + SpvParserTest_ImageAccessTest, + ::testing::ValuesIn(std::vector<ImageAccessCase>{ + // OpImageRead with no extra params + {"%float 2D 0 0 0 2 Rgba32f", + "%99 = OpImageRead %v4float %im %vi12", + R"(Variable{ Decorations{ SetDecoration{2} BindingDecoration{1} } x_20 uniform_constant - __storage_texture_read_only_2d_rgba32float + __access_control_read_only__storage_texture_2d_rgba32float })", - R"(VariableDeclStatement{ + R"(VariableDeclStatement{ VariableConst{ x_99 none @@ -3418,7 +3418,7 @@ } x_20 uniform_constant - __storage_texture_read_only_2d_rgba32float + __access_control_read_only__storage_texture_2d_rgba32float })", R"(VariableDeclStatement{ VariableConst{ @@ -3445,7 +3445,7 @@ } x_20 uniform_constant - __storage_texture_read_only_2d_rgba32uint + __access_control_read_only__storage_texture_2d_rgba32uint })", R"(VariableDeclStatement{ VariableConst{ @@ -3472,7 +3472,7 @@ } x_20 uniform_constant - __storage_texture_read_only_2d_rgba32uint + __access_control_read_only__storage_texture_2d_rgba32uint })", R"(VariableDeclStatement{ VariableConst{ @@ -3501,7 +3501,7 @@ } x_20 uniform_constant - __storage_texture_read_only_2d_rgba32sint + __access_control_read_only__storage_texture_2d_rgba32sint })", R"(VariableDeclStatement{ VariableConst{ @@ -3528,7 +3528,7 @@ } x_20 uniform_constant - __storage_texture_read_only_2d_rgba32sint + __access_control_read_only__storage_texture_2d_rgba32sint })", R"(VariableDeclStatement{ VariableConst{
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index d178cb9..831479a 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc
@@ -536,8 +536,9 @@ if (format.errored) return Failure::kErrored; - return module_.create<ast::type::StorageTexture>( - storage->first, storage->second, format.value); + return module_.create<ast::type::AccessControl>( + storage->second, module_.create<ast::type::StorageTexture>( + storage->first, format.value)); } return Failure::kNoMatch;
diff --git a/src/reader/wgsl/parser_impl_texture_sampler_types_test.cc b/src/reader/wgsl/parser_impl_texture_sampler_types_test.cc index dc74f29..967a57c 100644 --- a/src/reader/wgsl/parser_impl_texture_sampler_types_test.cc +++ b/src/reader/wgsl/parser_impl_texture_sampler_types_test.cc
@@ -13,6 +13,7 @@ // limitations under the License. #include "gtest/gtest.h" +#include "src/ast/type/access_control_type.h" #include "src/ast/type/depth_texture_type.h" #include "src/ast/type/f32_type.h" #include "src/ast/type/i32_type.h" @@ -295,17 +296,21 @@ TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Readonly1dR8Unorm_Old) { auto p = parser("texture_ro_1d<r8unorm>"); - auto t = p->texture_sampler_types(); + auto ac = p->texture_sampler_types(); ASSERT_FALSE(p->has_error()) << p->error(); - EXPECT_TRUE(t.matched); - EXPECT_FALSE(t.errored); - ASSERT_NE(t.value, nullptr); + EXPECT_TRUE(ac.matched); + EXPECT_FALSE(ac.errored); + ASSERT_NE(ac.value, nullptr); + + ASSERT_TRUE(ac->Is<ast::type::AccessControl>()); + EXPECT_EQ(ac->As<ast::type::AccessControl>()->access_control(), + ast::AccessControl::kReadOnly); + + auto* t = ac->As<ast::type::AccessControl>()->type(); ASSERT_TRUE(t->Is<ast::type::Texture>()); ASSERT_TRUE(t->Is<ast::type::StorageTexture>()); EXPECT_EQ(t->As<ast::type::StorageTexture>()->image_format(), ast::type::ImageFormat::kR8Unorm); - EXPECT_EQ(t->As<ast::type::StorageTexture>()->access(), - ast::AccessControl::kReadOnly); EXPECT_EQ(t->As<ast::type::Texture>()->dim(), ast::type::TextureDimension::k1d); } @@ -313,17 +318,21 @@ TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Writeonly2dR16Float_Old) { auto p = parser("texture_wo_2d<r16float>"); - auto t = p->texture_sampler_types(); + auto ac = p->texture_sampler_types(); ASSERT_FALSE(p->has_error()) << p->error(); - EXPECT_TRUE(t.matched); - EXPECT_FALSE(t.errored); - ASSERT_NE(t.value, nullptr); + EXPECT_TRUE(ac.matched); + EXPECT_FALSE(ac.errored); + ASSERT_NE(ac.value, nullptr); + + ASSERT_TRUE(ac->Is<ast::type::AccessControl>()); + EXPECT_EQ(ac->As<ast::type::AccessControl>()->access_control(), + ast::AccessControl::kWriteOnly); + + auto* t = ac->As<ast::type::AccessControl>()->type(); ASSERT_TRUE(t->Is<ast::type::Texture>()); ASSERT_TRUE(t->Is<ast::type::StorageTexture>()); EXPECT_EQ(t->As<ast::type::StorageTexture>()->image_format(), ast::type::ImageFormat::kR16Float); - EXPECT_EQ(t->As<ast::type::StorageTexture>()->access(), - ast::AccessControl::kWriteOnly); EXPECT_EQ(t->As<ast::type::Texture>()->dim(), ast::type::TextureDimension::k2d); } @@ -367,34 +376,42 @@ TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Readonly1dR8Unorm) { auto p = parser("texture_storage_ro_1d<r8unorm>"); - auto t = p->texture_sampler_types(); + auto ac = p->texture_sampler_types(); ASSERT_FALSE(p->has_error()) << p->error(); - EXPECT_TRUE(t.matched); - EXPECT_FALSE(t.errored); - ASSERT_NE(t.value, nullptr); + EXPECT_TRUE(ac.matched); + EXPECT_FALSE(ac.errored); + ASSERT_NE(ac.value, nullptr); + + ASSERT_TRUE(ac->Is<ast::type::AccessControl>()); + EXPECT_EQ(ac->As<ast::type::AccessControl>()->access_control(), + ast::AccessControl::kReadOnly); + + auto* t = ac->As<ast::type::AccessControl>()->type(); ASSERT_TRUE(t->Is<ast::type::Texture>()); ASSERT_TRUE(t->Is<ast::type::StorageTexture>()); EXPECT_EQ(t->As<ast::type::StorageTexture>()->image_format(), ast::type::ImageFormat::kR8Unorm); - EXPECT_EQ(t->As<ast::type::StorageTexture>()->access(), - ast::AccessControl::kReadOnly); EXPECT_EQ(t->As<ast::type::Texture>()->dim(), ast::type::TextureDimension::k1d); } TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Writeonly2dR16Float) { auto p = parser("texture_storage_wo_2d<r16float>"); - auto t = p->texture_sampler_types(); + auto ac = p->texture_sampler_types(); ASSERT_FALSE(p->has_error()) << p->error(); - EXPECT_TRUE(t.matched); - EXPECT_FALSE(t.errored); - ASSERT_NE(t.value, nullptr); + EXPECT_TRUE(ac.matched); + EXPECT_FALSE(ac.errored); + ASSERT_NE(ac.value, nullptr); + + ASSERT_TRUE(ac->Is<ast::type::AccessControl>()); + EXPECT_EQ(ac->As<ast::type::AccessControl>()->access_control(), + ast::AccessControl::kWriteOnly); + + auto* t = ac->As<ast::type::AccessControl>()->type(); ASSERT_TRUE(t->Is<ast::type::Texture>()); ASSERT_TRUE(t->Is<ast::type::StorageTexture>()); EXPECT_EQ(t->As<ast::type::StorageTexture>()->image_format(), ast::type::ImageFormat::kR16Float); - EXPECT_EQ(t->As<ast::type::StorageTexture>()->access(), - ast::AccessControl::kWriteOnly); EXPECT_EQ(t->As<ast::type::Texture>()->dim(), ast::type::TextureDimension::k2d); }
diff --git a/src/type_determiner.cc b/src/type_determiner.cc index b8aab37..8f3f64b 100644 --- a/src/type_determiner.cc +++ b/src/type_determiner.cc
@@ -91,7 +91,8 @@ bool TypeDeterminer::Determine() { std::vector<ast::type::StorageTexture*> storage_textures; for (auto& it : mod_->types()) { - if (auto* storage = it.second->As<ast::type::StorageTexture>()) { + if (auto* storage = + it.second->UnwrapIfNeeded()->As<ast::type::StorageTexture>()) { storage_textures.emplace_back(storage); } } @@ -548,16 +549,13 @@ ast::intrinsic::TextureSignature::Parameters param; auto* texture_param = expr->params()[0]; - if (!texture_param->result_type() - ->UnwrapPtrIfNeeded() - ->Is<ast::type::Texture>()) { + if (!texture_param->result_type()->UnwrapAll()->Is<ast::type::Texture>()) { set_error(expr->source(), "invalid first argument for " + mod_->SymbolToName(ident->symbol())); return false; } - ast::type::Texture* texture = texture_param->result_type() - ->UnwrapPtrIfNeeded() - ->As<ast::type::Texture>(); + ast::type::Texture* texture = + texture_param->result_type()->UnwrapAll()->As<ast::type::Texture>(); bool is_array = ast::type::IsTextureArray(texture->dim()); bool is_multisampled = texture->Is<ast::type::MultisampledTexture>();
diff --git a/src/type_determiner_test.cc b/src/type_determiner_test.cc index df295ae..300cc99 100644 --- a/src/type_determiner_test.cc +++ b/src/type_determiner_test.cc
@@ -1433,8 +1433,8 @@ auto coords_type = get_coords_type(dim, ty.i32); - ast::type::Type* texture_type = mod->create<ast::type::StorageTexture>( - dim, ast::AccessControl::kReadOnly, format); + ast::type::Type* texture_type = + mod->create<ast::type::StorageTexture>(dim, format); ast::ExpressionList call_params;
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index fe191ab..e1a5737 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc
@@ -693,7 +693,7 @@ auto* texture = params[pidx.texture]; auto* texture_type = - texture->result_type()->UnwrapPtrIfNeeded()->As<ast::type::Texture>(); + texture->result_type()->UnwrapAll()->As<ast::type::Texture>(); if (ident->intrinsic() == ast::Intrinsic::kTextureDimensions) { // Declare a variable to hold the texture dimensions @@ -2101,6 +2101,12 @@ bool GeneratorImpl::EmitType(std::ostream& out, ast::type::Type* type, const std::string& name) { + // HLSL doesn't have the read/write only markings so just unwrap the access + // control type. + if (auto* ac = type->As<ast::type::AccessControl>()) { + return EmitType(out, ac->type(), name); + } + if (auto* alias = type->As<ast::type::Alias>()) { out << namer_.NameFor(module_->SymbolToName(alias->symbol())); } else if (auto* ary = type->As<ast::type::Array>()) {
diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc index 644e75f..9cf3c4f 100644 --- a/src/writer/hlsl/generator_impl_type_test.cc +++ b/src/writer/hlsl/generator_impl_type_test.cc
@@ -19,6 +19,7 @@ #include "src/ast/struct_member.h" #include "src/ast/struct_member_decoration.h" #include "src/ast/struct_member_offset_decoration.h" +#include "src/ast/type/access_control_type.h" #include "src/ast/type/array_type.h" #include "src/ast/type/bool_type.h" #include "src/ast/type/depth_texture_type.h" @@ -320,11 +321,12 @@ auto params = GetParam(); ast::type::StorageTexture s(params.dim, - params.ro ? ast::AccessControl::kReadOnly - : ast::AccessControl::kWriteOnly, params.imgfmt); + ast::type::AccessControl ac(params.ro ? ast::AccessControl::kReadOnly + : ast::AccessControl::kWriteOnly, + &s); - ASSERT_TRUE(gen.EmitType(out, &s, "")) << gen.error(); + ASSERT_TRUE(gen.EmitType(out, &ac, "")) << gen.error(); EXPECT_EQ(result(), params.result); } INSTANTIATE_TEST_SUITE_P(
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index fa072e9..7efe837 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc
@@ -1835,6 +1835,20 @@ } bool GeneratorImpl::EmitType(ast::type::Type* type, const std::string& name) { + std::string access_str = ""; + if (auto* ac = type->As<ast::type::AccessControl>()) { + if (ac->access_control() == ast::AccessControl::kReadOnly) { + access_str = "read"; + } else if (ac->access_control() == ast::AccessControl::kWriteOnly) { + access_str = "write"; + } else { + error_ = "Invalid access control for storage texture"; + return false; + } + + type = ac->type(); + } + if (auto* alias = type->As<ast::type::Alias>()) { out_ << namer_.NameFor(module_->SymbolToName(alias->symbol())); } else if (auto* ary = type->As<ast::type::Array>()) { @@ -1923,15 +1937,7 @@ if (!EmitType(storage->type(), "")) { return false; } - out_ << ", access::"; - if (storage->access() == ast::AccessControl::kReadOnly) { - out_ << "read"; - } else if (storage->access() == ast::AccessControl::kWriteOnly) { - out_ << "write"; - } else { - error_ = "Invalid access control for storage texture"; - return false; - } + out_ << ", access::" << access_str; } else if (auto* ms = tex->As<ast::type::MultisampledTexture>()) { if (!EmitType(ms->type(), "")) { return false;
diff --git a/src/writer/msl/generator_impl_type_test.cc b/src/writer/msl/generator_impl_type_test.cc index 8f50096..90c3c88 100644 --- a/src/writer/msl/generator_impl_type_test.cc +++ b/src/writer/msl/generator_impl_type_test.cc
@@ -20,6 +20,7 @@ #include "src/ast/struct_member.h" #include "src/ast/struct_member_decoration.h" #include "src/ast/struct_member_offset_decoration.h" +#include "src/ast/type/access_control_type.h" #include "src/ast/type/array_type.h" #include "src/ast/type/bool_type.h" #include "src/ast/type/depth_texture_type.h" @@ -332,12 +333,13 @@ auto params = GetParam(); ast::type::StorageTexture s(params.dim, - params.ro ? ast::AccessControl::kReadOnly - : ast::AccessControl::kWriteOnly, ast::type::ImageFormat::kR16Float); + ast::type::AccessControl ac(params.ro ? ast::AccessControl::kReadOnly + : ast::AccessControl::kWriteOnly, + &s); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); - ASSERT_TRUE(gen.EmitType(&s, "")) << gen.error(); + ASSERT_TRUE(gen.EmitType(&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 598a0db..0bc1565 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc
@@ -735,16 +735,17 @@ {Operand::Int(var_id), Operand::String(mod_->SymbolToName(var->symbol()))}); - auto* type = var->type()->UnwrapAll(); - 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 = var->type()->UnwrapAll(); if (var->has_constructor()) { ops.push_back(Operand::Int(init_id)); - } else if (auto* tex = type->As<ast::type::Texture>()) { - // Decorate storage texture variables with NonRead/Writeable if needed. - if (auto* storage = tex->As<ast::type::StorageTexture>()) { - switch (storage->access()) { + } else if (type->Is<ast::type::Texture>()) { + if (auto* ac = var->type()->As<ast::type::AccessControl>()) { + switch (ac->access_control()) { case ast::AccessControl::kWriteOnly: push_annot( spv::Op::OpDecorate, @@ -2727,6 +2728,11 @@ if (auto* alias = type->As<ast::type::Alias>()) { return GenerateTypeIfNeeded(alias->type()); } + if (auto* ac = type->As<ast::type::AccessControl>()) { + if (!ac->type()->UnwrapIfNeeded()->Is<ast::type::Struct>()) { + return GenerateTypeIfNeeded(ac->type()); + } + } auto val = type_name_to_id_.find(type->type_name()); if (val != type_name_to_id_.end()) { @@ -2735,13 +2741,9 @@ auto result = result_op(); auto id = result.to_i(); - if (auto* ac = type->As<ast::type::AccessControl>()) { + // The non-struct case was handled above. auto* subtype = ac->type()->UnwrapIfNeeded(); - if (!subtype->Is<ast::type::Struct>()) { - error_ = "Access control attached to non-struct type."; - return 0; - } if (!GenerateStructType(subtype->As<ast::type::Struct>(), ac->access_control(), result)) { return 0;
diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc index 6d67e53..c4830f6 100644 --- a/src/writer/spirv/builder_global_variable_test.cc +++ b/src/writer/spirv/builder_global_variable_test.cc
@@ -501,11 +501,12 @@ // var<uniform_constant> a : texture_storage_ro_2d<r32uint>; ast::type::StorageTexture type(ast::type::TextureDimension::k2d, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR32Uint); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&type)) << td.error(); - auto* var_a = Var("a", ast::StorageClass::kUniformConstant, &type); + ast::type::AccessControl ac(ast::AccessControl::kReadOnly, &type); + + auto* var_a = Var("a", ast::StorageClass::kUniformConstant, &ac); EXPECT_TRUE(b.GenerateGlobalVariable(var_a)) << b.error(); EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 NonWritable @@ -521,11 +522,12 @@ // var<uniform_constant> a : texture_storage_wo_2d<r32uint>; ast::type::StorageTexture type(ast::type::TextureDimension::k2d, - ast::AccessControl::kWriteOnly, ast::type::ImageFormat::kR32Uint); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&type)) << td.error(); - auto* var_a = Var("a", ast::StorageClass::kUniformConstant, &type); + ast::type::AccessControl ac(ast::AccessControl::kWriteOnly, &type); + + auto* var_a = Var("a", ast::StorageClass::kUniformConstant, &ac); EXPECT_TRUE(b.GenerateGlobalVariable(var_a)) << b.error(); EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 NonReadable
diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc index db2f45a..9e29ab2 100644 --- a/src/writer/spirv/builder_type_test.cc +++ b/src/writer/spirv/builder_type_test.cc
@@ -736,9 +736,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_1d_R16Float) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_1d_R16Float) { ast::type::StorageTexture s(ast::type::TextureDimension::k1d, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR16Float); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -754,9 +753,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_1d_R8SNorm) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_1d_R8SNorm) { ast::type::StorageTexture s(ast::type::TextureDimension::k1d, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR8Snorm); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -772,9 +770,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_1d_R8UNorm) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_1d_R8UNorm) { ast::type::StorageTexture s(ast::type::TextureDimension::k1d, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR8Unorm); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -790,9 +787,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_1d_R8Uint) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_1d_R8Uint) { ast::type::StorageTexture s(ast::type::TextureDimension::k1d, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR8Uint); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -803,9 +799,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_1d_R8Sint) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_1d_R8Sint) { ast::type::StorageTexture s(ast::type::TextureDimension::k1d, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR8Sint); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -816,9 +811,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_1d_array) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_1d_array) { ast::type::StorageTexture s(ast::type::TextureDimension::k1dArray, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR16Float); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -834,9 +828,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_2d) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_2d) { ast::type::StorageTexture s(ast::type::TextureDimension::k2d, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR16Float); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -847,9 +840,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_2dArray) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_2dArray) { ast::type::StorageTexture s(ast::type::TextureDimension::k2dArray, - ast::AccessControl::kReadOnly, ast::type::ImageFormat::kR16Float); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -860,84 +852,8 @@ )"); } -TEST_F(BuilderTest_Type, StorageTexture_GenerateReadonly_3d) { +TEST_F(BuilderTest_Type, StorageTexture_Generate_3d) { ast::type::StorageTexture s(ast::type::TextureDimension::k3d, - ast::AccessControl::kReadOnly, - ast::type::ImageFormat::kR16Float); - - ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); - EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); - ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 -%1 = OpTypeImage %2 3D 0 0 0 2 R16f -)"); -} - -TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_1d) { - ast::type::StorageTexture s(ast::type::TextureDimension::k1d, - ast::AccessControl::kWriteOnly, - ast::type::ImageFormat::kR16Float); - - ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); - EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); - ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 -%1 = OpTypeImage %2 1D 0 0 0 2 R16f -)"); - - EXPECT_EQ(DumpInstructions(b.capabilities()), - R"(OpCapability Image1D -OpCapability StorageImageExtendedFormats -)"); -} - -TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_1dArray) { - ast::type::StorageTexture s(ast::type::TextureDimension::k1dArray, - ast::AccessControl::kWriteOnly, - ast::type::ImageFormat::kR16Float); - - ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); - EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); - ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 -%1 = OpTypeImage %2 1D 0 1 0 2 R16f -)"); - - EXPECT_EQ(DumpInstructions(b.capabilities()), - R"(OpCapability Image1D -OpCapability StorageImageExtendedFormats -)"); -} - -TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_2d) { - ast::type::StorageTexture s(ast::type::TextureDimension::k2d, - ast::AccessControl::kWriteOnly, - ast::type::ImageFormat::kR16Float); - - ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); - EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); - ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 -%1 = OpTypeImage %2 2D 0 0 0 2 R16f -)"); -} - -TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_2dArray) { - ast::type::StorageTexture s(ast::type::TextureDimension::k2dArray, - ast::AccessControl::kWriteOnly, - ast::type::ImageFormat::kR16Float); - - ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); - EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); - ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 -%1 = OpTypeImage %2 2D 0 1 0 2 R16f -)"); -} - -TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_3d) { - ast::type::StorageTexture s(ast::type::TextureDimension::k3d, - ast::AccessControl::kWriteOnly, ast::type::ImageFormat::kR16Float); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -949,9 +865,8 @@ } TEST_F(BuilderTest_Type, - StorageTexture_GenerateWriteonly_SampledTypeFloat_Format_r32float) { + StorageTexture_Generate_SampledTypeFloat_Format_r32float) { ast::type::StorageTexture s(ast::type::TextureDimension::k2d, - ast::AccessControl::kWriteOnly, ast::type::ImageFormat::kR32Float); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -963,9 +878,8 @@ } TEST_F(BuilderTest_Type, - StorageTexture_GenerateWriteonly_SampledTypeSint_Format_r32sint) { + StorageTexture_Generate_SampledTypeSint_Format_r32sint) { ast::type::StorageTexture s(ast::type::TextureDimension::k2d, - ast::AccessControl::kWriteOnly, ast::type::ImageFormat::kR32Sint); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); @@ -977,9 +891,8 @@ } TEST_F(BuilderTest_Type, - StorageTexture_GenerateWriteonly_SampledTypeUint_Format_r32uint) { + StorageTexture_Generate_SampledTypeUint_Format_r32uint) { ast::type::StorageTexture s(ast::type::TextureDimension::k2d, - ast::AccessControl::kWriteOnly, ast::type::ImageFormat::kR32Uint); ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error();
diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index bbcc22d..9b67d42 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc
@@ -399,21 +399,41 @@ } bool GeneratorImpl::EmitType(ast::type::Type* type) { + std::string storage_texture_access = ""; if (auto* ac = type->As<ast::type::AccessControl>()) { - out_ << "[[access("; - if (ac->IsReadOnly()) { - out_ << "read"; - } else if (ac->IsReadWrite()) { - out_ << "read_write"; + // TODO(dsinclair): Removing the special casing for storage textures when + // we've converted over to parsing texture_storage_yy. + if (ac->type()->Is<ast::type::StorageTexture>()) { + if (ac->access_control() == ast::AccessControl::kReadOnly) { + storage_texture_access = "ro_"; + } else if (ac->access_control() == ast::AccessControl::kWriteOnly) { + storage_texture_access = "wo_"; + } else { + error_ = "unknown storage texture access"; + return false; + } + + // We want to generate the wrapped type in this case. + type = ac->type(); } else { - error_ = "invalid access control"; - return false; + out_ << "[[access("; + if (ac->IsReadOnly()) { + out_ << "read"; + } else if (ac->IsReadWrite()) { + out_ << "read_write"; + } else { + error_ = "invalid access control"; + return false; + } + out_ << ")]]" << std::endl; + if (!EmitType(ac->type())) { + return false; + } + return true; } - out_ << ")]]" << std::endl; - if (!EmitType(ac->type())) { - return false; - } - } else if (auto* alias = type->As<ast::type::Alias>()) { + } + + if (auto* alias = type->As<ast::type::Alias>()) { out_ << module_.SymbolToName(alias->symbol()); } else if (auto* ary = type->As<ast::type::Array>()) { for (auto* deco : ary->decorations()) { @@ -468,15 +488,7 @@ } else if (texture->Is<ast::type::MultisampledTexture>()) { out_ << "multisampled_"; } else if (auto* storage = texture->As<ast::type::StorageTexture>()) { - out_ << "storage_"; - if (storage->access() == ast::AccessControl::kReadOnly) { - out_ << "ro_"; - } else if (storage->access() == ast::AccessControl::kWriteOnly) { - out_ << "wo_"; - } else { - error_ = "unknown storage texture access"; - return false; - } + out_ << "storage_" << storage_texture_access; } else { error_ = "unknown texture type"; return false;
diff --git a/src/writer/wgsl/generator_impl_type_test.cc b/src/writer/wgsl/generator_impl_type_test.cc index dfc9e7c..2b8a0e8 100644 --- a/src/writer/wgsl/generator_impl_type_test.cc +++ b/src/writer/wgsl/generator_impl_type_test.cc
@@ -322,9 +322,10 @@ TEST_P(WgslGenerator_StorageTextureTest, EmitType_StorageTexture) { auto param = GetParam(); - ast::type::StorageTexture t(param.dim, param.access, param.fmt); + ast::type::StorageTexture t(param.dim, param.fmt); + ast::type::AccessControl ac(param.access, &t); - ASSERT_TRUE(gen.EmitType(&t)) << gen.error(); + ASSERT_TRUE(gen.EmitType(&ac)) << gen.error(); EXPECT_EQ(gen.result(), param.name); } INSTANTIATE_TEST_SUITE_P(