From 15e7f94b762e86dec964bd040230271c39134df3 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Fri, 2 Dec 2022 02:59:44 +0000 Subject: [PATCH] Remove ArrayCount helpers. This CL removes the helpers in sem::Array to determine the type of ArrayCount. Instead the `Is` and `As` functions from Castable are used at the call sites. Bug: tint:1718 Change-Id: Ie666bfbfca6bb1be8ead613266a7221d88f7a76d Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/112442 Kokoro: Kokoro Commit-Queue: Dan Sinclair Reviewed-by: Ben Clayton --- src/tint/resolver/intrinsic_table.cc | 2 +- src/tint/resolver/resolver.cc | 2 +- src/tint/resolver/validator.cc | 15 ++++++++------- src/tint/sem/array.h | 15 --------------- src/tint/sem/array_test.cc | 4 ++-- .../module_scope_var_to_entry_point_param.cc | 3 ++- src/tint/transform/pad_structs.cc | 2 +- src/tint/transform/robustness.cc | 2 +- src/tint/transform/spirv_atomic.cc | 2 +- src/tint/transform/transform.cc | 2 +- src/tint/writer/glsl/generator_impl.cc | 4 ++-- src/tint/writer/hlsl/generator_impl.cc | 2 +- src/tint/writer/msl/generator_impl.cc | 4 ++-- src/tint/writer/spirv/builder.cc | 2 +- 14 files changed, 24 insertions(+), 37 deletions(-) diff --git a/src/tint/resolver/intrinsic_table.cc b/src/tint/resolver/intrinsic_table.cc index aed49c3566..ebd5043a44 100644 --- a/src/tint/resolver/intrinsic_table.cc +++ b/src/tint/resolver/intrinsic_table.cc @@ -523,7 +523,7 @@ bool match_array(MatchState&, const sem::Type* ty, const sem::Type*& T) { } if (auto* a = ty->As()) { - if (a->IsRuntimeSized()) { + if (a->Count()->Is()) { T = a->ElemType(); return true; } diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index f909bf0394..ca66ac2b3d 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -3639,7 +3639,7 @@ bool Resolver::ApplyAddressSpaceUsageToType(ast::AddressSpace address_space, if (auto* arr = ty->As()) { if (address_space != ast::AddressSpace::kStorage) { - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { AddError("runtime-sized arrays can only be used in the address space", usage); return false; diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index 5a7f9d88e6..e5c7c1a6bb 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc @@ -194,7 +194,7 @@ bool Validator::IsFixedFootprint(const sem::Type* type) const { [&](const sem::Matrix*) { return true; }, // [&](const sem::Atomic*) { return true; }, [&](const sem::Array* arr) { - return !arr->IsRuntimeSized() && IsFixedFootprint(arr->ElemType()); + return !arr->Count()->Is() && IsFixedFootprint(arr->ElemType()); }, [&](const sem::Struct* str) { for (auto* member : str->Members()) { @@ -1763,12 +1763,13 @@ bool Validator::ArrayInitializer(const ast::CallExpression* ctor, } } - if (array_type->IsRuntimeSized()) { + auto* c = array_type->Count(); + if (c->Is()) { AddError("cannot construct a runtime-sized array", ctor->source); return false; } - if (array_type->IsOverrideSized()) { + if (c->IsAnyOf()) { AddError("cannot construct an array that has an override-expression count", ctor->source); return false; } @@ -1778,12 +1779,12 @@ bool Validator::ArrayInitializer(const ast::CallExpression* ctor, return false; } - if (!array_type->IsConstantSized()) { + if (!c->Is()) { TINT_ICE(Resolver, diagnostics_) << "Invalid ArrayCount found"; return false; } - const auto count = array_type->Count()->As()->value; + const auto count = c->As()->value; if (!values.IsEmpty() && (values.Length() != count)) { std::string fm = values.Length() < count ? "few" : "many"; AddError("array initializer has too " + fm + " elements: expected " + @@ -2026,7 +2027,7 @@ bool Validator::Structure(const sem::Struct* str, ast::PipelineStage stage) cons utils::Hashset locations; for (auto* member : str->Members()) { if (auto* r = member->Type()->As()) { - if (r->IsRuntimeSized()) { + if (r->Count()->Is()) { if (member != str->Members().back()) { AddError("runtime arrays may only appear as the last member of a struct", member->Source()); @@ -2398,7 +2399,7 @@ bool Validator::IsValidationEnabled(utils::VectorRef attr bool Validator::IsArrayWithOverrideCount(const sem::Type* ty) const { if (auto* arr = ty->UnwrapRef()->As()) { - if (arr->IsOverrideSized()) { + if (arr->Count()->IsAnyOf()) { return true; } } diff --git a/src/tint/sem/array.h b/src/tint/sem/array.h index c068948499..7a484e5094 100644 --- a/src/tint/sem/array.h +++ b/src/tint/sem/array.h @@ -103,21 +103,6 @@ class Array final : public Castable { /// natural stride bool IsStrideImplicit() const { return stride_ == implicit_stride_; } - /// @returns true if this array is sized using an const-expression - bool IsConstantSized() const { return count_->Is(); } - - /// @returns true if this array is sized using a named override variable - bool IsNamedOverrideSized() const { return count_->Is(); } - - /// @returns true if this array is sized using an unnamed override variable - bool IsUnnamedOverrideSized() const { return count_->Is(); } - - /// @returns true if this array is sized using a named or unnamed override variable - bool IsOverrideSized() const { return IsNamedOverrideSized() || IsUnnamedOverrideSized(); } - - /// @returns true if this array is runtime sized - bool IsRuntimeSized() const { return count_->Is(); } - /// @param symbols the program's symbol table /// @returns the name for this type that closely resembles how it would be /// declared in WGSL. diff --git a/src/tint/sem/array_test.cc b/src/tint/sem/array_test.cc index 44073df3d6..a2bc3d3ba2 100644 --- a/src/tint/sem/array_test.cc +++ b/src/tint/sem/array_test.cc @@ -36,7 +36,7 @@ TEST_F(ArrayTest, CreateSizedArray) { EXPECT_EQ(a->Stride(), 32u); EXPECT_EQ(a->ImplicitStride(), 16u); EXPECT_FALSE(a->IsStrideImplicit()); - EXPECT_FALSE(a->IsRuntimeSized()); + EXPECT_FALSE(a->Count()->Is()); EXPECT_EQ(a, b); EXPECT_NE(a, c); @@ -61,7 +61,7 @@ TEST_F(ArrayTest, CreateRuntimeArray) { EXPECT_EQ(a->Stride(), 32u); EXPECT_EQ(a->ImplicitStride(), 32u); EXPECT_TRUE(a->IsStrideImplicit()); - EXPECT_TRUE(a->IsRuntimeSized()); + EXPECT_TRUE(a->Count()->Is()); EXPECT_EQ(a, b); EXPECT_NE(a, c); diff --git a/src/tint/transform/module_scope_var_to_entry_point_param.cc b/src/tint/transform/module_scope_var_to_entry_point_param.cc index 9c9c42fb3f..9156205be2 100644 --- a/src/tint/transform/module_scope_var_to_entry_point_param.cc +++ b/src/tint/transform/module_scope_var_to_entry_point_param.cc @@ -146,7 +146,8 @@ struct ModuleScopeVarToEntryPointParam::State { attributes.Push(ctx.dst->Disable(ast::DisabledValidation::kIgnoreAddressSpace)); auto* param_type = store_type(); - if (auto* arr = ty->As(); arr && arr->IsRuntimeSized()) { + if (auto* arr = ty->As(); + arr && arr->Count()->Is()) { // Wrap runtime-sized arrays in structures, so that we can declare pointers to // them. Ideally we'd just emit the array itself as a pointer, but this is not // representable in Tint's AST. diff --git a/src/tint/transform/pad_structs.cc b/src/tint/transform/pad_structs.cc index 4ceb39dc82..e8c21a7585 100644 --- a/src/tint/transform/pad_structs.cc +++ b/src/tint/transform/pad_structs.cc @@ -84,7 +84,7 @@ Transform::ApplyResult PadStructs::Apply(const Program* src, const DataMap&, Dat // std140 structs should be padded out to 16 bytes. size = utils::RoundUp(16u, size); } else if (auto* array_ty = ty->As()) { - if (array_ty->IsRuntimeSized()) { + if (array_ty->Count()->Is()) { has_runtime_sized_array = true; } } diff --git a/src/tint/transform/robustness.cc b/src/tint/transform/robustness.cc index 75d63f25b5..c9f62923e5 100644 --- a/src/tint/transform/robustness.cc +++ b/src/tint/transform/robustness.cc @@ -106,7 +106,7 @@ struct Robustness::State { }, [&](const sem::Array* arr) -> const ast::Expression* { const ast::Expression* max = nullptr; - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { // Size is unknown until runtime. // Must clamp, even if the index is constant. auto* arr_ptr = b.AddressOf(ctx.Clone(expr->object)); diff --git a/src/tint/transform/spirv_atomic.cc b/src/tint/transform/spirv_atomic.cc index 319975ae44..de3cdab0d3 100644 --- a/src/tint/transform/spirv_atomic.cc +++ b/src/tint/transform/spirv_atomic.cc @@ -202,7 +202,7 @@ struct SpirvAtomic::State { [&](const sem::U32*) { return b.ty.atomic(CreateASTTypeFor(ctx, ty)); }, [&](const sem::Struct* str) { return b.ty.type_name(Fork(str->Declaration()).name); }, [&](const sem::Array* arr) -> const ast::Type* { - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { return b.ty.array(AtomicTypeFor(arr->ElemType())); } auto count = arr->ConstantCount(); diff --git a/src/tint/transform/transform.cc b/src/tint/transform/transform.cc index d2eea428d4..cbfb90a83f 100644 --- a/src/tint/transform/transform.cc +++ b/src/tint/transform/transform.cc @@ -106,7 +106,7 @@ const ast::Type* Transform::CreateASTTypeFor(CloneContext& ctx, const sem::Type* if (!a->IsStrideImplicit()) { attrs.Push(ctx.dst->create(a->Stride())); } - if (a->IsRuntimeSized()) { + if (a->Count()->Is()) { return ctx.dst->ty.array(el, nullptr, std::move(attrs)); } if (auto* override = a->Count()->As()) { diff --git a/src/tint/writer/glsl/generator_impl.cc b/src/tint/writer/glsl/generator_impl.cc index f304d82da9..c9ca4295e7 100644 --- a/src/tint/writer/glsl/generator_impl.cc +++ b/src/tint/writer/glsl/generator_impl.cc @@ -292,7 +292,7 @@ bool GeneratorImpl::Generate() { auto* sem = builder_.Sem().Get(str); bool has_rt_arr = false; if (auto* arr = sem->Members().back()->Type()->As()) { - has_rt_arr = arr->IsRuntimeSized(); + has_rt_arr = arr->Count()->Is(); } bool is_block = ast::HasAttribute(str->attributes); @@ -2834,7 +2834,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, const sem::Type* base_type = ary; std::vector sizes; while (auto* arr = base_type->As()) { - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { sizes.push_back(0); } else { auto count = arr->ConstantCount(); diff --git a/src/tint/writer/hlsl/generator_impl.cc b/src/tint/writer/hlsl/generator_impl.cc index 9d09795123..0c3856a32e 100644 --- a/src/tint/writer/hlsl/generator_impl.cc +++ b/src/tint/writer/hlsl/generator_impl.cc @@ -3924,7 +3924,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, const sem::Type* base_type = ary; std::vector sizes; while (auto* arr = base_type->As()) { - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { TINT_ICE(Writer, diagnostics_) << "runtime arrays may only exist in storage buffers, which should have " "been transformed into a ByteAddressBuffer"; diff --git a/src/tint/writer/msl/generator_impl.cc b/src/tint/writer/msl/generator_impl.cc index 9d06439f1f..c7354f0018 100644 --- a/src/tint/writer/msl/generator_impl.cc +++ b/src/tint/writer/msl/generator_impl.cc @@ -2537,7 +2537,7 @@ bool GeneratorImpl::EmitType(std::ostream& out, return false; } out << ", "; - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { out << "1"; } else { auto count = arr->ConstantCount(); @@ -3165,7 +3165,7 @@ GeneratorImpl::SizeAndAlign GeneratorImpl::MslPackedTypeSizeAndAlign(const sem:: << "arrays with explicit strides should not exist past the SPIR-V reader"; return SizeAndAlign{}; } - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { return SizeAndAlign{arr->Stride(), arr->Align()}; } if (auto count = arr->ConstantCount()) { diff --git a/src/tint/writer/spirv/builder.cc b/src/tint/writer/spirv/builder.cc index f2db43048d..f2b8f853f7 100644 --- a/src/tint/writer/spirv/builder.cc +++ b/src/tint/writer/spirv/builder.cc @@ -3839,7 +3839,7 @@ bool Builder::GenerateArrayType(const sem::Array* arr, const Operand& result) { } auto result_id = std::get(result); - if (arr->IsRuntimeSized()) { + if (arr->Count()->Is()) { push_type(spv::Op::OpTypeRuntimeArray, {result, Operand(elem_type)}); } else { auto count = arr->ConstantCount();