encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Antonio Maiorano 2021-05-21 14:32:38 +00:00 committed by Tint LUCI CQ
parent ee0b69ce53
commit 6cf7f2eca5
12 changed files with 162 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/ast/access_control.cc
View File

@ -37,9 +37,6 @@ AccessControl::~AccessControl() = default;
std::string AccessControl::type_name() const { std::string AccessControl::type_name() const {
std::string name = "__access_control_"; std::string name = "__access_control_";
switch (access_) { switch (access_) {
case ast::AccessControl::kInvalid:
name += "invalid";
break;
case ast::AccessControl::kReadOnly: case ast::AccessControl::kReadOnly:
name += "read_only"; name += "read_only";
break; break;
@ -57,9 +54,6 @@ std::string AccessControl::FriendlyName(const SymbolTable& symbols) const {
std::ostringstream out; std::ostringstream out;
out << "[[access("; out << "[[access(";
switch (access_) { switch (access_) {
case ast::AccessControl::kInvalid:
out << "invalid";
break;
case ast::AccessControl::kReadOnly: case ast::AccessControl::kReadOnly:
out << "read"; out << "read";
break; break;
@ -83,10 +77,6 @@ AccessControl* AccessControl::Clone(CloneContext* ctx) const {
std::ostream& operator<<(std::ostream& out, AccessControl::Access access) { std::ostream& operator<<(std::ostream& out, AccessControl::Access access) {
switch (access) { switch (access) {
case ast::AccessControl::kInvalid: {
out << "invalid";
break;
}
case ast::AccessControl::kReadOnly: { case ast::AccessControl::kReadOnly: {
out << "read_only"; out << "read_only";
break; break;

3
src/ast/access_control.h
View File

@ -28,9 +28,6 @@ class AccessControl : public Castable<AccessControl, Type> {
public: public:
/// The access control settings /// The access control settings
enum Access { enum Access {
/// Invalid
// TODO(crbug.com/tint/805): Remove this.
kInvalid = -1,
/// Read only /// Read only
kReadOnly, kReadOnly,
/// Write only /// Write only

4
src/inspector/inspector.cc
View File

@ -641,10 +641,6 @@ std::vector<ResourceBinding> Inspector::GetStorageBufferResourceBindingsImpl(
auto* var = rsv.first; auto* var = rsv.first;
auto binding_info = rsv.second; auto binding_info = rsv.second;
if (var->AccessControl() == ast::AccessControl::kInvalid) {
continue;
}
if (read_only != (var->AccessControl() == ast::AccessControl::kReadOnly)) { if (read_only != (var->AccessControl() == ast::AccessControl::kReadOnly)) {
continue; continue;
} }

74
src/intrinsic_table.cc
View File

@ -40,7 +40,8 @@ enum class OpenType {
enum class OpenNumber { enum class OpenNumber {
N, // Typically used for vecN N, // Typically used for vecN
M, // Typically used for matNxM 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` /// @return a string of the OpenType symbol `ty`
@ -64,6 +65,8 @@ const char* str(OpenNumber num) {
return "M"; return "M";
case OpenNumber::F: case OpenNumber::F:
return "F"; return "F";
case OpenNumber::A:
return "A";
} }
return ""; return "";
} }
@ -516,13 +519,23 @@ class DepthTextureBuilder : public Builder {
/// the given texel and channel formats. /// the given texel and channel formats.
class StorageTextureBuilder : public Builder { class StorageTextureBuilder : public Builder {
public: public:
explicit StorageTextureBuilder( StorageTextureBuilder(ast::TextureDimension dimensions,
ast::TextureDimension dimensions,
ast::AccessControl::Access access, ast::AccessControl::Access access,
OpenNumber texel_format, // a.k.a "image format" OpenNumber texel_format, // a.k.a "image format"
OpenType channel_format) // a.k.a "storage subtype" OpenType channel_format) // a.k.a "storage subtype"
: dimensions_(dimensions), : dimensions_(dimensions),
access_(access), 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), texel_format_(texel_format),
channel_format_(channel_format) {} channel_format_(channel_format) {}
@ -531,11 +544,17 @@ class StorageTextureBuilder : public Builder {
if (MatchOpenNumber(state, texel_format_, if (MatchOpenNumber(state, texel_format_,
static_cast<uint32_t>(tex->image_format()))) { static_cast<uint32_t>(tex->image_format()))) {
if (MatchOpenType(state, channel_format_, tex->type())) { if (MatchOpenType(state, channel_format_, tex->type())) {
// AccessControl::kInvalid means match any if (access_is_open_num_) {
if (access_ != ast::AccessControl::kInvalid && if (!MatchOpenNumber(
access_ != tex->access_control()) { state, access_.open_num,
static_cast<uint32_t>(tex->access_control()))) {
return false; return false;
} }
} else {
if (access_.enum_val != tex->access_control()) {
return false;
}
}
return tex->dim() == dimensions_; return tex->dim() == dimensions_;
} }
@ -547,9 +566,13 @@ class StorageTextureBuilder : public Builder {
sem::Type* Build(BuildState& state) const override { sem::Type* Build(BuildState& state) const override {
auto texel_format = auto texel_format =
static_cast<ast::ImageFormat>(state.open_numbers.at(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_); auto* channel_format = state.open_types.at(channel_format_);
return state.ty_mgr.Get<sem::StorageTexture>( return state.ty_mgr.Get<sem::StorageTexture>(
dimensions_, texel_format, access_, dimensions_, texel_format, access,
const_cast<sem::Type*>(channel_format)); const_cast<sem::Type*>(channel_format));
} }
@ -557,10 +580,10 @@ class StorageTextureBuilder : public Builder {
std::stringstream ss; std::stringstream ss;
ss << "texture_storage_" << dimensions_ << "<F, "; ss << "texture_storage_" << dimensions_ << "<F, ";
if (access_ == ast::AccessControl::Access::kInvalid) { if (access_is_open_num_) {
ss << "A"; ss << "A";
} else { } else {
ss << access_; ss << access_.enum_val;
} }
ss << ">"; ss << ">";
@ -569,7 +592,14 @@ class StorageTextureBuilder : public Builder {
private: private:
ast::TextureDimension const dimensions_; 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_; OpenNumber const texel_format_;
OpenType const channel_format_; OpenType const channel_format_;
}; };
@ -748,6 +778,14 @@ class Impl : public IntrinsicTable {
dimensions, access, texel_format, channel_format); 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 /// @returns a Matcher / Builder that matches an external texture
Builder* external_texture() { Builder* external_texture() {
return matcher_allocator_.Create<ExternalTextureBuilder>(); return matcher_allocator_.Create<ExternalTextureBuilder>();
@ -1079,14 +1117,14 @@ Impl::Impl() {
auto* tex_depth_cube = depth_texture(Dim::kCube); auto* tex_depth_cube = depth_texture(Dim::kCube);
auto* tex_depth_cube_array = depth_texture(Dim::kCubeArray); auto* tex_depth_cube_array = depth_texture(Dim::kCubeArray);
auto* tex_external = external_texture(); auto* tex_external = external_texture();
auto* tex_storage_1d_FT = storage_texture( auto* tex_storage_1d_FT =
Dim::k1d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); storage_texture(Dim::k1d, OpenNumber::A, OpenNumber::F, OpenType::T);
auto* tex_storage_2d_FT = storage_texture( auto* tex_storage_2d_FT =
Dim::k2d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); storage_texture(Dim::k2d, OpenNumber::A, OpenNumber::F, OpenType::T);
auto* tex_storage_2d_array_FT = storage_texture( auto* tex_storage_2d_array_FT =
Dim::k2dArray, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); storage_texture(Dim::k2dArray, OpenNumber::A, OpenNumber::F, OpenType::T);
auto* tex_storage_3d_FT = storage_texture( auto* tex_storage_3d_FT =
Dim::k3d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T); storage_texture(Dim::k3d, OpenNumber::A, OpenNumber::F, OpenType::T);
auto* tex_storage_ro_1d_FT = storage_texture( auto* tex_storage_ro_1d_FT = storage_texture(
Dim::k1d, ast::AccessControl::kReadOnly, OpenNumber::F, OpenType::T); Dim::k1d, ast::AccessControl::kReadOnly, OpenNumber::F, OpenType::T);
auto* tex_storage_ro_2d_FT = storage_texture( auto* tex_storage_ro_2d_FT = storage_texture(

51
src/resolver/resolver.cc
View File

@ -284,7 +284,7 @@ sem::Type* Resolver::Type(const ast::Type* ty) {
return builder_->create<sem::F32>(); return builder_->create<sem::F32>();
} }
if (auto* t = ty->As<ast::AccessControl>()) { 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())) { if (auto* el = Type(t->type())) {
return el; return el;
} }
@ -337,18 +337,15 @@ sem::Type* Resolver::Type(const ast::Type* ty) {
} }
if (auto* t = ty->As<ast::StorageTexture>()) { if (auto* t = ty->As<ast::StorageTexture>()) {
if (auto* el = Type(t->type())) { if (auto* el = Type(t->type())) {
auto ac = ast::AccessControl::kInvalid; if (!current_access_control_) {
if (curent_access_control_) { diagnostics_.add_error("storage textures must have access control",
ac = curent_access_control_->access_control(); t->source());
} else { return nullptr;
// 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>>).
} }
return builder_->create<sem::StorageTexture>( 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; return nullptr;
} }
@ -485,11 +482,7 @@ Resolver::VariableInfo* Resolver::Variable(ast::Variable* var,
return nullptr; return nullptr;
}; };
auto access_control = ast::AccessControl::kInvalid; auto* access_control = find_first_access_control(var->type());
if (auto* ac = find_first_access_control(var->type())) {
access_control = ac->access_control();
}
auto* info = variable_infos_.Create(var, const_cast<sem::Type*>(type), auto* info = variable_infos_.Create(var, const_cast<sem::Type*>(type),
type_name, storage_class, access_control); type_name, storage_class, access_control);
variable_to_info_.emplace(var, info); variable_to_info_.emplace(var, info);
@ -666,7 +659,7 @@ bool Resolver::ValidateGlobalVariable(const VariableInfo* info) {
// Its store type must be a host-shareable structure type with block // Its store type must be a host-shareable structure type with block
// attribute, satisfying the storage class constraints. // 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>() ? info->type->UnwrapRef()->As<sem::Struct>()
: nullptr; : nullptr;
@ -740,7 +733,7 @@ bool Resolver::ValidateVariable(const VariableInfo* info) {
if (auto* r = storage_type->As<sem::MultisampledTexture>()) { if (auto* r = storage_type->As<sem::MultisampledTexture>()) {
if (r->dim() != ast::TextureDimension::k2d) { 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()); var->source());
return false; return false;
} }
@ -754,24 +747,18 @@ bool Resolver::ValidateVariable(const VariableInfo* info) {
} }
if (auto* storage_tex = info->type->UnwrapRef()->As<sem::StorageTexture>()) { if (auto* storage_tex = info->type->UnwrapRef()->As<sem::StorageTexture>()) {
if (storage_tex->access_control() == ast::AccessControl::kInvalid) { if (info->access_control->access_control() ==
diagnostics_.add_error("Storage Textures must have access control.", ast::AccessControl::kReadWrite) {
var->source());
return false;
}
if (info->access_control == ast::AccessControl::kReadWrite) {
diagnostics_.add_error( diagnostics_.add_error(
"Storage Textures only support Read-Only and Write-Only access " "storage textures only support read-only and write-only access",
"control.",
var->source()); var->source());
return false; return false;
} }
if (!IsValidStorageTextureDimension(storage_tex->dim())) { if (!IsValidStorageTextureDimension(storage_tex->dim())) {
diagnostics_.add_error( diagnostics_.add_error(
"Cube dimensions for storage textures are not " "cube dimensions for storage textures are not "
"supported.", "supported",
var->source()); var->source());
return false; return false;
} }
@ -2548,7 +2535,9 @@ void Resolver::CreateSemanticNodes() const {
sem_var = builder_->create<sem::Variable>(var, info->type, constant_id); sem_var = builder_->create<sem::Variable>(var, info->type, constant_id);
} else { } else {
sem_var = builder_->create<sem::Variable>( 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; std::vector<const sem::VariableUser*> users;
@ -3235,7 +3224,7 @@ Resolver::VariableInfo::VariableInfo(const ast::Variable* decl,
sem::Type* ty, sem::Type* ty,
const std::string& tn, const std::string& tn,
ast::StorageClass sc, ast::StorageClass sc,
ast::AccessControl::Access ac) const ast::AccessControl* ac)
: declaration(decl), : declaration(decl),
type(ty), type(ty),
type_name(tn), type_name(tn),

6
src/resolver/resolver.h
View File

@ -88,14 +88,14 @@ class Resolver {
sem::Type* type, sem::Type* type,
const std::string& type_name, const std::string& type_name,
ast::StorageClass storage_class, ast::StorageClass storage_class,
ast::AccessControl::Access ac); const ast::AccessControl* ac);
~VariableInfo(); ~VariableInfo();
ast::Variable const* const declaration; ast::Variable const* const declaration;
sem::Type* type; sem::Type* type;
std::string const type_name; std::string const type_name;
ast::StorageClass storage_class; ast::StorageClass storage_class;
ast::AccessControl::Access const access_control; ast::AccessControl const* const access_control;
std::vector<ast::IdentifierExpression*> users; std::vector<ast::IdentifierExpression*> users;
sem::BindingPoint binding_point; sem::BindingPoint binding_point;
}; };
@ -382,7 +382,7 @@ class Resolver {
FunctionInfo* current_function_ = nullptr; FunctionInfo* current_function_ = nullptr;
sem::Statement* current_statement_ = 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<VariableInfo> variable_infos_;
BlockAllocator<FunctionInfo> function_infos_; BlockAllocator<FunctionInfo> function_infos_;
}; };

30
src/resolver/type_validation_test.cc
View File

@ -430,7 +430,7 @@ static constexpr DimensionParams dimension_cases[] = {
using MultisampledTextureDimensionTest = ResolverTestWithParam<DimensionParams>; using MultisampledTextureDimensionTest = ResolverTestWithParam<DimensionParams>;
TEST_P(MultisampledTextureDimensionTest, All) { TEST_P(MultisampledTextureDimensionTest, All) {
auto& params = GetParam(); 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::StorageClass::kNone, nullptr,
ast::DecorationList{ ast::DecorationList{
create<ast::BindingDecoration>(0), create<ast::BindingDecoration>(0),
@ -441,6 +441,8 @@ TEST_P(MultisampledTextureDimensionTest, All) {
EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(r()->Resolve()) << r()->error();
} else { } else {
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: only 2d multisampled textures are supported");
} }
} }
INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest, INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@ -473,7 +475,7 @@ using MultisampledTextureTypeTest = ResolverTestWithParam<TypeParams>;
TEST_P(MultisampledTextureTypeTest, All) { TEST_P(MultisampledTextureTypeTest, All) {
auto& params = GetParam(); auto& params = GetParam();
Global( Global(
"a", Source{{12, 34}}, "a",
ty.multisampled_texture(ast::TextureDimension::k2d, params.type_func(ty)), ty.multisampled_texture(ast::TextureDimension::k2d, params.type_func(ty)),
ast::StorageClass::kNone, nullptr, ast::StorageClass::kNone, nullptr,
ast::DecorationList{ ast::DecorationList{
@ -485,6 +487,9 @@ TEST_P(MultisampledTextureTypeTest, All) {
EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(r()->Resolve()) << r()->error();
} else { } else {
EXPECT_FALSE(r()->Resolve()); 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, INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@ -516,7 +521,7 @@ TEST_P(StorageTextureDimensionTest, All) {
auto* st = ty.storage_texture(params.dim, ast::ImageFormat::kR32Uint); 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, Global(Source{{12, 34}}, "a", ac, ast::StorageClass::kNone, nullptr,
ast::DecorationList{ ast::DecorationList{
create<ast::BindingDecoration>(0), create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0), create<ast::GroupDecoration>(0),
@ -526,6 +531,9 @@ TEST_P(StorageTextureDimensionTest, All) {
EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(r()->Resolve()) << r()->error();
} else { } else {
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
"12:34 error: cube dimensions for storage textures are not supported");
} }
} }
INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest, INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@ -588,7 +596,7 @@ TEST_P(StorageTextureFormatTest, All) {
auto* st_a = ty.storage_texture(ast::TextureDimension::k1d, params.format); 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, Global(Source{{12, 34}}, "a", ac_a, ast::StorageClass::kNone, nullptr,
ast::DecorationList{ ast::DecorationList{
create<ast::BindingDecoration>(0), create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0), create<ast::GroupDecoration>(0),
@ -623,6 +631,10 @@ TEST_P(StorageTextureFormatTest, All) {
EXPECT_TRUE(r()->Resolve()) << r()->error(); EXPECT_TRUE(r()->Resolve()) << r()->error();
} else { } else {
EXPECT_FALSE(r()->Resolve()); 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, INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@ -635,7 +647,7 @@ TEST_F(StorageTextureAccessControlTest, MissingAccessControl_Fail) {
// [[group(0), binding(0)]] // [[group(0), binding(0)]]
// var a : texture_storage_1d<ru32int>; // 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); ast::ImageFormat::kR32Uint);
Global("a", st, ast::StorageClass::kNone, nullptr, Global("a", st, ast::StorageClass::kNone, nullptr,
@ -645,6 +657,8 @@ TEST_F(StorageTextureAccessControlTest, MissingAccessControl_Fail) {
}); });
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: storage textures must have access control");
} }
TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) { TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) {
@ -653,14 +667,18 @@ TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) {
auto* st = ty.storage_texture(ast::TextureDimension::k1d, auto* st = ty.storage_texture(ast::TextureDimension::k1d,
ast::ImageFormat::kR32Uint); 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{ ast::DecorationList{
create<ast::BindingDecoration>(0), create<ast::BindingDecoration>(0),
create<ast::GroupDecoration>(0), create<ast::GroupDecoration>(0),
}); });
EXPECT_FALSE(r()->Resolve()); 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) { TEST_F(StorageTextureAccessControlTest, ReadOnlyAccessControl_Pass) {

2
src/sem/variable.cc
View File

@ -39,7 +39,7 @@ Variable::Variable(const ast::Variable* declaration,
: declaration_(declaration), : declaration_(declaration),
type_(type), type_(type),
storage_class_(ast::StorageClass::kNone), storage_class_(ast::StorageClass::kNone),
access_control_(ast::AccessControl::kInvalid), access_control_(ast::AccessControl::kReadWrite),
is_pipeline_constant_(true), is_pipeline_constant_(true),
constant_id_(constant_id) {} constant_id_(constant_id) {}

2
src/transform/decompose_storage_access.cc
View File

@ -364,7 +364,7 @@ ast::Type* MaybeCreateASTAccessControl(CloneContext* ctx,
const sem::VariableUser* var_user, const sem::VariableUser* var_user,
ast::Type* ty) { ast::Type* ty) {
if (var_user && if (var_user &&
var_user->Variable()->AccessControl() != ast::AccessControl::kInvalid) { var_user->Variable()->StorageClass() == ast::StorageClass::kStorage) {
return ctx->dst->create<ast::AccessControl>( return ctx->dst->create<ast::AccessControl>(
var_user->Variable()->AccessControl(), ty); var_user->Variable()->AccessControl(), ty);
} }

17
src/writer/hlsl/generator_impl.cc
View File

@ -249,7 +249,7 @@ bool GeneratorImpl::EmitBitcast(std::ostream& pre,
out << "as"; out << "as";
if (!EmitType(out, type, ast::StorageClass::kNone, if (!EmitType(out, type, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) { ast::AccessControl::kReadWrite, "")) {
return false; return false;
} }
out << "("; out << "(";
@ -1309,7 +1309,7 @@ bool GeneratorImpl::EmitTypeConstructor(std::ostream& pre,
out << "{"; out << "{";
} else { } else {
if (!EmitType(out, type, ast::StorageClass::kNone, if (!EmitType(out, type, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) { ast::AccessControl::kReadWrite, "")) {
return false; return false;
} }
out << "("; out << "(";
@ -1571,7 +1571,7 @@ bool GeneratorImpl::EmitFunctionInternal(std::ostream& out,
auto* func = builder_.Sem().Get(func_ast); 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, "")) { ast::AccessControl::kReadWrite, "")) {
return false; return false;
} }
@ -1750,11 +1750,6 @@ bool GeneratorImpl::EmitEntryPointData(
continue; // Global already emitted 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(); auto* type = var->Type()->UnwrapRef();
if (!EmitType(out, type, ast::StorageClass::kStorage, var->AccessControl(), if (!EmitType(out, type, ast::StorageClass::kStorage, var->AccessControl(),
"")) { "")) {
@ -2129,7 +2124,7 @@ bool GeneratorImpl::EmitZeroValue(std::ostream& out, const sem::Type* type) {
out << "0u"; out << "0u";
} else if (auto* vec = type->As<sem::Vector>()) { } else if (auto* vec = type->As<sem::Vector>()) {
if (!EmitType(out, type, ast::StorageClass::kNone, if (!EmitType(out, type, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) { ast::AccessControl::kReadWrite, "")) {
return false; return false;
} }
ScopedParen sp(out); ScopedParen sp(out);
@ -2143,7 +2138,7 @@ bool GeneratorImpl::EmitZeroValue(std::ostream& out, const sem::Type* type) {
} }
} else if (auto* mat = type->As<sem::Matrix>()) { } else if (auto* mat = type->As<sem::Matrix>()) {
if (!EmitType(out, type, ast::StorageClass::kNone, if (!EmitType(out, type, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) { ast::AccessControl::kReadWrite, "")) {
return false; return false;
} }
ScopedParen sp(out); ScopedParen sp(out);
@ -2515,7 +2510,7 @@ bool GeneratorImpl::EmitStructType(std::ostream& out,
auto mem_name = builder_.Symbols().NameFor(mem->Declaration()->symbol()); auto mem_name = builder_.Symbols().NameFor(mem->Declaration()->symbol());
if (!EmitType(out, mem->Type(), ast::StorageClass::kNone, if (!EmitType(out, mem->Type(), ast::StorageClass::kNone,
ast::AccessControl::kInvalid, mem_name)) { ast::AccessControl::kReadWrite, mem_name)) {
return false; return false;
} }
// Array member name will be output with the type // Array member name will be output with the type

38
src/writer/hlsl/generator_impl_type_test.cc
View File

@ -38,7 +38,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Array) {
GeneratorImpl& gen = Build(); 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, "ary")) ast::AccessControl::kReadWrite, "ary"))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "bool ary[4]"); EXPECT_EQ(result(), "bool ary[4]");
} }
@ -50,7 +50,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_ArrayOfArray) {
GeneratorImpl& gen = Build(); 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, "ary")) ast::AccessControl::kReadWrite, "ary"))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "bool ary[5][4]"); EXPECT_EQ(result(), "bool ary[5][4]");
} }
@ -64,7 +64,7 @@ TEST_F(HlslGeneratorImplTest_Type,
GeneratorImpl& gen = Build(); 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, "ary")) ast::AccessControl::kReadWrite, "ary"))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "bool ary[5][4][1]"); EXPECT_EQ(result(), "bool ary[5][4][1]");
} }
@ -76,7 +76,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_ArrayOfArrayOfArray) {
GeneratorImpl& gen = Build(); 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, "ary")) ast::AccessControl::kReadWrite, "ary"))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "bool ary[6][5][4]"); EXPECT_EQ(result(), "bool ary[6][5][4]");
} }
@ -88,7 +88,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Array_WithoutName) {
GeneratorImpl& gen = Build(); 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, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "bool[4]"); EXPECT_EQ(result(), "bool[4]");
} }
@ -100,7 +100,7 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_RuntimeArray) {
GeneratorImpl& gen = Build(); 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, "ary")) ast::AccessControl::kReadWrite, "ary"))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "bool ary[]"); EXPECT_EQ(result(), "bool ary[]");
} }
@ -111,7 +111,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Bool) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, bool_, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, bool_, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "bool"); EXPECT_EQ(result(), "bool");
} }
@ -122,7 +122,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_F32) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, f32, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, f32, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "float"); EXPECT_EQ(result(), "float");
} }
@ -133,7 +133,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_I32) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, i32, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, i32, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "int"); EXPECT_EQ(result(), "int");
} }
@ -146,7 +146,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Matrix) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, mat2x3, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, mat2x3, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "float2x3"); EXPECT_EQ(result(), "float2x3");
} }
@ -159,7 +159,7 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Pointer) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, p, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, p, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "float*"); EXPECT_EQ(result(), "float*");
} }
@ -214,7 +214,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Struct) {
auto* sem_s = program->TypeOf(s)->As<sem::Struct>(); auto* sem_s = program->TypeOf(s)->As<sem::Struct>();
ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "S"); EXPECT_EQ(result(), "S");
} }
@ -234,7 +234,7 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Struct_InjectPadding) {
auto* sem_s = program->TypeOf(s)->As<sem::Struct>(); auto* sem_s = program->TypeOf(s)->As<sem::Struct>();
ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(gen.result(), R"(struct S { EXPECT_EQ(gen.result(), R"(struct S {
int a; int a;
@ -290,7 +290,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_U32) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, u32, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, u32, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "uint"); EXPECT_EQ(result(), "uint");
} }
@ -302,7 +302,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Vector) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, vec3, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, vec3, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "float3"); EXPECT_EQ(result(), "float3");
} }
@ -313,7 +313,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Void) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, void_, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, void_, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "void"); EXPECT_EQ(result(), "void");
} }
@ -324,7 +324,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitSampler) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "SamplerState"); EXPECT_EQ(result(), "SamplerState");
} }
@ -335,7 +335,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitSamplerComparison) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "SamplerComparisonState"); EXPECT_EQ(result(), "SamplerComparisonState");
} }
@ -532,7 +532,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitMultisampledTexture) {
GeneratorImpl& gen = Build(); GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, s, ast::StorageClass::kNone, ASSERT_TRUE(gen.EmitType(out, s, ast::StorageClass::kNone,
ast::AccessControl::kInvalid, "")) ast::AccessControl::kReadWrite, ""))
<< gen.error(); << gen.error();
EXPECT_EQ(result(), "Texture2DMS<float4>"); EXPECT_EQ(result(), "Texture2DMS<float4>");
} }

