From 132807e13889cecb8036601dc9422a1237809a3f Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 30 Nov 2023 18:10:02 -0800 Subject: [PATCH] Refactor ValueStore to reduce template type repetition. (#3437) There's a trade-off here of explicitness in the use versus repetition, but I'm hoping the Id offers sufficient info (also, some Ids already relied on this, so this builds consistency). The forward declarations I'm mixed on, but they are difficult to avoid if heading down this route due to interdependencies between ids and types which contain ids. --- toolchain/base/value_store.h | 29 +++++++++++--------- toolchain/check/context.h | 15 ++++------- toolchain/sem_ir/file.h | 36 +++++++++++-------------- toolchain/sem_ir/ids.h | 25 ++++++++++++++++++ toolchain/sem_ir/value_stores.h | 47 ++++++++++++++++++++------------- 5 files changed, 91 insertions(+), 61 deletions(-) diff --git a/toolchain/base/value_store.h b/toolchain/base/value_store.h index 78d2a62d0504..c6520a2f399a 100644 --- a/toolchain/base/value_store.h +++ b/toolchain/base/value_store.h @@ -49,7 +49,7 @@ class Real : public Printable { // Corresponds to an integer value represented by an APInt. struct IntId : public IdBase, public Printable { - using IndexedType = const llvm::APInt; + using ValueType = const llvm::APInt; static const IntId Invalid; using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { @@ -61,7 +61,7 @@ constexpr IntId IntId::Invalid(IntId::InvalidIndex); // Corresponds to a Real value. struct RealId : public IdBase, public Printable { - using IndexedType = const Real; + using ValueType = const Real; static const RealId Invalid; using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { @@ -73,7 +73,7 @@ constexpr RealId RealId::Invalid(RealId::InvalidIndex); // Corresponds to a StringRef. struct StringId : public IdBase, public Printable { - using IndexedType = const std::string; + using ValueType = const std::string; static const StringId Invalid; using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { @@ -117,14 +117,19 @@ class ValueStoreNotPrintable {}; // A simple wrapper for accumulating values, providing IDs to later retrieve the // value. This does not do deduplication. -template +// +// IdT::ValueType must represent the type being indexed. +template class ValueStore - : public std::conditional, ValueT>, - Yaml::Printable>, - Internal::ValueStoreNotPrintable> { + : public std::conditional< + std::is_base_of_v, + typename IdT::ValueType>, + Yaml::Printable>, Internal::ValueStoreNotPrintable> { public: + using ValueType = typename IdT::ValueType; + // Stores the value and returns an ID to reference it. - auto Add(ValueT value) -> IdT { + auto Add(ValueType value) -> IdT { IdT id = IdT(values_.size()); CARBON_CHECK(id.index >= 0) << "Id overflow"; values_.push_back(std::move(value)); @@ -139,13 +144,13 @@ class ValueStore } // Returns a mutable value for an ID. - auto Get(IdT id) -> ValueT& { + auto Get(IdT id) -> ValueType& { CARBON_CHECK(id.index >= 0) << id.index; return values_[id.index]; } // Returns the value for an ID. - auto Get(IdT id) const -> const ValueT& { + auto Get(IdT id) const -> const ValueType& { CARBON_CHECK(id.index >= 0) << id.index; return values_[id.index]; } @@ -163,11 +168,11 @@ class ValueStore }); } - auto array_ref() const -> llvm::ArrayRef { return values_; } + auto array_ref() const -> llvm::ArrayRef { return values_; } auto size() const -> int { return values_.size(); } private: - llvm::SmallVector> values_; + llvm::SmallVector> values_; }; // Storage for StringRefs. The caller is responsible for ensuring storage is diff --git a/toolchain/check/context.h b/toolchain/check/context.h index d411bef0214b..f703d3785efc 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -302,24 +302,19 @@ class Context { auto string_literals() -> StringStoreWrapper& { return sem_ir().string_literals(); } - auto functions() -> ValueStore& { + auto functions() -> ValueStore& { return sem_ir().functions(); } - auto classes() -> ValueStore& { - return sem_ir().classes(); - } - auto cross_ref_irs() -> ValueStore& { + auto classes() -> ValueStore& { return sem_ir().classes(); } + auto cross_ref_irs() -> ValueStore& { return sem_ir().cross_ref_irs(); } auto names() -> SemIR::NameStoreWrapper { return sem_ir().names(); } auto name_scopes() -> SemIR::NameScopeStore& { return sem_ir().name_scopes(); } - auto types() -> ValueStore& { - return sem_ir().types(); - } - auto type_blocks() - -> SemIR::BlockValueStore& { + auto types() -> ValueStore& { return sem_ir().types(); } + auto type_blocks() -> SemIR::BlockValueStore& { return sem_ir().type_blocks(); } auto insts() -> SemIR::InstStore& { return sem_ir().insts(); } diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index 2448d6fe9e07..e7cbb9937a1d 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -289,16 +289,12 @@ class File : public Printable { return value_stores_->string_literals(); } - auto functions() -> ValueStore& { return functions_; } - auto functions() const -> const ValueStore& { - return functions_; - } - auto classes() -> ValueStore& { return classes_; } - auto classes() const -> const ValueStore& { return classes_; } - auto cross_ref_irs() -> ValueStore& { - return cross_ref_irs_; - } - auto cross_ref_irs() const -> const ValueStore& { + auto functions() -> ValueStore& { return functions_; } + auto functions() const -> const ValueStore& { return functions_; } + auto classes() -> ValueStore& { return classes_; } + auto classes() const -> const ValueStore& { return classes_; } + auto cross_ref_irs() -> ValueStore& { return cross_ref_irs_; } + auto cross_ref_irs() const -> const ValueStore& { return cross_ref_irs_; } auto names() const -> NameStoreWrapper { @@ -306,12 +302,10 @@ class File : public Printable { } auto name_scopes() -> NameScopeStore& { return name_scopes_; } auto name_scopes() const -> const NameScopeStore& { return name_scopes_; } - auto types() -> ValueStore& { return types_; } - auto types() const -> const ValueStore& { return types_; } - auto type_blocks() -> BlockValueStore& { - return type_blocks_; - } - auto type_blocks() const -> const BlockValueStore& { + auto types() -> ValueStore& { return types_; } + auto types() const -> const ValueStore& { return types_; } + auto type_blocks() -> BlockValueStore& { return type_blocks_; } + auto type_blocks() const -> const BlockValueStore& { return type_blocks_; } auto insts() -> InstStore& { return insts_; } @@ -353,28 +347,28 @@ class File : public Printable { std::string filename_; // Storage for callable objects. - ValueStore functions_; + ValueStore functions_; // Storage for classes. - ValueStore classes_; + ValueStore classes_; // Related IRs. There will always be at least 2 entries, the builtin IR (used // for references of builtins) followed by the current IR (used for references // crossing instruction blocks). - ValueStore cross_ref_irs_; + ValueStore cross_ref_irs_; // Storage for name scopes. NameScopeStore name_scopes_; // Descriptions of types used in this file. - ValueStore types_; + ValueStore types_; // Types that were completed in this file. llvm::SmallVector complete_types_; // Type blocks within the IR. These reference entries in types_. Storage for // the data is provided by allocator_. - BlockValueStore type_blocks_; + BlockValueStore type_blocks_; // All instructions. The first entries will always be cross-references to // builtins, at indices matching BuiltinKind ordering. diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 353a98686d30..b50657a4b14c 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -13,8 +13,17 @@ namespace Carbon::SemIR { +// Forward declare indexed types, for integration with ValueStore. +class File; +class Inst; +struct Class; +struct Function; +struct TypeInfo; + // The ID of an instruction. struct InstId : public IdBase, public Printable { + using ValueType = Inst; + // An explicitly invalid instruction ID. static const InstId Invalid; @@ -52,6 +61,8 @@ constexpr InstId InstId::Invalid = InstId(InstId::InvalidIndex); // The ID of a function. struct FunctionId : public IdBase, public Printable { + using ValueType = Function; + // An explicitly invalid function ID. static const FunctionId Invalid; @@ -66,6 +77,8 @@ constexpr FunctionId FunctionId::Invalid = FunctionId(FunctionId::InvalidIndex); // The ID of a class. struct ClassId : public IdBase, public Printable { + using ValueType = Class; + // An explicitly invalid class ID. static const ClassId Invalid; @@ -80,6 +93,8 @@ constexpr ClassId ClassId::Invalid = ClassId(ClassId::InvalidIndex); // The ID of a cross-referenced IR. struct CrossRefIRId : public IdBase, public Printable { + using ValueType = const File*; + static const CrossRefIRId Builtins; using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { @@ -164,6 +179,8 @@ constexpr NameId NameId::ReturnSlot = NameId(NameId::InvalidIndex - 3); // The ID of a name scope. struct NameScopeId : public IdBase, public Printable { + using ValueType = llvm::DenseMap; + // An explicitly invalid ID. static const NameScopeId Invalid; @@ -179,6 +196,9 @@ constexpr NameScopeId NameScopeId::Invalid = // The ID of an instruction block. struct InstBlockId : public IdBase, public Printable { + using ElementType = InstId; + using ValueType = llvm::MutableArrayRef; + // All File instances must provide the 0th instruction block as empty. static const InstBlockId Empty; @@ -207,6 +227,8 @@ constexpr InstBlockId InstBlockId::Unreachable = // The ID of a type. struct TypeId : public IdBase, public Printable { + using ValueType = TypeInfo; + // The builtin TypeType. static const TypeId TypeType; @@ -235,6 +257,9 @@ constexpr TypeId TypeId::Invalid = TypeId(TypeId::InvalidIndex); // The ID of a type block. struct TypeBlockId : public IdBase, public Printable { + using ElementType = TypeId; + using ValueType = llvm::MutableArrayRef; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "typeBlock"; diff --git a/toolchain/sem_ir/value_stores.h b/toolchain/sem_ir/value_stores.h index e42107217958..95bde6f7097e 100644 --- a/toolchain/sem_ir/value_stores.h +++ b/toolchain/sem_ir/value_stores.h @@ -42,7 +42,7 @@ class InstStore { auto size() const -> int { return values_.size(); } private: - ValueStore values_; + ValueStore values_; }; // Provides storage for instructions representing global constants. @@ -121,7 +121,7 @@ class NameScopeStore { } private: - ValueStore> values_; + ValueStore values_; }; // Provides a block-based ValueStore, which uses slab allocation of added @@ -130,22 +130,33 @@ class NameScopeStore { // // BlockValueStore is used as-is, but there are also children that expose the // protected members for type-specific functionality. -template -class BlockValueStore : public Yaml::Printable> { +// +// On IdT, this requires: +// - IdT::ElementType to represent the underlying type in the block. +// - IdT::ValueType to be llvm::MutableArrayRef for +// compatibility with ValueStore. +template +class BlockValueStore : public Yaml::Printable> { public: + using ElementType = typename IdT::ElementType; + explicit BlockValueStore(llvm::BumpPtrAllocator& allocator) : allocator_(&allocator) {} // Adds a block with the given content, returning an ID to reference it. - auto Add(llvm::ArrayRef content) -> IdT { + auto Add(llvm::ArrayRef content) -> IdT { return values_.Add(AllocateCopy(content)); } // Returns the requested block. - auto Get(IdT id) const -> llvm::ArrayRef { return values_.Get(id); } + auto Get(IdT id) const -> llvm::ArrayRef { + return values_.Get(id); + } // Returns the requested block. - auto Get(IdT id) -> llvm::MutableArrayRef { return values_.Get(id); } + auto Get(IdT id) -> llvm::MutableArrayRef { + return values_.Get(id); + } auto OutputYaml() const -> Yaml::OutputMapping { return Yaml::OutputMapping([&](Yaml::OutputMapping::Map map) { @@ -184,31 +195,31 @@ class BlockValueStore : public Yaml::Printable> { private: // Allocates an uninitialized array using our slab allocator. auto AllocateUninitialized(std::size_t size) - -> llvm::MutableArrayRef { + -> llvm::MutableArrayRef { // We're not going to run a destructor, so ensure that's OK. - static_assert(std::is_trivially_destructible_v); + static_assert(std::is_trivially_destructible_v); - auto storage = static_cast( - allocator_->Allocate(size * sizeof(ValueT), alignof(ValueT))); - return llvm::MutableArrayRef(storage, size); + auto storage = static_cast( + allocator_->Allocate(size * sizeof(ElementType), alignof(ElementType))); + return llvm::MutableArrayRef(storage, size); } // Allocates a copy of the given data using our slab allocator. - auto AllocateCopy(llvm::ArrayRef data) - -> llvm::MutableArrayRef { + auto AllocateCopy(llvm::ArrayRef data) + -> llvm::MutableArrayRef { auto result = AllocateUninitialized(data.size()); std::uninitialized_copy(data.begin(), data.end(), result.begin()); return result; } llvm::BumpPtrAllocator* allocator_; - ValueStore> values_; + ValueStore values_; }; // Adapts BlockValueStore for instruction blocks. -class InstBlockStore : public BlockValueStore { +class InstBlockStore : public BlockValueStore { public: - using BaseType = BlockValueStore; + using BaseType = BlockValueStore; using BaseType::AddDefaultValue; using BaseType::AddUninitialized; @@ -216,7 +227,7 @@ class InstBlockStore : public BlockValueStore { auto Set(InstBlockId block_id, llvm::ArrayRef content) -> void { CARBON_CHECK(block_id != InstBlockId::Unreachable); - BlockValueStore::Set(block_id, content); + BlockValueStore::Set(block_id, content); } };