diff --git a/src/ast/intrinsic_texture_helper_test.cc b/src/ast/intrinsic_texture_helper_test.cc index 3ff61565af..8837ecd247 100644 --- a/src/ast/intrinsic_texture_helper_test.cc +++ b/src/ast/intrinsic_texture_helper_test.cc @@ -158,24 +158,23 @@ ast::Variable* TextureOverloadCase::buildTextureVariable( return b->Global("texture", b->ty.sampled_texture(texture_dimension, buildResultVectorComponentType(b)), - ast::StorageClass::kUniformConstant, nullptr, decos); + ast::StorageClass::kNone, nullptr, decos); case ast::intrinsic::test::TextureKind::kDepth: return b->Global("texture", b->ty.depth_texture(texture_dimension), - ast::StorageClass::kUniformConstant, nullptr, decos); + ast::StorageClass::kNone, nullptr, decos); case ast::intrinsic::test::TextureKind::kMultisampled: return b->Global( "texture", b->ty.multisampled_texture(texture_dimension, buildResultVectorComponentType(b)), - ast::StorageClass::kUniformConstant, nullptr, decos); + ast::StorageClass::kNone, nullptr, decos); case ast::intrinsic::test::TextureKind::kStorage: { auto st = b->ty.storage_texture(texture_dimension, image_format); auto ac = b->ty.access(access_control, st); - return b->Global("texture", ac, ast::StorageClass::kUniformConstant, - nullptr, decos); + return b->Global("texture", ac, ast::StorageClass::kNone, nullptr, decos); } } @@ -190,7 +189,7 @@ ast::Variable* TextureOverloadCase::buildSamplerVariable( b->create(1), }; return b->Global("sampler", b->ty.sampler(sampler_kind), - ast::StorageClass::kUniformConstant, nullptr, decos); + ast::StorageClass::kNone, nullptr, decos); } std::vector TextureOverloadCase::ValidCases() { diff --git a/src/ast/storage_class.cc b/src/ast/storage_class.cc index 13aa6963d0..62e29a4685 100644 --- a/src/ast/storage_class.cc +++ b/src/ast/storage_class.cc @@ -19,6 +19,10 @@ namespace ast { std::ostream& operator<<(std::ostream& out, StorageClass sc) { switch (sc) { + case StorageClass::kInvalid: { + out << "invalid"; + break; + } case StorageClass::kNone: { out << "none"; break; diff --git a/src/ast/storage_class.h b/src/ast/storage_class.h index a6e5983a7a..8a5cace355 100644 --- a/src/ast/storage_class.h +++ b/src/ast/storage_class.h @@ -22,7 +22,8 @@ namespace ast { /// Storage class of a given pointer. enum class StorageClass { - kNone = -1, + kInvalid = -1, + kNone, kInput, kOutput, kUniform, diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc index 0d12013824..e5589c8850 100644 --- a/src/inspector/inspector_test.cc +++ b/src/inspector/inspector_test.cc @@ -316,8 +316,7 @@ class InspectorHelper : public ProgramBuilder { /// @param group the binding/group to use for the storage buffer /// @param binding the binding number to use for the storage buffer void AddSampler(const std::string& name, uint32_t group, uint32_t binding) { - AddBinding(name, sampler_type(), ast::StorageClass::kUniformConstant, group, - binding); + AddBinding(name, sampler_type(), ast::StorageClass::kNone, group, binding); } /// Adds a comparison sampler variable to the program @@ -327,8 +326,8 @@ class InspectorHelper : public ProgramBuilder { void AddComparisonSampler(const std::string& name, uint32_t group, uint32_t binding) { - AddBinding(name, comparison_sampler_type(), - ast::StorageClass::kUniformConstant, group, binding); + AddBinding(name, comparison_sampler_type(), ast::StorageClass::kNone, group, + binding); } /// Generates a SampledTexture appropriate for the params @@ -366,7 +365,7 @@ class InspectorHelper : public ProgramBuilder { ast::Type* type, uint32_t group, uint32_t binding) { - AddBinding(name, type, ast::StorageClass::kUniformConstant, group, binding); + AddBinding(name, type, ast::StorageClass::kNone, group, binding); } /// Adds a multi-sampled texture variable to the program @@ -378,7 +377,7 @@ class InspectorHelper : public ProgramBuilder { ast::Type* type, uint32_t group, uint32_t binding) { - AddBinding(name, type, ast::StorageClass::kUniformConstant, group, binding); + AddBinding(name, type, ast::StorageClass::kNone, group, binding); } void AddGlobalVariable(const std::string& name, ast::Type* type) { @@ -394,7 +393,7 @@ class InspectorHelper : public ProgramBuilder { ast::Type* type, uint32_t group, uint32_t binding) { - AddBinding(name, type, ast::StorageClass::kUniformConstant, group, binding); + AddBinding(name, type, ast::StorageClass::kNone, group, binding); } /// Generates a function that references a specific sampler variable @@ -566,7 +565,7 @@ class InspectorHelper : public ProgramBuilder { typ::Type type, uint32_t group, uint32_t binding) { - AddBinding(name, type, ast::StorageClass::kUniformConstant, group, binding); + AddBinding(name, type, ast::StorageClass::kNone, group, binding); } /// Generates a function that references a storage texture variable. diff --git a/src/reader/spirv/enum_converter.cc b/src/reader/spirv/enum_converter.cc index 2a782485fd..cfcad1b9aa 100644 --- a/src/reader/spirv/enum_converter.cc +++ b/src/reader/spirv/enum_converter.cc @@ -49,7 +49,7 @@ ast::StorageClass EnumConverter::ToStorageClass(const SpvStorageClass sc) { case SpvStorageClassWorkgroup: return ast::StorageClass::kWorkgroup; case SpvStorageClassUniformConstant: - return ast::StorageClass::kUniformConstant; + return ast::StorageClass::kNone; case SpvStorageClassStorageBuffer: return ast::StorageClass::kStorage; case SpvStorageClassImage: @@ -63,7 +63,7 @@ ast::StorageClass EnumConverter::ToStorageClass(const SpvStorageClass sc) { } Fail() << "unknown SPIR-V storage class: " << uint32_t(sc); - return ast::StorageClass::kNone; + return ast::StorageClass::kInvalid; } ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b) { diff --git a/src/reader/spirv/enum_converter_test.cc b/src/reader/spirv/enum_converter_test.cc index 7cb09325b3..d19054ea97 100644 --- a/src/reader/spirv/enum_converter_test.cc +++ b/src/reader/spirv/enum_converter_test.cc @@ -143,7 +143,7 @@ INSTANTIATE_TEST_SUITE_P( StorageClassCase{SpvStorageClassWorkgroup, true, ast::StorageClass::kWorkgroup}, StorageClassCase{SpvStorageClassUniformConstant, true, - ast::StorageClass::kUniformConstant}, + ast::StorageClass::kNone}, StorageClassCase{SpvStorageClassStorageBuffer, true, ast::StorageClass::kStorage}, StorageClassCase{SpvStorageClassImage, true, ast::StorageClass::kImage}, @@ -156,7 +156,7 @@ INSTANTIATE_TEST_SUITE_P(EnumConverterBad, SpvStorageClassTest, testing::Values(StorageClassCase{ static_cast(9999), false, - ast::StorageClass::kNone})); + ast::StorageClass::kInvalid})); // Builtin diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index e9c15c9130..1428e7aa0b 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -4005,11 +4005,13 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() { << "pointer defined in function from unknown opcode: " << inst.PrettyPrint(); } - if (info->storage_class == ast::StorageClass::kUniformConstant) { - info->skip = SkipReason::kOpaqueObject; - } } - if (type->AsSampler() || type->AsImage() || type->AsSampledImage()) { + auto* unwrapped = type; + while (auto* ptr = unwrapped->AsPointer()) { + unwrapped = ptr->pointee_type(); + } + if (unwrapped->AsSampler() || unwrapped->AsImage() || + unwrapped->AsSampledImage()) { // Defer code generation until the instruction that actually acts on // the image. info->skip = SkipReason::kOpaqueObject; @@ -4032,7 +4034,7 @@ ast::StorageClass FunctionEmitter::GetStorageClassForPointerValue(uint32_t id) { return ptr->storage_class; } } - return ast::StorageClass::kNone; + return ast::StorageClass::kInvalid; } const Type* FunctionEmitter::RemapStorageClass(const Type* type, diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 06878cf310..b1392a0d6e 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -1060,7 +1060,7 @@ const Type* ParserImpl::ConvertType(uint32_t type_id, } auto ast_storage_class = enum_converter_.ToStorageClass(storage_class); - if (ast_storage_class == ast::StorageClass::kNone) { + if (ast_storage_class == ast::StorageClass::kInvalid) { Fail() << "SPIR-V pointer type with ID " << type_id << " has invalid storage class " << static_cast(storage_class); @@ -1236,6 +1236,7 @@ bool ParserImpl::EmitModuleScopeVariables() { continue; } switch (enum_converter_.ToStorageClass(spirv_storage_class)) { + case ast::StorageClass::kNone: case ast::StorageClass::kInput: case ast::StorageClass::kOutput: case ast::StorageClass::kUniform: diff --git a/src/reader/spirv/parser_impl_convert_type_test.cc b/src/reader/spirv/parser_impl_convert_type_test.cc index 3b32374947..ea46b0ffb5 100644 --- a/src/reader/spirv/parser_impl_convert_type_test.cc +++ b/src/reader/spirv/parser_impl_convert_type_test.cc @@ -692,7 +692,7 @@ TEST_F(SpvParserTest, ConvertType_PointerUniformConstant) { auto* ptr_ty = type->As(); EXPECT_NE(ptr_ty, nullptr); EXPECT_TRUE(ptr_ty->type->Is()); - EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kUniformConstant); + EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kNone); EXPECT_TRUE(p->error().empty()); } diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 3ea562a3a5..9aba063c93 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -564,17 +564,6 @@ Maybe ParserImpl::variable_decl() { if (decl.errored) return Failure::kErrored; - if (decl->type && decl->type->UnwrapAll()->is_handle()) { - // handle types implicitly have the `UniformConstant` storage class. - if (explicit_sc.matched) { - return add_error( - explicit_sc.source, - decl->type->UnwrapAll()->FriendlyName(builder_.Symbols()) + - " variables must not have a storage class"); - } - sc = ast::StorageClass::kUniformConstant; - } - return VarDeclInfo{decl->source, decl->name, sc, decl->type}; } diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc index b56f6fe8ee..c9c7c833d0 100644 --- a/src/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/reader/wgsl/parser_impl_error_msg_test.cc @@ -1102,22 +1102,6 @@ TEST_F(ParserImplErrorTest, GlobalDeclVarVectorMissingType) { " ^\n"); } -TEST_F(ParserImplErrorTest, GlobalDeclSamplerExplicitStorageClass) { - EXPECT( - "var x : sampler;", - "test.wgsl:1:5 error: sampler variables must not have a storage class\n" - "var x : sampler;\n" - " ^^^^^^^\n"); -} - -TEST_F(ParserImplErrorTest, GlobalDeclTextureExplicitStorageClass) { - EXPECT("var x : [[access(read)]] texture_1d;", - "test.wgsl:1:5 error: texture_1d variables must not have a " - "storage class\n" - "var x : [[access(read)]] texture_1d;\n" - " ^^^^^^^\n"); -} - TEST_F(ParserImplErrorTest, IfStmtMissingLParen) { EXPECT("fn f() { if true) {} }", "test.wgsl:1:13 error: expected '('\n" diff --git a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc index cb469a7bec..7887b02e2c 100644 --- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc +++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc @@ -168,38 +168,6 @@ TEST_F(ParserImplTest, GlobalVariableDecl_InvalidVariableDecl) { EXPECT_EQ(p->error(), "1:5: invalid storage class for variable decoration"); } -TEST_F(ParserImplTest, GlobalVariableDecl_SamplerImplicitStorageClass) { - auto p = parser("var s : sampler;"); - auto decos = p->decoration_list(); - EXPECT_FALSE(decos.errored); - EXPECT_FALSE(decos.matched); - auto e = p->global_variable_decl(decos.value); - ASSERT_FALSE(p->has_error()) << p->error(); - EXPECT_FALSE(e.errored); - EXPECT_TRUE(e.matched); - ASSERT_NE(e.value, nullptr); - - EXPECT_EQ(e->symbol(), p->builder().Symbols().Get("s")); - EXPECT_TRUE(e->type()->Is()); - EXPECT_EQ(e->declared_storage_class(), ast::StorageClass::kUniformConstant); -} - -TEST_F(ParserImplTest, GlobalVariableDecl_TextureImplicitStorageClass) { - auto p = parser("var s : [[access(read)]] texture_1d;"); - auto decos = p->decoration_list(); - EXPECT_FALSE(decos.errored); - EXPECT_FALSE(decos.matched); - auto e = p->global_variable_decl(decos.value); - ASSERT_FALSE(p->has_error()) << p->error(); - EXPECT_FALSE(e.errored); - EXPECT_TRUE(e.matched); - ASSERT_NE(e.value, nullptr); - - EXPECT_EQ(e->symbol(), p->builder().Symbols().Get("s")); - EXPECT_TRUE(e->type()->UnwrapAll()->Is()); - EXPECT_EQ(e->declared_storage_class(), ast::StorageClass::kUniformConstant); -} - TEST_F(ParserImplTest, GlobalVariableDecl_StorageClassIn_Deprecated) { auto p = parser("[[location(0)]] var a : f32"); auto f = p->function_header(); diff --git a/src/resolver/assignment_validation_test.cc b/src/resolver/assignment_validation_test.cc index 52782a1ff3..f469a06580 100644 --- a/src/resolver/assignment_validation_test.cc +++ b/src/resolver/assignment_validation_test.cc @@ -244,19 +244,15 @@ TEST_F(ResolverAssignmentValidationTest, AssignFromPointer_Fail) { return ty.access(ast::AccessControl::kReadOnly, tex_type); }; - auto* var_a = Var("a", make_type(), ast::StorageClass::kFunction); - auto* var_b = Var("b", make_type(), ast::StorageClass::kFunction); + auto* var_a = Global("a", make_type(), ast::StorageClass::kNone); + auto* var_b = Global("b", make_type(), ast::StorageClass::kNone); - auto* lhs = Expr("a"); - auto* rhs = Expr("b"); - - auto* assign = Assign(Source{{12, 34}}, lhs, rhs); - WrapInFunction(Decl(var_a), Decl(var_b), assign); + WrapInFunction(Assign(Source{{12, 34}}, var_a, var_b)); EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), "12:34 error v-000x: invalid assignment: right-hand-side is not " - "storable: ptr>"); } diff --git a/src/resolver/intrinsic_test.cc b/src/resolver/intrinsic_test.cc index 876fa1d4d1..58811f6fbe 100644 --- a/src/resolver/intrinsic_test.cc +++ b/src/resolver/intrinsic_test.cc @@ -243,7 +243,10 @@ class ResolverIntrinsicTest_TextureOperation void add_call_param(std::string name, typ::Type type, ast::ExpressionList* call_params) { - Global(name, type, ast::StorageClass::kInput); + ast::StorageClass storage_class = type->UnwrapAll()->is_handle() + ? ast::StorageClass::kNone + : ast::StorageClass::kPrivate; + Global(name, type, storage_class); call_params->push_back(Expr(name)); } typ::Type subtype(Texture type) { diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 3e863c6167..8f4007626b 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -500,7 +500,7 @@ bool Resolver::GlobalVariable(ast::Variable* var) { return false; } - if (!ApplyStorageClassUsageToType(var->declared_storage_class(), info->type, + if (!ApplyStorageClassUsageToType(info->storage_class, info->type, var->source())) { diagnostics_.add_note("while instantiating variable " + builder_->Symbols().NameFor(var->symbol()), @@ -577,11 +577,12 @@ bool Resolver::ValidateGlobalVariable(const VariableInfo* info) { break; } - return ValidateVariable(info->declaration); + return ValidateVariable(info); } -bool Resolver::ValidateVariable(const ast::Variable* var) { - auto* type = variable_to_info_[var]->type; +bool Resolver::ValidateVariable(const VariableInfo* info) { + auto* var = info->declaration; + auto* type = info->type; if (auto* r = type->As()) { if (r->IsRuntimeSized()) { diagnostics_.add_error( @@ -643,11 +644,23 @@ bool Resolver::ValidateVariable(const ast::Variable* var) { } } + if (type->UnwrapAll()->is_handle() && + var->declared_storage_class() != ast::StorageClass::kNone) { + // https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables + // If the store type is a texture type or a sampler type, then the variable + // declaration must not have a storage class decoration. The storage class + // will always be handle. + diagnostics_.add_error("variables of type '" + info->type_name + + "' must not have a storage class", + var->source()); + return false; + } + return true; } -bool Resolver::ValidateParameter(const ast::Variable* param) { - return ValidateVariable(param); +bool Resolver::ValidateParameter(const VariableInfo* info) { + return ValidateVariable(info); } bool Resolver::ValidateFunction(const ast::Function* func, @@ -662,7 +675,7 @@ bool Resolver::ValidateFunction(const ast::Function* func, } for (auto* param : func->params()) { - if (!ValidateParameter(param)) { + if (!ValidateParameter(variable_to_info_.at(param))) { return false; } } @@ -2060,7 +2073,7 @@ bool Resolver::VariableDeclStatement(const ast::VariableDeclStatement* stmt) { variable_stack_.set(var->symbol(), info); current_block_->decls.push_back(var); - if (!ValidateVariable(var)) { + if (!ValidateVariable(info)) { return false; } @@ -2833,7 +2846,16 @@ Resolver::VariableInfo::VariableInfo(const ast::Variable* decl, : declaration(decl), type(ctype), type_name(tn), - storage_class(decl->declared_storage_class()) {} + storage_class(decl->declared_storage_class()) { + if (storage_class == ast::StorageClass::kNone && + type->UnwrapAll()->is_handle()) { + // https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables + // If the store type is a texture type or a sampler type, then the variable + // declaration must not have a storage class decoration. The storage class + // will always be handle. + storage_class = ast::StorageClass::kUniformConstant; + } +} Resolver::VariableInfo::~VariableInfo() = default; diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index 378927bc79..c537eae1a5 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h @@ -230,11 +230,11 @@ class Resolver { bool ValidateGlobalVariable(const VariableInfo* var); bool ValidateMatrixConstructor(const ast::TypeConstructorExpression* ctor, const sem::Matrix* matrix_type); - bool ValidateParameter(const ast::Variable* param); + bool ValidateParameter(const VariableInfo* info); bool ValidateReturn(const ast::ReturnStatement* ret); bool ValidateStructure(const sem::Struct* str); bool ValidateSwitch(const ast::SwitchStatement* s); - bool ValidateVariable(const ast::Variable* param); + bool ValidateVariable(const VariableInfo* info); bool ValidateVectorConstructor(const ast::TypeConstructorExpression* ctor, const sem::Vector* vec_type); diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc index fe8d221946..323434812f 100644 --- a/src/resolver/resolver_test.cc +++ b/src/resolver/resolver_test.cc @@ -1458,6 +1458,27 @@ TEST_F(ResolverTest, StorageClass_SetsIfMissing) { EXPECT_EQ(Sem().Get(var)->StorageClass(), ast::StorageClass::kFunction); } +TEST_F(ResolverTest, StorageClass_SetForSampler) { + auto t = ty.sampler(ast::SamplerKind::kSampler); + auto* var = Global("var", t, ast::StorageClass::kNone); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); + + EXPECT_EQ(Sem().Get(var)->StorageClass(), + ast::StorageClass::kUniformConstant); +} + +TEST_F(ResolverTest, StorageClass_SetForTexture) { + auto t = ty.sampled_texture(ast::TextureDimension::k1d, ty.f32()); + auto ac = ty.access(ast::AccessControl::Access::kReadOnly, t); + auto* var = Global("var", ac, ast::StorageClass::kNone); + + EXPECT_TRUE(r()->Resolve()) << r()->error(); + + EXPECT_EQ(Sem().Get(var)->StorageClass(), + ast::StorageClass::kUniformConstant); +} + TEST_F(ResolverTest, StorageClass_DoesNotSetOnConst) { auto* var = Const("var", ty.i32(), Construct(ty.i32())); auto* stmt = Decl(var); diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc index b5dfcc1ea7..f67192504b 100644 --- a/src/resolver/type_validation_test.cc +++ b/src/resolver/type_validation_test.cc @@ -551,7 +551,7 @@ using MultisampledTextureDimensionTest = ResolverTestWithParam; TEST_P(MultisampledTextureDimensionTest, All) { auto& params = GetParam(); Global("a", ty.multisampled_texture(params.dim, ty.i32()), - ast::StorageClass::kUniformConstant, nullptr, + ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -595,7 +595,7 @@ TEST_P(MultisampledTextureTypeTest, All) { Global( "a", ty.multisampled_texture(ast::TextureDimension::k2d, params.type_func(ty)), - ast::StorageClass::kUniformConstant, nullptr, + ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -636,7 +636,7 @@ TEST_P(StorageTextureDimensionTest, All) { auto st = ty.storage_texture(params.dim, ast::ImageFormat::kR32Uint); auto ac = ty.access(ast::AccessControl::kReadOnly, st); - Global("a", ac, ast::StorageClass::kUniformConstant, nullptr, + Global("a", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -708,7 +708,7 @@ TEST_P(StorageTextureFormatTest, All) { 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::kUniformConstant, nullptr, + Global("a", ac_a, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -716,7 +716,7 @@ TEST_P(StorageTextureFormatTest, All) { auto st_b = ty.storage_texture(ast::TextureDimension::k2d, params.format); auto ac_b = ty.access(ast::AccessControl::kReadOnly, st_b); - Global("b", ac_b, ast::StorageClass::kUniformConstant, nullptr, + Global("b", ac_b, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(1), @@ -725,7 +725,7 @@ TEST_P(StorageTextureFormatTest, All) { auto st_c = ty.storage_texture(ast::TextureDimension::k2dArray, params.format); auto ac_c = ty.access(ast::AccessControl::kReadOnly, st_c); - Global("c", ac_c, ast::StorageClass::kUniformConstant, nullptr, + Global("c", ac_c, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(2), @@ -733,7 +733,7 @@ TEST_P(StorageTextureFormatTest, All) { auto st_d = ty.storage_texture(ast::TextureDimension::k3d, params.format); auto ac_d = ty.access(ast::AccessControl::kReadOnly, st_d); - Global("d", ac_d, ast::StorageClass::kUniformConstant, nullptr, + Global("d", ac_d, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(3), @@ -758,7 +758,7 @@ TEST_F(StorageTextureAccessControlTest, MissingAccessControl_Fail) { auto st = ty.storage_texture(ast::TextureDimension::k1d, ast::ImageFormat::kR32Uint); - Global("a", st, ast::StorageClass::kUniformConstant, nullptr, + Global("a", st, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -775,7 +775,7 @@ TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) { ast::ImageFormat::kR32Uint); auto ac = ty.access(ast::AccessControl::kReadWrite, st); - Global("a", ac, ast::StorageClass::kUniformConstant, nullptr, + Global("a", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -792,7 +792,7 @@ TEST_F(StorageTextureAccessControlTest, ReadOnlyAccessControl_Pass) { ast::ImageFormat::kR32Uint); auto ac = ty.access(ast::AccessControl::kReadOnly, st); - Global("a", ac, ast::StorageClass::kUniformConstant, nullptr, + Global("a", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), @@ -809,7 +809,7 @@ TEST_F(StorageTextureAccessControlTest, WriteOnlyAccessControl_Pass) { ast::ImageFormat::kR32Uint); auto ac = ty.access(ast::AccessControl::kWriteOnly, st); - Global("a", ac, ast::StorageClass::kUniformConstant, nullptr, + Global("a", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(0), create(0), diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc index 7889f8d588..ba3dde3d3c 100644 --- a/src/resolver/validation_test.cc +++ b/src/resolver/validation_test.cc @@ -375,6 +375,28 @@ TEST_F(ResolverValidationTest, StorageClass_NonFunctionClassError) { "error: function variable has a non-function storage class"); } +TEST_F(ResolverValidationTest, StorageClass_SamplerExplicitStorageClass) { + auto t = ty.sampler(ast::SamplerKind::kSampler); + Global(Source{{12, 34}}, "var", t, ast::StorageClass::kUniformConstant); + + EXPECT_FALSE(r()->Resolve()); + + EXPECT_EQ( + r()->error(), + R"(12:34 error: variables of type 'sampler' must not have a storage class)"); +} + +TEST_F(ResolverValidationTest, StorageClass_TextureExplicitStorageClass) { + auto t = ty.sampled_texture(ast::TextureDimension::k1d, ty.f32()); + Global(Source{{12, 34}}, "var", t, ast::StorageClass::kUniformConstant); + + EXPECT_FALSE(r()->Resolve()) << r()->error(); + + EXPECT_EQ( + r()->error(), + R"(12:34 error: variables of type 'texture_1d' must not have a storage class)"); +} + TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_BadChar) { Global("my_vec", ty.vec3(), ast::StorageClass::kInput); diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc index 0905be086a..6354625bd6 100644 --- a/src/writer/hlsl/generator_impl_type_test.cc +++ b/src/writer/hlsl/generator_impl_type_test.cc @@ -335,7 +335,7 @@ TEST_P(HlslDepthTexturesTest, Emit) { auto t = ty.depth_texture(params.dim); - Global("tex", t, ast::StorageClass::kUniformConstant, nullptr, + Global("tex", t, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(1), create(2), @@ -394,7 +394,7 @@ TEST_P(HlslSampledTexturesTest, Emit) { } auto t = ty.sampled_texture(params.dim, datatype); - Global("tex", t, ast::StorageClass::kUniformConstant, nullptr, + Global("tex", t, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(1), create(2), @@ -536,7 +536,7 @@ TEST_P(HlslStorageTexturesTest, Emit) { : ast::AccessControl::kWriteOnly, t); - Global("tex", ac, ast::StorageClass::kUniformConstant, nullptr, + Global("tex", ac, ast::StorageClass::kNone, nullptr, ast::DecorationList{ create(1), create(2), diff --git a/src/writer/msl/generator_impl_type_test.cc b/src/writer/msl/generator_impl_type_test.cc index a726ff7b76..80c64c4e77 100644 --- a/src/writer/msl/generator_impl_type_test.cc +++ b/src/writer/msl/generator_impl_type_test.cc @@ -745,7 +745,7 @@ TEST_P(MslStorageTexturesTest, Emit) { auto ac = ty.access(params.ro ? ast::AccessControl::kReadOnly : ast::AccessControl::kWriteOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); GeneratorImpl& gen = Build(); diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 570d131f51..ee09323077 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -769,8 +769,8 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { if (auto* builtin = deco->As()) { push_annot(spv::Op::OpDecorate, {Operand::Int(var_id), Operand::Int(SpvDecorationBuiltIn), - Operand::Int(ConvertBuiltin(builtin->value(), - var->declared_storage_class()))}); + Operand::Int( + ConvertBuiltin(builtin->value(), sem->StorageClass()))}); } else if (auto* location = deco->As()) { push_annot(spv::Op::OpDecorate, {Operand::Int(var_id), Operand::Int(SpvDecorationLocation), @@ -3243,6 +3243,8 @@ bool Builder::GenerateVectorType(const sem::Vector* vec, SpvStorageClass Builder::ConvertStorageClass(ast::StorageClass klass) const { switch (klass) { + case ast::StorageClass::kInvalid: + return SpvStorageClassMax; case ast::StorageClass::kInput: return SpvStorageClassInput; case ast::StorageClass::kOutput: diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc index dadc9b40be..4c28e9ffdf 100644 --- a/src/writer/spirv/builder_global_variable_test.cc +++ b/src/writer/spirv/builder_global_variable_test.cc @@ -526,7 +526,7 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageReadOnly) { auto ac = ty.access(ast::AccessControl::kReadOnly, type); - auto* var_a = Global("a", ac, ast::StorageClass::kUniformConstant); + auto* var_a = Global("a", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -549,7 +549,7 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageWriteOnly) { auto ac = ty.access(ast::AccessControl::kWriteOnly, type); - auto* var_a = Global("a", ac, ast::StorageClass::kUniformConstant); + auto* var_a = Global("a", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -573,12 +573,12 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageWithDifferentAccess) { auto type_a = ty.access(ast::AccessControl::kReadOnly, ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Uint)); - auto* var_a = Global("a", type_a, ast::StorageClass::kUniformConstant); + auto* var_a = Global("a", type_a, ast::StorageClass::kNone); auto type_b = ty.access(ast::AccessControl::kWriteOnly, ty.storage_texture(ast::TextureDimension::k2d, ast::ImageFormat::kR32Uint)); - auto* var_b = Global("b", type_b, ast::StorageClass::kUniformConstant); + auto* var_b = Global("b", type_b, ast::StorageClass::kNone); spirv::Builder& b = Build(); diff --git a/src/writer/spirv/builder_intrinsic_test.cc b/src/writer/spirv/builder_intrinsic_test.cc index 911d2430e7..9e353c1765 100644 --- a/src/writer/spirv/builder_intrinsic_test.cc +++ b/src/writer/spirv/builder_intrinsic_test.cc @@ -373,9 +373,9 @@ TEST_F(IntrinsicBuilderTest, Call_TextureSampleCompare_Twice) { auto s = ty.sampler(ast::SamplerKind::kComparisonSampler); auto t = ty.depth_texture(ast::TextureDimension::k2d); - auto* tex = Global("texture", t, ast::StorageClass::kInput); + auto* tex = Global("texture", t, ast::StorageClass::kNone); - auto* sampler = Global("sampler", s, ast::StorageClass::kInput); + auto* sampler = Global("sampler", s, ast::StorageClass::kNone); auto* expr1 = Call("textureSampleCompare", "texture", "sampler", vec2(1.0f, 2.0f), 2.0f); @@ -397,11 +397,11 @@ TEST_F(IntrinsicBuilderTest, Call_TextureSampleCompare_Twice) { EXPECT_EQ(DumpInstructions(b.types()), R"(%4 = OpTypeFloat 32 %3 = OpTypeImage %4 2D 1 0 0 1 Unknown -%2 = OpTypePointer Input %3 -%1 = OpVariable %2 Input +%2 = OpTypePointer UniformConstant %3 +%1 = OpVariable %2 UniformConstant %7 = OpTypeSampler -%6 = OpTypePointer Input %7 -%5 = OpVariable %6 Input +%6 = OpTypePointer UniformConstant %7 +%5 = OpVariable %6 UniformConstant %11 = OpTypeSampledImage %3 %13 = OpTypeVector %4 2 %14 = OpConstant %4 1 diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc index bf158d9b4e..477007b897 100644 --- a/src/writer/spirv/builder_type_test.cc +++ b/src/writer/spirv/builder_type_test.cc @@ -816,7 +816,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_1d) { ast::ImageFormat::kR32Float); auto ac = ty.access(ast::AccessControl::kReadOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -832,7 +832,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_2d) { ast::ImageFormat::kR32Float); auto ac = ty.access(ast::AccessControl::kReadOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -848,7 +848,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_2dArray) { ast::ImageFormat::kR32Float); auto ac = ty.access(ast::AccessControl::kReadOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -864,7 +864,7 @@ TEST_F(BuilderTest_Type, StorageTexture_Generate_3d) { ast::ImageFormat::kR32Float); auto ac = ty.access(ast::AccessControl::kReadOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -881,7 +881,7 @@ TEST_F(BuilderTest_Type, ast::ImageFormat::kR32Float); auto ac = ty.access(ast::AccessControl::kReadOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -898,7 +898,7 @@ TEST_F(BuilderTest_Type, ast::ImageFormat::kR32Sint); auto ac = ty.access(ast::AccessControl::kReadOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); @@ -915,7 +915,7 @@ TEST_F(BuilderTest_Type, ast::ImageFormat::kR32Uint); auto ac = ty.access(ast::AccessControl::kReadOnly, s); - Global("test_var", ac, ast::StorageClass::kInput); + Global("test_var", ac, ast::StorageClass::kNone); spirv::Builder& b = Build(); diff --git a/src/writer/wgsl/generator_impl_global_decl_test.cc b/src/writer/wgsl/generator_impl_global_decl_test.cc index 491cad1655..a77acee31f 100644 --- a/src/writer/wgsl/generator_impl_global_decl_test.cc +++ b/src/writer/wgsl/generator_impl_global_decl_test.cc @@ -101,8 +101,7 @@ TEST_F(WgslGeneratorImplTest, Emit_GlobalsInterleaved) { } TEST_F(WgslGeneratorImplTest, Emit_Global_Sampler) { - Global("s", ty.sampler(ast::SamplerKind::kSampler), - ast::StorageClass::kUniformConstant); + Global("s", ty.sampler(ast::SamplerKind::kSampler), ast::StorageClass::kNone); GeneratorImpl& gen = Build(); @@ -115,7 +114,7 @@ TEST_F(WgslGeneratorImplTest, Emit_Global_Sampler) { TEST_F(WgslGeneratorImplTest, Emit_Global_Texture) { auto st = ty.sampled_texture(ast::TextureDimension::k1d, ty.f32()); Global("t", ty.access(ast::AccessControl::kReadOnly, st), - ast::StorageClass::kUniformConstant); + ast::StorageClass::kNone); GeneratorImpl& gen = Build();