14
src/writer/spirv/builder.cc
View File

@ -784,12 +784,10 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
if (var->has_constructor()) { if (var->has_constructor()) {
ops.push_back(Operand::Int(init_id)); ops.push_back(Operand::Int(init_id));
} else if (sem->AccessControl() != ast::AccessControl::kInvalid) { } else {
if (type->Is<sem::StorageTexture>() || type->Is<sem::Struct>()) {
// type is a sem::Struct or a sem::StorageTexture // type is a sem::Struct or a sem::StorageTexture
switch (sem->AccessControl()) { switch (sem->AccessControl()) {
case ast::AccessControl::kInvalid:
TINT_ICE(builder_.Diagnostics()) << "missing access control";
break;
case ast::AccessControl::kWriteOnly: case ast::AccessControl::kWriteOnly:
push_annot( push_annot(
spv::Op::OpDecorate, spv::Op::OpDecorate,
@ -803,9 +801,10 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
case ast::AccessControl::kReadWrite: case ast::AccessControl::kReadWrite:
break; break;
} }
} else if (!type->Is<sem::Sampler>()) { }
// If we don't have a constructor and we're an Output or Private variable, if (!type->Is<sem::Sampler>()) {
// then WGSL requires that we zero-initialize. // 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 || if (sem->StorageClass() == ast::StorageClass::kPrivate ||
sem->StorageClass() == ast::StorageClass::kOutput) { sem->StorageClass() == ast::StorageClass::kOutput) {
init_id = GenerateConstantNullIfNeeded(type); init_id = GenerateConstantNullIfNeeded(type);
@ -815,6 +814,7 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
ops.push_back(Operand::Int(init_id)); ops.push_back(Operand::Int(init_id));
} }
} }
}
push_type(spv::Op::OpVariable, std::move(ops)); push_type(spv::Op::OpVariable, std::move(ops));