Remove ast::AccessControl::kInvalid
Also spruce up texture validation tests so that they validate error
messages.
Bug: tint:805
Change-Id: I6c86fc16014b127a7ef8254e5badf9b5bed08623
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51860
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/ast/access_control.cc b/src/ast/access_control.cc
index 52e3532..635eeb6 100644
--- a/src/ast/access_control.cc
+++ b/src/ast/access_control.cc
@@ -37,9 +37,6 @@
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;
@@ -57,9 +54,6 @@
std::ostringstream out;
out << "[[access(";
switch (access_) {
- case ast::AccessControl::kInvalid:
- out << "invalid";
- break;
case ast::AccessControl::kReadOnly:
out << "read";
break;
@@ -83,10 +77,6 @@
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 23ce43d..fa5f92a 100644
--- a/src/ast/access_control.h
+++ b/src/ast/access_control.h
@@ -28,9 +28,6 @@
public:
/// The access control settings
enum Access {
- /// Invalid
- // TODO(crbug.com/tint/805): Remove this.
- kInvalid = -1,
/// Read only
kReadOnly,
/// Write only
diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc
index b687b22..527e0d1 100644
--- a/src/inspector/inspector.cc
+++ b/src/inspector/inspector.cc
@@ -641,10 +641,6 @@
auto* var = rsv.first;
auto binding_info = rsv.second;
- if (var->AccessControl() == ast::AccessControl::kInvalid) {
- continue;
- }
-
if (read_only != (var->AccessControl() == ast::AccessControl::kReadOnly)) {
continue;
}
diff --git a/src/intrinsic_table.cc b/src/intrinsic_table.cc
index e313d0a..bcbb5c3 100644
--- a/src/intrinsic_table.cc
+++ b/src/intrinsic_table.cc
@@ -40,7 +40,8 @@
enum class OpenNumber {
N, // Typically used for vecN
M, // Typically used for matNxM
- F, // Typically used for texture_storage_2d<F>
+ F, // Typically used F in for texture_storage_2d<F, A>
+ A, // Typically used for A in texture_storage_2d<F, A>
};
/// @return a string of the OpenType symbol `ty`
@@ -64,6 +65,8 @@
return "M";
case OpenNumber::F:
return "F";
+ case OpenNumber::A:
+ return "A";
}
return "";
}
@@ -516,13 +519,23 @@
/// the given texel and channel formats.
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"
+ 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),
+ access_is_open_num_(false),
+ texel_format_(texel_format),
+ channel_format_(channel_format) {}
+
+ StorageTextureBuilder(ast::TextureDimension dimensions,
+ OpenNumber access,
+ OpenNumber texel_format, // a.k.a "image format"
+ OpenType channel_format) // a.k.a "storage subtype"
+ : dimensions_(dimensions),
+ access_(access),
+ access_is_open_num_(true),
texel_format_(texel_format),
channel_format_(channel_format) {}
@@ -531,10 +544,16 @@
if (MatchOpenNumber(state, texel_format_,
static_cast<uint32_t>(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;
+ if (access_is_open_num_) {
+ if (!MatchOpenNumber(
+ state, access_.open_num,
+ static_cast<uint32_t>(tex->access_control()))) {
+ return false;
+ }
+ } else {
+ if (access_.enum_val != tex->access_control()) {
+ return false;
+ }
}
return tex->dim() == dimensions_;
@@ -547,9 +566,13 @@
sem::Type* Build(BuildState& state) const override {
auto texel_format =
static_cast<ast::ImageFormat>(state.open_numbers.at(texel_format_));
+ auto access = access_is_open_num_
+ ? static_cast<ast::AccessControl::Access>(
+ state.open_numbers.at(access_.open_num))
+ : access_.enum_val;
auto* channel_format = state.open_types.at(channel_format_);
return state.ty_mgr.Get<sem::StorageTexture>(
- dimensions_, texel_format, access_,
+ dimensions_, texel_format, access,
const_cast<sem::Type*>(channel_format));
}
@@ -557,10 +580,10 @@
std::stringstream ss;
ss << "texture_storage_" << dimensions_ << "<F, ";
- if (access_ == ast::AccessControl::Access::kInvalid) {
+ if (access_is_open_num_) {
ss << "A";
} else {
- ss << access_;
+ ss << access_.enum_val;
}
ss << ">";
@@ -569,7 +592,14 @@
private:
ast::TextureDimension const dimensions_;
- ast::AccessControl::Access const access_;
+ union Access {
+ Access(OpenNumber in) : open_num(in) {}
+ Access(ast::AccessControl::Access in) : enum_val(in) {}
+
+ OpenNumber const open_num;
+ ast::AccessControl::Access const enum_val;
+ } access_;
+ bool access_is_open_num_;
OpenNumber const texel_format_;
OpenType const channel_format_;
};
@@ -748,6 +778,14 @@
dimensions, access, texel_format, channel_format);
}
+ Builder* storage_texture(ast::TextureDimension dimensions,
+ OpenNumber access,
+ OpenNumber texel_format,
+ OpenType channel_format) {
+ return matcher_allocator_.Create<StorageTextureBuilder>(
+ dimensions, access, texel_format, channel_format);
+ }
+
/// @returns a Matcher / Builder that matches an external texture
Builder* external_texture() {
return matcher_allocator_.Create<ExternalTextureBuilder>();
@@ -1079,14 +1117,14 @@
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, 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_1d_FT =
+ storage_texture(Dim::k1d, OpenNumber::A, OpenNumber::F, OpenType::T);
+ auto* tex_storage_2d_FT =
+ storage_texture(Dim::k2d, OpenNumber::A, OpenNumber::F, OpenType::T);
+ auto* tex_storage_2d_array_FT =
+ storage_texture(Dim::k2dArray, OpenNumber::A, OpenNumber::F, OpenType::T);
+ auto* tex_storage_3d_FT =
+ storage_texture(Dim::k3d, OpenNumber::A, 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(
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 384cfde..e63800f 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -284,7 +284,7 @@
return builder_->create<sem::F32>();
}
if (auto* t = ty->As<ast::AccessControl>()) {
- TINT_SCOPED_ASSIGNMENT(curent_access_control_, t);
+ TINT_SCOPED_ASSIGNMENT(current_access_control_, t);
if (auto* el = Type(t->type())) {
return el;
}
@@ -337,18 +337,15 @@
}
if (auto* t = ty->As<ast::StorageTexture>()) {
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<accesscontrol<storagetexture>>).
+ if (!current_access_control_) {
+ diagnostics_.add_error("storage textures must have access control",
+ t->source());
+ return nullptr;
}
-
return builder_->create<sem::StorageTexture>(
- t->dim(), t->image_format(), ac, const_cast<sem::Type*>(el));
+ t->dim(), t->image_format(),
+ current_access_control_->access_control(),
+ const_cast<sem::Type*>(el));
}
return nullptr;
}
@@ -485,11 +482,7 @@
return nullptr;
};
- auto access_control = ast::AccessControl::kInvalid;
- if (auto* ac = find_first_access_control(var->type())) {
- access_control = ac->access_control();
- }
-
+ auto* access_control = find_first_access_control(var->type());
auto* info = variable_infos_.Create(var, const_cast<sem::Type*>(type),
type_name, storage_class, access_control);
variable_to_info_.emplace(var, info);
@@ -666,7 +659,7 @@
// Its store type must be a host-shareable structure type with block
// attribute, satisfying the storage class constraints.
- auto* str = info->access_control != ast::AccessControl::kInvalid
+ auto* str = info->access_control
? info->type->UnwrapRef()->As<sem::Struct>()
: nullptr;
@@ -740,7 +733,7 @@
if (auto* r = storage_type->As<sem::MultisampledTexture>()) {
if (r->dim() != ast::TextureDimension::k2d) {
- diagnostics_.add_error("Only 2d multisampled textures are supported",
+ diagnostics_.add_error("only 2d multisampled textures are supported",
var->source());
return false;
}
@@ -754,24 +747,18 @@
}
if (auto* storage_tex = info->type->UnwrapRef()->As<sem::StorageTexture>()) {
- if (storage_tex->access_control() == ast::AccessControl::kInvalid) {
- diagnostics_.add_error("Storage Textures must have access control.",
- var->source());
- return false;
- }
-
- if (info->access_control == ast::AccessControl::kReadWrite) {
+ if (info->access_control->access_control() ==
+ ast::AccessControl::kReadWrite) {
diagnostics_.add_error(
- "Storage Textures only support Read-Only and Write-Only access "
- "control.",
+ "storage textures only support read-only and write-only access",
var->source());
return false;
}
if (!IsValidStorageTextureDimension(storage_tex->dim())) {
diagnostics_.add_error(
- "Cube dimensions for storage textures are not "
- "supported.",
+ "cube dimensions for storage textures are not "
+ "supported",
var->source());
return false;
}
@@ -2548,7 +2535,9 @@
sem_var = builder_->create<sem::Variable>(var, info->type, constant_id);
} else {
sem_var = builder_->create<sem::Variable>(
- var, info->type, info->storage_class, info->access_control);
+ var, info->type, info->storage_class,
+ info->access_control ? info->access_control->access_control()
+ : ast::AccessControl::kReadWrite);
}
std::vector<const sem::VariableUser*> users;
@@ -3235,7 +3224,7 @@
sem::Type* ty,
const std::string& tn,
ast::StorageClass sc,
- ast::AccessControl::Access ac)
+ const ast::AccessControl* ac)
: declaration(decl),
type(ty),
type_name(tn),
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 2df4789..402e051 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -88,14 +88,14 @@
sem::Type* type,
const std::string& type_name,
ast::StorageClass storage_class,
- ast::AccessControl::Access ac);
+ const ast::AccessControl* 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;
+ ast::AccessControl const* const access_control;
std::vector<ast::IdentifierExpression*> users;
sem::BindingPoint binding_point;
};
@@ -382,7 +382,7 @@
FunctionInfo* current_function_ = nullptr;
sem::Statement* current_statement_ = nullptr;
- const ast::AccessControl* curent_access_control_ = nullptr;
+ const ast::AccessControl* current_access_control_ = nullptr;
BlockAllocator<VariableInfo> variable_infos_;
BlockAllocator<FunctionInfo> function_infos_;
};
diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc
index 86481e9..29365ae 100644
--- a/src/resolver/type_validation_test.cc
+++ b/src/resolver/type_validation_test.cc
@@ -430,7 +430,7 @@
using MultisampledTextureDimensionTest = ResolverTestWithParam<DimensionParams>;
TEST_P(MultisampledTextureDimensionTest, All) {
auto& params = GetParam();
- Global("a", ty.multisampled_texture(params.dim, ty.i32()),
+ Global(Source{{12, 34}}, "a", ty.multisampled_texture(params.dim, ty.i32()),
ast::StorageClass::kNone, nullptr,
ast::DecorationList{
create<ast::BindingDecoration>(0),
@@ -441,6 +441,8 @@
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: only 2d multisampled textures are supported");
}
}
INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -473,7 +475,7 @@
TEST_P(MultisampledTextureTypeTest, All) {
auto& params = GetParam();
Global(
- "a",
+ Source{{12, 34}}, "a",
ty.multisampled_texture(ast::TextureDimension::k2d, params.type_func(ty)),
ast::StorageClass::kNone, nullptr,
ast::DecorationList{
@@ -485,6 +487,9 @@
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: texture_multisampled_2d<type>: type must be f32, "
+ "i32 or u32");
}
}
INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -516,7 +521,7 @@
auto* st = ty.storage_texture(params.dim, ast::ImageFormat::kR32Uint);
auto* ac = ty.access(ast::AccessControl::kReadOnly, st);
- Global("a", ac, ast::StorageClass::kNone, nullptr,
+ Global(Source{{12, 34}}, "a", ac, ast::StorageClass::kNone, nullptr,
ast::DecorationList{
create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0),
@@ -526,6 +531,9 @@
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ "12:34 error: cube dimensions for storage textures are not supported");
}
}
INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -588,7 +596,7 @@
auto* st_a = ty.storage_texture(ast::TextureDimension::k1d, params.format);
auto* ac_a = ty.access(ast::AccessControl::kReadOnly, st_a);
- Global("a", ac_a, ast::StorageClass::kNone, nullptr,
+ Global(Source{{12, 34}}, "a", ac_a, ast::StorageClass::kNone, nullptr,
ast::DecorationList{
create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0),
@@ -623,6 +631,10 @@
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: image format must be one of the texel formats "
+ "specified for storage textues in "
+ "https://gpuweb.github.io/gpuweb/wgsl/#texel-formats");
}
}
INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -635,7 +647,7 @@
// [[group(0), binding(0)]]
// var a : texture_storage_1d<ru32int>;
- auto* st = ty.storage_texture(ast::TextureDimension::k1d,
+ auto* st = ty.storage_texture(Source{{12, 34}}, ast::TextureDimension::k1d,
ast::ImageFormat::kR32Uint);
Global("a", st, ast::StorageClass::kNone, nullptr,
@@ -645,6 +657,8 @@
});
EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: storage textures must have access control");
}
TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) {
@@ -653,14 +667,18 @@
auto* st = ty.storage_texture(ast::TextureDimension::k1d,
ast::ImageFormat::kR32Uint);
+ auto* ac = ty.access(ast::AccessControl::kReadWrite, st);
- Global("a", st, ast::StorageClass::kNone, nullptr,
+ Global(Source{{12, 34}}, "a", ac, ast::StorageClass::kNone, nullptr,
ast::DecorationList{
create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0),
});
EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: storage textures only support read-only and "
+ "write-only access");
}
TEST_F(StorageTextureAccessControlTest, ReadOnlyAccessControl_Pass) {
diff --git a/src/sem/variable.cc b/src/sem/variable.cc
index 3700c06..a8cd011 100644
--- a/src/sem/variable.cc
+++ b/src/sem/variable.cc
@@ -39,7 +39,7 @@
: declaration_(declaration),
type_(type),
storage_class_(ast::StorageClass::kNone),
- access_control_(ast::AccessControl::kInvalid),
+ access_control_(ast::AccessControl::kReadWrite),
is_pipeline_constant_(true),
constant_id_(constant_id) {}
diff --git a/src/transform/decompose_storage_access.cc b/src/transform/decompose_storage_access.cc
index 46495a1..e718b13 100644
--- a/src/transform/decompose_storage_access.cc
+++ b/src/transform/decompose_storage_access.cc
@@ -364,7 +364,7 @@
const sem::VariableUser* var_user,
ast::Type* ty) {
if (var_user &&
- var_user->Variable()->AccessControl() != ast::AccessControl::kInvalid) {
+ var_user->Variable()->StorageClass() == ast::StorageClass::kStorage) {
return ctx->dst->create<ast::AccessControl>(
var_user->Variable()->AccessControl(), ty);
}
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index 626cc77..5a3621b 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -249,7 +249,7 @@
out << "as";
if (!EmitType(out, type, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "")) {
+ ast::AccessControl::kReadWrite, "")) {
return false;
}
out << "(";
@@ -1309,7 +1309,7 @@
out << "{";
} else {
if (!EmitType(out, type, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "")) {
+ ast::AccessControl::kReadWrite, "")) {
return false;
}
out << "(";
@@ -1571,7 +1571,7 @@
auto* func = builder_.Sem().Get(func_ast);
if (!EmitType(out, func->ReturnType(), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "")) {
+ ast::AccessControl::kReadWrite, "")) {
return false;
}
@@ -1750,11 +1750,6 @@
continue; // Global already emitted
}
- if (var->AccessControl() == ast::AccessControl::kInvalid) {
- diagnostics_.add_error("access control type required for storage buffer");
- return false;
- }
-
auto* type = var->Type()->UnwrapRef();
if (!EmitType(out, type, ast::StorageClass::kStorage, var->AccessControl(),
"")) {
@@ -2129,7 +2124,7 @@
out << "0u";
} else if (auto* vec = type->As<sem::Vector>()) {
if (!EmitType(out, type, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "")) {
+ ast::AccessControl::kReadWrite, "")) {
return false;
}
ScopedParen sp(out);
@@ -2143,7 +2138,7 @@
}
} else if (auto* mat = type->As<sem::Matrix>()) {
if (!EmitType(out, type, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "")) {
+ ast::AccessControl::kReadWrite, "")) {
return false;
}
ScopedParen sp(out);
@@ -2515,7 +2510,7 @@
auto mem_name = builder_.Symbols().NameFor(mem->Declaration()->symbol());
if (!EmitType(out, mem->Type(), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, mem_name)) {
+ ast::AccessControl::kReadWrite, mem_name)) {
return false;
}
// Array member name will be output with the type
diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc
index a4c2b97..1346cc8 100644
--- a/src/writer/hlsl/generator_impl_type_test.cc
+++ b/src/writer/hlsl/generator_impl_type_test.cc
@@ -38,7 +38,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "ary"))
+ ast::AccessControl::kReadWrite, "ary"))
<< gen.error();
EXPECT_EQ(result(), "bool ary[4]");
}
@@ -50,7 +50,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "ary"))
+ ast::AccessControl::kReadWrite, "ary"))
<< gen.error();
EXPECT_EQ(result(), "bool ary[5][4]");
}
@@ -64,7 +64,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "ary"))
+ ast::AccessControl::kReadWrite, "ary"))
<< gen.error();
EXPECT_EQ(result(), "bool ary[5][4][1]");
}
@@ -76,7 +76,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "ary"))
+ ast::AccessControl::kReadWrite, "ary"))
<< gen.error();
EXPECT_EQ(result(), "bool ary[6][5][4]");
}
@@ -88,7 +88,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "bool[4]");
}
@@ -100,7 +100,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, "ary"))
+ ast::AccessControl::kReadWrite, "ary"))
<< gen.error();
EXPECT_EQ(result(), "bool ary[]");
}
@@ -111,7 +111,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, bool_, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "bool");
}
@@ -122,7 +122,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, f32, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "float");
}
@@ -133,7 +133,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, i32, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "int");
}
@@ -146,7 +146,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, mat2x3, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "float2x3");
}
@@ -159,7 +159,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, p, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "float*");
}
@@ -214,7 +214,7 @@
auto* sem_s = program->TypeOf(s)->As<sem::Struct>();
ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "S");
}
@@ -234,7 +234,7 @@
auto* sem_s = program->TypeOf(s)->As<sem::Struct>();
ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(gen.result(), R"(struct S {
int a;
@@ -290,7 +290,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, u32, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "uint");
}
@@ -302,7 +302,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, vec3, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "float3");
}
@@ -313,7 +313,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, void_, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "void");
}
@@ -324,7 +324,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "SamplerState");
}
@@ -335,7 +335,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "SamplerComparisonState");
}
@@ -532,7 +532,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, s, ast::StorageClass::kNone,
- ast::AccessControl::kInvalid, ""))
+ ast::AccessControl::kReadWrite, ""))
<< gen.error();
EXPECT_EQ(result(), "Texture2DMS<float4>");
}
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index f267f06..e10bab5 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -784,35 +784,35 @@
if (var->has_constructor()) {
ops.push_back(Operand::Int(init_id));
- } 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->Is<sem::Sampler>()) {
- // 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);
- if (init_id == 0) {
- return 0;
+ } else {
+ if (type->Is<sem::StorageTexture>() || type->Is<sem::Struct>()) {
+ // type is a sem::Struct or a sem::StorageTexture
+ switch (sem->AccessControl()) {
+ 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;
}
- ops.push_back(Operand::Int(init_id));
+ }
+ if (!type->Is<sem::Sampler>()) {
+ // 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);
+ if (init_id == 0) {
+ return 0;
+ }
+ ops.push_back(Operand::Int(init_id));
+ }
}
}