From 002756b4cc2c2abde0155f3f430695f02e518a92 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 2 Jul 2025 12:21:01 -0700 Subject: [PATCH] Change BlockValueStore to take ElementT as a parameter (#5758) Also modify CopyOnWriteBlock to just pull block type information from the return type of the function it receives, rather than taking some as a parameter. I chose the `RefType`/`ConstRefType` names based on other similar `ValueStore` uses which I think are equivalent. --- toolchain/sem_ir/block_value_store.h | 48 ++++++++++---------------- toolchain/sem_ir/copy_on_write_block.h | 24 +++++++------ toolchain/sem_ir/ids.h | 11 ++---- toolchain/sem_ir/inst.h | 6 ++-- toolchain/sem_ir/struct_type_field.h | 3 +- 5 files changed, 40 insertions(+), 52 deletions(-) diff --git a/toolchain/sem_ir/block_value_store.h b/toolchain/sem_ir/block_value_store.h index 888bf0605f2c..f14d60b5b3f9 100644 --- a/toolchain/sem_ir/block_value_store.h +++ b/toolchain/sem_ir/block_value_store.h @@ -22,26 +22,24 @@ namespace Carbon::SemIR { // // BlockValueStore is used as-is, but there are also children that expose the // protected members for type-specific functionality. -// -// 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> { +template +class BlockValueStore : public Yaml::Printable> { public: - using ElementType = IdT::ElementType; + using IdType = IdT; + using ElementType = ElementT; + using RefType = llvm::MutableArrayRef; + using ConstRefType = llvm::ArrayRef; explicit BlockValueStore(llvm::BumpPtrAllocator& allocator) : allocator_(&allocator) { - auto empty = llvm::MutableArrayRef(); + auto empty = RefType(); auto empty_val = canonical_blocks_.Insert( empty, [&] { return values_.Add(empty); }, KeyContext(this)); CARBON_CHECK(empty_val.key() == IdT::Empty); } // Adds a block with the given content, returning an ID to reference it. - auto Add(llvm::ArrayRef content) -> IdT { + auto Add(ConstRefType content) -> IdT { if (content.empty()) { return IdT::Empty; } @@ -49,16 +47,12 @@ class BlockValueStore : public Yaml::Printable> { } // Returns the requested block. - auto Get(IdT id) const -> llvm::ArrayRef { - return values_.Get(id); - } + auto Get(IdT id) const -> ConstRefType { return values_.Get(id); } // Returns a mutable view of the requested block. This operation should be // avoided where possible; we generally want blocks to be immutable once // created. - auto GetMutable(IdT id) -> llvm::MutableArrayRef { - return values_.Get(id); - } + auto GetMutable(IdT id) -> RefType { return values_.Get(id); } // Returns a new block formed by applying `transform(elem_id)` to each element // in the specified block. @@ -70,7 +64,7 @@ class BlockValueStore : public Yaml::Printable> { // Adds a block or finds an existing canonical block with the given content, // and returns an ID to reference it. - auto AddCanonical(llvm::ArrayRef content) -> IdT { + auto AddCanonical(ConstRefType content) -> IdT { if (content.empty()) { return IdT::Empty; } @@ -115,44 +109,40 @@ class BlockValueStore : public Yaml::Printable> { protected: // Allocates a copy of the given data using our slab allocator. - auto AllocateCopy(llvm::ArrayRef data) - -> llvm::MutableArrayRef { + auto AllocateCopy(ConstRefType data) -> RefType { auto result = AllocateUninitialized(data.size()); std::uninitialized_copy(data.begin(), data.end(), result.begin()); return result; } // Allocates an uninitialized array using our slab allocator. - auto AllocateUninitialized(size_t size) - -> llvm::MutableArrayRef { + auto AllocateUninitialized(size_t size) -> RefType { // We're not going to run a destructor, so ensure that's OK. static_assert(std::is_trivially_destructible_v); auto storage = static_cast( allocator_->Allocate(size * sizeof(ElementType), alignof(ElementType))); - return llvm::MutableArrayRef(storage, size); + return RefType(storage, size); } // Allow children to have more complex value handling. - auto values() -> ValueStore& { return values_; } + auto values() -> ValueStore& { return values_; } private: class KeyContext; llvm::BumpPtrAllocator* allocator_; - ValueStore values_; + ValueStore values_; Set canonical_blocks_; }; -template -class BlockValueStore::KeyContext +template +class BlockValueStore::KeyContext : public TranslatingKeyContext { public: explicit KeyContext(const BlockValueStore* store) : store_(store) {} - auto TranslateKey(IdT id) const -> llvm::ArrayRef { - return store_->Get(id); - } + auto TranslateKey(IdT id) const -> ConstRefType { return store_->Get(id); } private: const BlockValueStore* store_; diff --git a/toolchain/sem_ir/copy_on_write_block.h b/toolchain/sem_ir/copy_on_write_block.h index 5d19d05b7937..8d3bd5555aed 100644 --- a/toolchain/sem_ir/copy_on_write_block.h +++ b/toolchain/sem_ir/copy_on_write_block.h @@ -5,6 +5,7 @@ #ifndef CARBON_TOOLCHAIN_SEM_IR_COPY_ON_WRITE_BLOCK_H_ #define CARBON_TOOLCHAIN_SEM_IR_COPY_ON_WRITE_BLOCK_H_ +#include "llvm/ADT/STLExtras.h" #include "toolchain/sem_ir/file.h" #include "toolchain/sem_ir/ids.h" @@ -19,39 +20,42 @@ namespace Carbon::SemIR { // // This is intended to avoid an unnecessary block allocation in the case where // the new block ends up being exactly the same as the original block. -template +template class CopyOnWriteBlock { public: + using BlockType = std::remove_cvref_t< + typename llvm::function_traits::result_t>; + struct UninitializedBlock { size_t size; }; // Constructs the block. `source_id` is used as the initial value of the // block. `file` must not be null. - explicit CopyOnWriteBlock(File* file, BlockIdType source_id) + explicit CopyOnWriteBlock(File* file, BlockType::IdType source_id) : file_(file), source_id_(source_id) {} // Constructs the block, treating the original block as an uninitialized block // with `size` elements. `file` must not be null. explicit CopyOnWriteBlock(File* file, UninitializedBlock uninit) : file_(file), - source_id_(BlockIdType::None), + source_id_(BlockType::IdType::None), id_((file_->*ValueStore)().AddUninitialized(uninit.size)) {} // Gets a block ID containing the resulting elements. Note that further // modifications may or may not allocate a new ID, so this should only be // called once all modifications have been performed. - auto id() const -> BlockIdType { return id_; } + auto id() const -> BlockType::IdType { return id_; } // Gets a canonical block ID containing the resulting elements. This assumes // the original block ID, if specified, was also canonical. - auto GetCanonical() const -> BlockIdType { + auto GetCanonical() const -> BlockType::IdType { return id_ == source_id_ ? id_ : (file_->*ValueStore)().MakeCanonical(id_); } // Sets the element at index `i` within the block. Lazily allocates a new // block when the value changes for the first time. - auto Set(int i, typename BlockIdType::ElementType value) -> void { + auto Set(int i, BlockType::ElementType value) -> void { if (source_id_.has_value() && (file_->*ValueStore)().Get(id_)[i] == value) { return; } @@ -63,13 +67,13 @@ class CopyOnWriteBlock { private: File* file_; - BlockIdType source_id_; - BlockIdType id_ = source_id_; + BlockType::IdType source_id_; + BlockType::IdType id_ = source_id_; }; -using CopyOnWriteInstBlock = CopyOnWriteBlock; +using CopyOnWriteInstBlock = CopyOnWriteBlock<&File::inst_blocks>; using CopyOnWriteStructTypeFieldsBlock = - CopyOnWriteBlock; + CopyOnWriteBlock<&File::struct_type_fields>; } // namespace Carbon::SemIR diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index a4a16556010e..3705f5b63ec7 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -20,7 +20,6 @@ namespace Carbon::SemIR { class File; struct FacetTypeInfo; struct SpecificInterface; -struct StructTypeField; // The ID of an `Inst`. struct InstId : public IdBase { @@ -579,12 +578,9 @@ struct NameScopeId : public IdBase { constexpr NameScopeId NameScopeId::Package = NameScopeId(0); -// The ID of an instruction block. +// The ID of an `InstId` block. struct InstBlockId : public IdBase { static constexpr llvm::StringLiteral Label = "inst_block"; - // Types for BlockValueStore. - using ElementType = InstId; - using ValueType = llvm::MutableArrayRef; // The canonical empty block, reused to avoid allocating empty vectors. Always // the 0-index block. @@ -714,12 +710,9 @@ struct ExprRegionId : public IdBase { using IdBase::IdBase; }; -// The ID of a struct type field block. +// The ID of a `StructTypeField` block. struct StructTypeFieldsId : public IdBase { static constexpr llvm::StringLiteral Label = "struct_type_fields"; - // Types for BlockValueStore. - using ElementType = StructTypeField; - using ValueType = llvm::MutableArrayRef; // The canonical empty block, reused to avoid allocating empty vectors. Always // the 0-index block. diff --git a/toolchain/sem_ir/inst.h b/toolchain/sem_ir/inst.h index 8584b1a834a6..366cc2a3606e 100644 --- a/toolchain/sem_ir/inst.h +++ b/toolchain/sem_ir/inst.h @@ -619,9 +619,9 @@ class InstStore { }; // Adapts BlockValueStore for instruction blocks. -class InstBlockStore : public BlockValueStore { +class InstBlockStore : public BlockValueStore { public: - using BaseType = BlockValueStore; + using BaseType = BlockValueStore; explicit InstBlockStore(llvm::BumpPtrAllocator& allocator) : BaseType(allocator) { @@ -642,7 +642,7 @@ class InstBlockStore : public BlockValueStore { // Reserves and returns a block ID. The contents of the block should be // specified by calling ReplacePlaceholder. auto AddPlaceholder() -> InstBlockId { - return values().Add(llvm::MutableArrayRef()); + return values().Add(llvm::MutableArrayRef()); } // Sets the contents of a placeholder block to the given content. diff --git a/toolchain/sem_ir/struct_type_field.h b/toolchain/sem_ir/struct_type_field.h index 78ab661cd5a5..43d20a9788e7 100644 --- a/toolchain/sem_ir/struct_type_field.h +++ b/toolchain/sem_ir/struct_type_field.h @@ -24,7 +24,8 @@ struct StructTypeField : Printable { TypeInstId type_inst_id; }; -using StructTypeFieldsStore = BlockValueStore; +using StructTypeFieldsStore = + BlockValueStore; // See common/hashing.h. Supports canonicalization of fields. inline auto CarbonHashValue(const StructTypeField& value, uint64_t seed)