From 8992d22ab3996d9d1840634dc1e6d6dc01ab8f08 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 3 Jul 2024 03:10:44 +0200 Subject: [PATCH] Port the toolchain to use the new Carbon hashtable (#4097) This works to leverage the capabilities of the hashtable as much as possible, for example using the key context in the value stores. However, there may still be opportunities to refactor more deeply and use the functionality even better. Hopefully this is at least a reasonable start and gets us a clean baseline. On an Arm M1, this is a 15% improvement on my large lexing stress test, but ends up a wash on my x86-64 server. This is a smaller benefit than I expected, and it's because we're using a set-of-IDs and looking up values with a key context for things like identifiers. This pattern has a surprising tradeoff. The new hashtable uses significantly less memory, a 10% peak RSS reduction just from the hashtable change. But indirecting through the vector of values makes growing the hashtable dramatically less cache-friendly: it causes growth to randomly access every key when rehashing. On x86, everything gained by the faster hashtable is lost in even slower growth. And even on Arm, this eats into the benefits. But I have a plan to tweak how identifiers specifically work to avoid most of the growth, and so I suspect this is the right tradeoff on the whole. It gives us significant working set size reduction and we can likely avoid the regressed operation (growth with rehash) in most cases by clever reserving and if necessary by adding a hash caching layer to the table infrastructure. --------- Co-authored-by: Jon Ross-Perkins --- toolchain/base/BUILD | 1 + toolchain/base/index_base.h | 26 ----- toolchain/base/value_store.h | 141 ++++++++++++++------------- toolchain/check/BUILD | 4 + toolchain/check/check.cpp | 69 ++++++------- toolchain/check/context.cpp | 31 +++--- toolchain/check/context.h | 4 +- toolchain/check/convert.cpp | 16 +-- toolchain/check/decl_name_stack.cpp | 18 ++-- toolchain/check/handle_export.cpp | 4 +- toolchain/check/handle_struct.cpp | 9 +- toolchain/check/import.cpp | 40 ++++---- toolchain/check/merge.cpp | 5 +- toolchain/check/scope_stack.cpp | 17 ++-- toolchain/check/scope_stack.h | 9 +- toolchain/lower/BUILD | 1 + toolchain/lower/function_context.cpp | 11 +-- toolchain/lower/function_context.h | 12 +-- toolchain/sem_ir/BUILD | 2 + toolchain/sem_ir/bind_name.h | 54 +++++----- toolchain/sem_ir/block_value_store.h | 73 +++++--------- toolchain/sem_ir/constant.cpp | 14 ++- toolchain/sem_ir/constant.h | 3 +- toolchain/sem_ir/ids.h | 22 ----- toolchain/sem_ir/impl.h | 15 ++- toolchain/sem_ir/inst.h | 28 ++---- toolchain/sem_ir/name_scope.h | 16 +-- 27 files changed, 292 insertions(+), 353 deletions(-) diff --git a/toolchain/base/BUILD b/toolchain/base/BUILD index d5a6f84a20ba..c5f27547ee45 100644 --- a/toolchain/base/BUILD +++ b/toolchain/base/BUILD @@ -40,6 +40,7 @@ cc_library( "//common:check", "//common:hashing", "//common:ostream", + "//common:set", "@llvm-project//llvm:Support", ], ) diff --git a/toolchain/base/index_base.h b/toolchain/base/index_base.h index 887cccba2bbd..bd3f7a1a68de 100644 --- a/toolchain/base/index_base.h +++ b/toolchain/base/index_base.h @@ -9,7 +9,6 @@ #include #include "common/ostream.h" -#include "llvm/ADT/DenseMapInfo.h" namespace Carbon { @@ -75,31 +74,6 @@ auto operator<=>(IndexType lhs, IndexType rhs) -> std::strong_ordering { return lhs.index <=> rhs.index; } -// Provides base support for use of IdBase types as DenseMap/DenseSet keys. -// -// Usage (in global namespace): -// template <> -// struct llvm::DenseMapInfo -// : public Carbon::IndexMapInfo {}; -template -struct IndexMapInfo { - static inline auto getEmptyKey() -> Index { - return Index(llvm::DenseMapInfo::getEmptyKey()); - } - - static inline auto getTombstoneKey() -> Index { - return Index(llvm::DenseMapInfo::getTombstoneKey()); - } - - static auto getHashValue(const Index& val) -> unsigned { - return llvm::DenseMapInfo::getHashValue(val.index); - } - - static auto isEqual(const Index& lhs, const Index& rhs) -> bool { - return lhs == rhs; - } -}; - } // namespace Carbon #endif // CARBON_TOOLCHAIN_BASE_INDEX_BASE_H_ diff --git a/toolchain/base/value_store.h b/toolchain/base/value_store.h index c20a75b1e112..d598e4090021 100644 --- a/toolchain/base/value_store.h +++ b/toolchain/base/value_store.h @@ -8,11 +8,10 @@ #include #include "common/check.h" -#include "common/hashing.h" #include "common/ostream.h" +#include "common/set.h" #include "llvm/ADT/APFloat.h" #include "llvm/ADT/APInt.h" -#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/Sequence.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" @@ -77,26 +76,6 @@ struct FloatId : public IdBase, public Printable { }; constexpr FloatId FloatId::Invalid(FloatId::InvalidIndex); -// DenseMapInfo for llvm::APFloat, for use in the canonical float value store. -// LLVM has an implementation of this but it strangely lives in the non-public -// header lib/IR/LLVMContextImpl.h, so we use our own. -// TODO: Remove this once our new hash table is available. -struct APFloatDenseMapInfo { - static auto getEmptyKey() -> llvm::APFloat { - return llvm::APFloat(llvm::APFloat::Bogus(), 1); - } - static auto getTombstoneKey() -> llvm::APFloat { - return llvm::APFloat(llvm::APFloat::Bogus(), 2); - } - static auto getHashValue(const llvm::APFloat& val) -> unsigned { - return hash_value(val); - } - static auto isEqual(const llvm::APFloat& lhs, const llvm::APFloat& rhs) - -> bool { - return lhs.bitwiseIsEqual(rhs); - } -}; - // Corresponds to a Real value. struct RealId : public IdBase, public Printable { using ValueType = Real; @@ -220,28 +199,25 @@ class ValueStore // to later retrieve the value. // // IdT::ValueType must represent the type being indexed. -template > +template class CanonicalValueStore { public: using ValueType = typename IdT::ValueType; // Stores a canonical copy of the value and returns an ID to reference it. - auto Add(ValueType value) -> IdT { - auto [it, added] = map_.insert({value, IdT::Invalid}); - if (added) { - it->second = values_.Add(value); - } - return it->second; - } + auto Add(ValueType value) -> IdT; // Returns the value for an ID. auto Get(IdT id) const -> const ValueType& { return values_.Get(id); } // Reserves space. auto Reserve(size_t size) -> void { + // Compute the resulting new insert count using the size of values -- the + // set doesn't have a fast to compute current size. + if (size > values_.size()) { + set_.GrowForInsertCount(size - values_.size(), KeyContext(values_)); + } values_.Reserve(size); - map_.reserve(size); } // These are to support printable structures, and are not guaranteed. @@ -255,18 +231,34 @@ class CanonicalValueStore { auto size() const -> size_t { return values_.size(); } private: - // We store two copies of each value: one in the ValueStore for fast mapping - // from IdT to value, and another in the DenseMap for the reverse mapping. - // - // We could avoid this by storing the map instead as a DenseSet, but - // there's no way to provide a DenseMapInfo that looks the IDs up in the - // vector. - // - // TODO: Switch to a better representation that avoids the duplication. + class KeyContext; + ValueStore values_; - llvm::DenseMap map_; + Set set_; }; +template +class CanonicalValueStore::KeyContext + : public TranslatingKeyContext { + public: + explicit KeyContext(llvm::ArrayRef values) : values_(values) {} + + // Note that it is safe to return a `const` reference here as the underlying + // object's lifetime is provided by the `store_`. + auto TranslateKey(IdT id) const -> const ValueType& { + return values_[id.index]; + } + + private: + llvm::ArrayRef values_; +}; + +template +auto CanonicalValueStore::Add(ValueType value) -> IdT { + auto make_key = [&] { return IdT(values_.Add(std::move(value))); }; + return set_.Insert(value, make_key, KeyContext(values_.array_ref())).key(); +} + // Storage for StringRefs. The caller is responsible for ensuring storage is // allocated. template <> @@ -275,14 +267,7 @@ class CanonicalValueStore public: // Returns an ID to reference the value. May return an existing ID if the // string was previously added. - auto Add(llvm::StringRef value) -> StringId { - auto [it, inserted] = map_.insert({value, StringId(values_.size())}); - if (inserted) { - CARBON_CHECK(it->second.index >= 0) << "Too many unique strings"; - values_.push_back(value); - } - return it->second; - } + auto Add(llvm::StringRef value) -> StringId; // Returns the value for an ID. auto Get(StringId id) const -> llvm::StringRef { @@ -291,12 +276,7 @@ class CanonicalValueStore } // Returns an ID for the value, or Invalid if not found. - auto Lookup(llvm::StringRef value) const -> StringId { - if (auto it = map_.find(value); it != map_.end()) { - return it->second; - } - return StringId::Invalid; - } + auto Lookup(llvm::StringRef value) const -> StringId; auto OutputYaml() const -> Yaml::OutputMapping { return Yaml::OutputMapping([&](Yaml::OutputMapping::Map map) { @@ -309,12 +289,47 @@ class CanonicalValueStore auto size() const -> size_t { return values_.size(); } private: - llvm::DenseMap map_; - // Set inline size to 0 because these will typically be too large for the + class KeyContext; + + // Set inline sizes to 0 because these will typically be too large for the // stack, while this does make File smaller. + Set set_; llvm::SmallVector values_; }; +class CanonicalValueStore::KeyContext + : public TranslatingKeyContext { + public: + explicit KeyContext(llvm::ArrayRef values) + : values_(values) {} + + auto TranslateKey(StringId id) const -> llvm::StringRef { + return values_[id.index]; + } + + private: + llvm::ArrayRef values_; +}; + +inline auto CanonicalValueStore::Add(llvm::StringRef value) + -> StringId { + auto make_key = [&] { + auto id = static_cast(values_.size()); + CARBON_CHECK(id.index >= 0) << "Too many unique strings"; + values_.push_back(value); + return id; + }; + return set_.Insert(value, make_key, KeyContext(values_)).key(); +} + +inline auto CanonicalValueStore::Lookup(llvm::StringRef value) const + -> StringId { + if (auto result = set_.Lookup(value, KeyContext(values_))) { + return result.key(); + } + return StringId::Invalid; +} + // A thin wrapper around a `ValueStore` that provides a different IdT, // while using a unified storage for values. This avoids potentially // duplicative string hash maps, which are expensive. @@ -344,7 +359,7 @@ class StringStoreWrapper : public Printable> { CanonicalValueStore* values_; }; -using FloatValueStore = CanonicalValueStore; +using FloatValueStore = CanonicalValueStore; // Stores that will be used across compiler phases for a given compilation unit. // This is provided mainly so that they don't need to be passed separately. @@ -404,14 +419,4 @@ class SharedValueStores : public Yaml::Printable { } // namespace Carbon -// Support use of IdentifierId as DenseMap/DenseSet keys. -// TODO: Remove once NameId is used in checking. -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; -// Support use of StringId as DenseMap/DenseSet keys. -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; - #endif // CARBON_TOOLCHAIN_BASE_VALUE_STORE_H_ diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 0167ebd13233..f05e60e3573d 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -52,6 +52,7 @@ cc_library( deps = [ ":node_stack", "//common:check", + "//common:map", "//common:vlog", "//toolchain/base:index_base", "//toolchain/base:kind_switch", @@ -95,6 +96,7 @@ cc_library( ":sem_ir_diagnostic_converter", "//common:check", "//common:error", + "//common:map", "//common:ostream", "//common:variant_helpers", "//common:vlog", @@ -183,6 +185,7 @@ cc_library( deps = [ ":context", "//common:check", + "//common:map", "//toolchain/base:kind_switch", "//toolchain/parse:node_kind", "//toolchain/sem_ir:file", @@ -281,6 +284,7 @@ cc_library( deps = [ "//common:check", "//common:ostream", + "//common:set", "//common:vlog", "//toolchain/base:index_base", "//toolchain/sem_ir:file", diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 7dbefecf83cb..bc0d1c0b6d92 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -8,6 +8,7 @@ #include "common/check.h" #include "common/error.h" +#include "common/map.h" #include "common/variant_helpers.h" #include "common/vlog.h" #include "toolchain/base/kind_switch.h" @@ -81,7 +82,7 @@ struct UnitInfo { llvm::SmallVector package_imports; // A map of the package names to the outgoing imports above. - llvm::DenseMap package_imports_map; + Map package_imports_map; // The remaining number of imports which must be checked before this unit can // be processed. @@ -184,15 +185,15 @@ static auto ImportCurrentPackage(Context& context, UnitInfo& unit_info, SemIR::InstId package_inst_id, SemIR::TypeId namespace_type_id) -> void { // Add imports from the current package. - auto self_import_map_it = - unit_info.package_imports_map.find(IdentifierId::Invalid); - if (self_import_map_it == unit_info.package_imports_map.end()) { + auto import_map_lookup = + unit_info.package_imports_map.Lookup(IdentifierId::Invalid); + if (!import_map_lookup) { // Push the scope; there are no names to add. context.scope_stack().Push(package_inst_id, SemIR::NameScopeId::Package); return; } UnitInfo::PackageImports& self_import = - unit_info.package_imports[self_import_map_it->second]; + unit_info.package_imports[import_map_lookup.value()]; if (self_import.has_load_error) { context.name_scopes().Get(SemIR::NameScopeId::Package).has_error = true; @@ -239,11 +240,10 @@ static auto ImportOtherPackages(Context& context, UnitInfo& unit_info, // Translate the package ID from the API file to the implementation file. auto impl_package_id = impl_identifiers.Add(api_identifiers.Get(api_imports.package_id)); - if (auto it = unit_info.package_imports_map.find(impl_package_id); - it != unit_info.package_imports_map.end()) { + if (auto lookup = unit_info.package_imports_map.Lookup(impl_package_id)) { // On a hit, replace the entry to unify the API and implementation // imports. - api_imports_list[it->second] = {impl_package_id, api_imports_index}; + api_imports_list[lookup.value()] = {impl_package_id, api_imports_index}; } else { // On a miss, add the package as API-only. api_imports_list.push_back({impl_package_id, api_imports_index}); @@ -938,10 +938,10 @@ static auto RenderImportKey(ImportKey import_key) -> std::string { // // The ID comparisons between the import and unit are okay because they both // come from the same file. -static auto TrackImport( - llvm::DenseMap& api_map, - llvm::DenseMap* explicit_import_map, - UnitInfo& unit_info, Parse::Tree::PackagingNames import) -> void { +static auto TrackImport(Map& api_map, + Map* explicit_import_map, + UnitInfo& unit_info, Parse::Tree::PackagingNames import) + -> void { const auto& packaging = unit_info.unit->parse_tree->packaging_decl(); IdentifierId file_package_id = @@ -957,14 +957,14 @@ static auto TrackImport( // that might be valid with syntax fixes. if (explicit_import_map) { // Diagnose redundant imports. - if (auto [insert_it, success] = - explicit_import_map->insert({import_key, import.node_id}); - !success) { + if (auto insert_result = + explicit_import_map->Insert(import_key, import.node_id); + !insert_result.is_inserted()) { CARBON_DIAGNOSTIC(RepeatedImport, Error, "Library imported more than once."); CARBON_DIAGNOSTIC(FirstImported, Note, "First import here."); unit_info.emitter.Build(import.node_id, RepeatedImport) - .Note(insert_it->second, FirstImported) + .Note(insert_result.value(), FirstImported) .Emit(); return; } @@ -1039,26 +1039,28 @@ static auto TrackImport( } // Get the package imports, or create them if this is the first. - auto [package_imports_map_it, is_inserted] = - unit_info.package_imports_map.insert( - {import.package_id, unit_info.package_imports.size()}); - if (is_inserted) { + auto create_imports = [&]() -> int32_t { + int32_t index = unit_info.package_imports.size(); unit_info.package_imports.push_back( UnitInfo::PackageImports(import.package_id, import.node_id)); - } + return index; + }; + auto insert_result = + unit_info.package_imports_map.Insert(import.package_id, create_imports); UnitInfo::PackageImports& package_imports = - unit_info.package_imports[package_imports_map_it->second]; + unit_info.package_imports[insert_result.value()]; - if (auto api = api_map.find(import_key); api != api_map.end()) { + if (auto api_lookup = api_map.Lookup(import_key)) { // Add references between the file and imported api. - package_imports.imports.push_back({import, api->second}); + UnitInfo* api = api_lookup.value(); + package_imports.imports.push_back({import, api}); ++unit_info.imports_remaining; - api->second->incoming_imports.push_back(&unit_info); + api->incoming_imports.push_back(&unit_info); // If this is the implicit import, note we have it. if (!explicit_import_map) { CARBON_CHECK(!unit_info.api_for_impl); - unit_info.api_for_impl = api->second; + unit_info.api_for_impl = api; } } else { // The imported api is missing. @@ -1078,9 +1080,8 @@ static auto TrackImport( // related to the packaging because the strings are loaded as part of getting // the ImportKey (which we then do for `impl` files too). static auto BuildApiMapAndDiagnosePackaging( - llvm::MutableArrayRef unit_infos) - -> llvm::DenseMap { - llvm::DenseMap api_map; + llvm::MutableArrayRef unit_infos) -> Map { + Map api_map; for (auto& unit_info : unit_infos) { const auto& packaging = unit_info.unit->parse_tree->packaging_decl(); // An import key formed from the `package` or `library` declaration. Or, for @@ -1111,10 +1112,10 @@ static auto BuildApiMapAndDiagnosePackaging( // where the user forgets (or has syntax errors with) a package line // multiple times. if (!is_impl) { - auto [entry, success] = api_map.insert({import_key, &unit_info}); - if (!success) { + auto insert_result = api_map.Insert(import_key, &unit_info); + if (!insert_result.is_inserted()) { llvm::StringRef prev_filename = - entry->second->unit->tokens->source().filename(); + insert_result.value()->unit->tokens->source().filename(); if (packaging) { CARBON_DIAGNOSTIC(DuplicateLibraryApi, Error, "Library's API previously provided by `{0}`.", @@ -1178,7 +1179,7 @@ auto CheckParseTrees(llvm::MutableArrayRef units, bool prelude_import, node_converters.push_back(&unit_infos.back().converter); } - llvm::DenseMap api_map = + Map api_map = BuildApiMapAndDiagnosePackaging(unit_infos); // Mark down imports for all files. @@ -1193,7 +1194,7 @@ auto CheckParseTrees(llvm::MutableArrayRef units, bool prelude_import, TrackImport(api_map, nullptr, unit_info, implicit_names); } - llvm::DenseMap explicit_import_map; + Map explicit_import_map; // Add the prelude import. It's added to explicit_import_map so that it can // conflict with an explicit import of the prelude. diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 188eb20c8906..94eaf5644919 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -48,12 +48,12 @@ Context::Context(const Lex::TokenizedBuffer& tokens, DiagnosticEmitter& emitter, scope_stack_(sem_ir_->identifiers()) { // Map the builtin `` and `type` type constants to their corresponding // special `TypeId` values. - type_ids_for_type_constants_.insert( - {SemIR::ConstantId::ForTemplateConstant(SemIR::InstId::BuiltinError), - SemIR::TypeId::Error}); - type_ids_for_type_constants_.insert( - {SemIR::ConstantId::ForTemplateConstant(SemIR::InstId::BuiltinTypeType), - SemIR::TypeId::TypeType}); + type_ids_for_type_constants_.Insert( + SemIR::ConstantId::ForTemplateConstant(SemIR::InstId::BuiltinError), + SemIR::TypeId::Error); + type_ids_for_type_constants_.Insert( + SemIR::ConstantId::ForTemplateConstant(SemIR::InstId::BuiltinTypeType), + SemIR::TypeId::TypeType); // TODO: Remove this and add a `VerifyOnFinish` once we properly push and pop // in the right places. @@ -329,12 +329,12 @@ static auto LookupInImportIRScopes(Context& context, SemIRLoc loc, // Look up the name in the import scope. const auto& import_scope = import_ir.sem_ir->name_scopes().Get(import_scope_id); - auto it = import_scope.name_map.find(import_name_id); - if (it == import_scope.name_map.end()) { + auto lookup = import_scope.name_map.Lookup(import_name_id); + if (!lookup) { // Name doesn't exist in the import scope. continue; } - const auto& import_scope_entry = import_scope.names[it->second]; + const auto& import_scope_entry = import_scope.names[lookup.value()]; auto import_inst = import_ir.sem_ir->insts().Get(import_scope_entry.inst_id); if (import_inst.Is()) { @@ -378,8 +378,8 @@ auto Context::LookupNameInExactScope(SemIRLoc loc, SemIR::NameId name_id, SemIR::NameScopeId scope_id, const SemIR::NameScope& scope) -> SemIR::InstId { - if (auto it = scope.name_map.find(name_id); it != scope.name_map.end()) { - auto inst_id = scope.names[it->second].inst_id; + if (auto lookup = scope.name_map.Lookup(name_id)) { + auto inst_id = scope.names[lookup.value()].inst_id; LoadImportRef(*this, inst_id); return inst_id; } @@ -1058,12 +1058,9 @@ auto Context::GetTypeIdForTypeConstant(SemIR::ConstantId constant_id) CARBON_CHECK(constant_id.is_constant()) << "Canonicalizing non-constant type: " << constant_id; - auto [it, added] = type_ids_for_type_constants_.insert( - {constant_id, SemIR::TypeId::Invalid}); - if (added) { - it->second = types().Add({.constant_id = constant_id}); - } - return it->second; + auto result = type_ids_for_type_constants_.Insert( + constant_id, [&]() { return types().Add({.constant_id = constant_id}); }); + return result.value(); } // Gets or forms a type_id for a type, given the instruction kind and arguments. diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 429a3beb211a..2830503d3f91 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -5,7 +5,7 @@ #ifndef CARBON_TOOLCHAIN_CHECK_CONTEXT_H_ #define CARBON_TOOLCHAIN_CHECK_CONTEXT_H_ -#include "llvm/ADT/DenseMap.h" +#include "common/map.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/SmallVector.h" #include "toolchain/check/decl_introducer_state.h" @@ -496,7 +496,7 @@ class Context { // not clear whether that would result in more or fewer lookups. // // TODO: Should this be part of the `TypeStore`? - llvm::DenseMap type_ids_for_type_constants_; + Map type_ids_for_type_constants_; // The list which will form NodeBlockId::Exports. llvm::SmallVector exports_; diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index d94497bdefbd..bb3388aa7a87 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -8,6 +8,7 @@ #include #include "common/check.h" +#include "common/map.h" #include "llvm/ADT/STLExtras.h" #include "toolchain/base/kind_switch.h" #include "toolchain/check/context.h" @@ -412,12 +413,13 @@ static auto ConvertStructToStructOrClass(Context& context, } // Prepare to look up fields in the source by index. - llvm::SmallDenseMap src_field_indexes; + Map src_field_indexes; if (src_type.fields_id != dest_type.fields_id) { for (auto [i, field_id] : llvm::enumerate(src_elem_fields)) { - auto [it, added] = src_field_indexes.insert( - {context.insts().GetAs(field_id).name_id, i}); - CARBON_CHECK(added) << "Duplicate field in source structure"; + auto result = src_field_indexes.Insert( + context.insts().GetAs(field_id).name_id, i); + CARBON_CHECK(result.is_inserted()) + << "Duplicate field in source structure"; } } @@ -449,8 +451,9 @@ static auto ConvertStructToStructOrClass(Context& context, // Find the matching source field. auto src_field_index = i; if (src_type.fields_id != dest_type.fields_id) { - auto src_field_it = src_field_indexes.find(dest_field.name_id); - if (src_field_it == src_field_indexes.end()) { + if (auto lookup = src_field_indexes.Lookup(dest_field.name_id)) { + src_field_index = lookup.value(); + } else { if (literal_elems_id.is_valid()) { CARBON_DIAGNOSTIC( StructInitMissingFieldInLiteral, Error, @@ -469,7 +472,6 @@ static auto ConvertStructToStructOrClass(Context& context, } return SemIR::InstId::BuiltinError; } - src_field_index = src_field_it->second; } auto src_field = sem_ir.insts().GetAs( src_elem_fields[src_field_index]); diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 1b571515b5ec..34165d84d6e6 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -147,13 +147,17 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id, context_->AddExport(target_id); } - int scope_index = name_scope.names.size(); - name_scope.names.push_back({.name_id = name_context.unresolved_name_id, - .inst_id = target_id, - .access_kind = access_kind}); - auto [_, success] = name_scope.name_map.insert( - {name_context.unresolved_name_id, scope_index}); - CARBON_CHECK(success) + auto add_scope = [&] { + int index = name_scope.names.size(); + name_scope.names.push_back( + {.name_id = name_context.unresolved_name_id, + .inst_id = target_id, + .access_kind = access_kind}); + return index; + }; + auto result = name_scope.name_map.Insert( + name_context.unresolved_name_id, add_scope); + CARBON_CHECK(result.is_inserted()) << "Duplicate names should have been resolved previously: " << name_context.unresolved_name_id << " in " << name_context.parent_scope_id; diff --git a/toolchain/check/handle_export.cpp b/toolchain/check/handle_export.cpp index 83c74091aaa6..4263fe8dc676 100644 --- a/toolchain/check/handle_export.cpp +++ b/toolchain/check/handle_export.cpp @@ -78,8 +78,8 @@ auto HandleExportDecl(Context& context, Parse::ExportDeclId node_id) -> bool { // diagnostic and so that cross-package imports can find it easily. auto bind_name = context.bind_names().Get(import_ref->bind_name_id); auto& parent_scope = context.name_scopes().Get(bind_name.parent_scope_id); - auto it = parent_scope.name_map.find(bind_name.name_id); - auto& scope_inst_id = parent_scope.names[it->second].inst_id; + auto lookup = parent_scope.name_map.Lookup(bind_name.name_id); + auto& scope_inst_id = parent_scope.names[lookup.value()].inst_id; CARBON_CHECK(scope_inst_id == inst_id); scope_inst_id = export_id; diff --git a/toolchain/check/handle_struct.cpp b/toolchain/check/handle_struct.cpp index 41554c76b94e..825a9e58d1d9 100644 --- a/toolchain/check/handle_struct.cpp +++ b/toolchain/check/handle_struct.cpp @@ -2,6 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include "common/map.h" #include "toolchain/check/context.h" #include "toolchain/check/convert.h" #include "toolchain/check/handle.h" @@ -74,12 +75,12 @@ static auto DiagnoseDuplicateNames(Context& context, llvm::StringRef construct) -> bool { auto& sem_ir = context.sem_ir(); auto fields = sem_ir.inst_blocks().Get(type_block_id); - llvm::SmallDenseMap names; + Map names; auto& insts = sem_ir.insts(); for (SemIR::InstId field_inst_id : fields) { auto field_inst = insts.GetAs(field_inst_id); - auto [it, added] = names.insert({field_inst.name_id, field_inst_id}); - if (!added) { + auto result = names.Insert(field_inst.name_id, field_inst_id); + if (!result.is_inserted()) { CARBON_DIAGNOSTIC(StructNameDuplicate, Error, "Duplicated field name `{1}` in {0}.", std::string, SemIR::NameId); @@ -88,7 +89,7 @@ static auto DiagnoseDuplicateNames(Context& context, context.emitter() .Build(field_inst_id, StructNameDuplicate, construct.str(), field_inst.name_id) - .Note(it->second, StructNamePrevious) + .Note(result.value(), StructNamePrevious) .Emit(); return true; } diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index e5cdfa9d728b..b46a3da9a9c6 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -5,6 +5,7 @@ #include "toolchain/check/import.h" #include "common/check.h" +#include "common/map.h" #include "toolchain/base/kind_switch.h" #include "toolchain/check/context.h" #include "toolchain/check/import_ref.h" @@ -86,10 +87,10 @@ static auto AddNamespace( std::optional> make_import_id) -> std::tuple { auto* parent_scope = &context.name_scopes().Get(parent_scope_id); - auto [it, success] = - parent_scope->name_map.insert({name_id, parent_scope->names.size()}); - if (!success) { - auto inst_id = parent_scope->names[it->second].inst_id; + auto insert_result = + parent_scope->name_map.Insert(name_id, parent_scope->names.size()); + if (!insert_result.is_inserted()) { + auto inst_id = parent_scope->names[insert_result.value()].inst_id; if (auto namespace_inst = context.insts().TryGetAs(inst_id)) { if (diagnose_duplicate_namespace) { @@ -117,8 +118,8 @@ static auto AddNamespace( // Diagnose if there's a name conflict, but still produce the namespace to // supersede the name conflict in order to avoid repeat diagnostics. - if (!success) { - auto& entry = parent_scope->names[it->second]; + if (!insert_result.is_inserted()) { + auto& entry = parent_scope->names[insert_result.value()]; context.DiagnoseDuplicateName(namespace_id, entry.inst_id); entry.inst_id = namespace_id; entry.access_kind = SemIR::AccessKind::Public; @@ -134,11 +135,11 @@ static auto AddNamespace( // Adds a copied namespace to the cache. static auto CacheCopiedNamespace( - llvm::DenseMap& copied_namespaces, + Map& copied_namespaces, SemIR::NameScopeId import_scope_id, SemIR::NameScopeId to_scope_id) -> void { - auto [it, success] = copied_namespaces.insert({import_scope_id, to_scope_id}); - CARBON_CHECK(success || it->second == to_scope_id) + auto result = copied_namespaces.Insert(import_scope_id, to_scope_id); + CARBON_CHECK(result.is_inserted() || result.value() == to_scope_id) << "Copy result for namespace changed from " << import_scope_id << " to " << to_scope_id; } @@ -148,7 +149,7 @@ static auto CacheCopiedNamespace( // other names in conflicts. copied_namespaces is optional. static auto CopySingleNameScopeFromImportIR( Context& context, SemIR::TypeId namespace_type_id, - llvm::DenseMap* copied_namespaces, + Map* copied_namespaces, SemIR::ImportIRId ir_id, SemIR::InstId import_inst_id, SemIR::NameScopeId import_scope_id, SemIR::NameScopeId parent_scope_id, SemIR::NameId name_id) -> SemIR::NameScopeId { @@ -185,7 +186,7 @@ static auto CopyAncestorNameScopesFromImportIR( Context& context, SemIR::TypeId namespace_type_id, const SemIR::File& import_sem_ir, SemIR::ImportIRId ir_id, SemIR::NameScopeId import_parent_scope_id, - llvm::DenseMap& copied_namespaces) + Map& copied_namespaces) -> SemIR::NameScopeId { // Package-level names don't need work. if (import_parent_scope_id == SemIR::NameScopeId::Package) { @@ -200,11 +201,10 @@ static auto CopyAncestorNameScopesFromImportIR( llvm::SmallVector new_namespaces; while (import_parent_scope_id != SemIR::NameScopeId::Package) { // If the namespace was already copied, reuse the results. - if (auto it = copied_namespaces.find(import_parent_scope_id); - it != copied_namespaces.end()) { + if (auto result = copied_namespaces.Lookup(import_parent_scope_id)) { // We inject names at the provided scope, and don't need to keep // traversing parents. - scope_cursor = it->second; + scope_cursor = result.value(); break; } @@ -237,23 +237,25 @@ static auto AddImportRefOrMerge(Context& context, SemIR::ImportIRId ir_id, SemIR::NameId name_id) -> void { // Leave a placeholder that the inst comes from the other IR. auto& parent_scope = context.name_scopes().Get(parent_scope_id); - auto [it, success] = - parent_scope.name_map.insert({name_id, parent_scope.names.size()}); - if (success) { + auto insert = parent_scope.name_map.Insert(name_id, [&] { auto bind_name_id = context.bind_names().Add( {.name_id = name_id, .parent_scope_id = parent_scope_id, .bind_index = SemIR::CompileTimeBindIndex::Invalid}); + int index = parent_scope.names.size(); parent_scope.names.push_back( {.name_id = name_id, .inst_id = AddImportRef(context, {.ir_id = ir_id, .inst_id = import_inst_id}, bind_name_id), .access_kind = SemIR::AccessKind::Public}); + return index; + }); + if (insert.is_inserted()) { return; } - auto inst_id = parent_scope.names[it->second].inst_id; + auto inst_id = parent_scope.names[insert.value()].inst_id; auto prev_ir_inst = GetCanonicalImportIRInst(context, &context.sem_ir(), inst_id); VerifySameCanonicalImportIRInst(context, inst_id, prev_ir_inst, ir_id, @@ -361,7 +363,7 @@ auto ImportLibrariesFromCurrentPackage( auto [import_name_id, import_parent_scope_id] = GetImportName(*import_ir.sem_ir, import_inst); - llvm::DenseMap copied_namespaces; + Map copied_namespaces; auto name_id = CopyNameFromImportIR(context, *import_ir.sem_ir, import_name_id); diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index da290b962133..2c7c3cb1bfb7 100644 --- a/toolchain/check/merge.cpp +++ b/toolchain/check/merge.cpp @@ -124,9 +124,8 @@ auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id, SemIR::NameId name_id, SemIR::InstId new_inst_id) -> void { auto& scope = context.name_scopes().Get(scope_id); - auto it = scope.name_map.find(name_id); - if (it != scope.name_map.end()) { - scope.names[it->second].inst_id = new_inst_id; + if (auto lookup = scope.name_map.Lookup(name_id)) { + scope.names[lookup.value()].inst_id = new_inst_id; } } diff --git a/toolchain/check/scope_stack.cpp b/toolchain/check/scope_stack.cpp index d123f605f393..38b2b038ed4a 100644 --- a/toolchain/check/scope_stack.cpp +++ b/toolchain/check/scope_stack.cpp @@ -39,12 +39,12 @@ auto ScopeStack::Push(SemIR::InstId scope_inst_id, SemIR::NameScopeId scope_id, auto ScopeStack::Pop() -> void { auto scope = scope_stack_.pop_back_val(); - for (const auto& str_id : scope.names) { + scope.names.ForEach([&](SemIR::NameId str_id) { auto& lexical_results = lexical_lookup_.Get(str_id); CARBON_CHECK(lexical_results.back().scope_index == scope.index) << "Inconsistent scope index for name " << str_id; lexical_results.pop_back(); - } + }); if (scope.scope_id.is_valid()) { CARBON_CHECK(non_lexical_scope_stack_.back().scope_index == scope.index); @@ -120,12 +120,13 @@ auto ScopeStack::LookupInLexicalScopes(SemIR::NameId name_id) auto ScopeStack::LookupOrAddName(SemIR::NameId name_id, SemIR::InstId target_id) -> SemIR::InstId { - if (!scope_stack_.back().names.insert(name_id).second) { + if (!scope_stack_.back().names.Insert(name_id).is_inserted()) { auto existing = lexical_lookup_.Get(name_id).back().inst_id; CARBON_CHECK(existing.is_valid()) << "Name in scope but not in lexical lookups"; return existing; } + ++scope_stack_.back().num_names; // TODO: Reject if we previously performed a failed lookup for this name // in this scope or a scope nested within it. @@ -166,16 +167,18 @@ auto ScopeStack::Suspend() -> SuspendedScope { scope_stack_.empty() ? 0 : scope_stack_.back().next_compile_time_bind_index.index; - - result.suspended_items.reserve(result.entry.names.size() + + result.suspended_items.reserve(result.entry.num_names + compile_time_binding_stack_.size() - remaining_compile_time_bindings); - for (auto name_id : result.entry.names) { + + result.entry.names.ForEach([&](SemIR::NameId name_id) { auto [index, inst_id] = lexical_lookup_.Suspend(name_id); CARBON_CHECK(index != SuspendedScope::ScopeItem::IndexForCompileTimeBinding); result.suspended_items.push_back({.index = index, .inst_id = inst_id}); - } + }); + CARBON_CHECK(static_cast(result.suspended_items.size()) == + result.entry.num_names); // Move any compile-time bindings into the suspended scope. for (auto inst_id : llvm::ArrayRef(compile_time_binding_stack_) diff --git a/toolchain/check/scope_stack.h b/toolchain/check/scope_stack.h index b5667cb6b384..07337fd9044c 100644 --- a/toolchain/check/scope_stack.h +++ b/toolchain/check/scope_stack.h @@ -5,7 +5,7 @@ #ifndef CARBON_TOOLCHAIN_CHECK_SCOPE_STACK_H_ #define CARBON_TOOLCHAIN_CHECK_SCOPE_STACK_H_ -#include "llvm/ADT/DenseSet.h" +#include "common/set.h" #include "llvm/ADT/SmallVector.h" #include "toolchain/check/lexical_lookup.h" #include "toolchain/check/scope_index.h" @@ -64,7 +64,7 @@ class ScopeStack { // Pops the top scope from scope_stack_ if it contains no names. auto PopIfEmpty() -> void { - if (scope_stack_.back().names.empty()) { + if (scope_stack_.back().num_names == 0) { Pop(); } } @@ -178,9 +178,12 @@ class ScopeStack { // unregistered when the scope ends. bool has_returned_var = false; + // Whether there are any ids in the `names` set. + int num_names = 0; + // Names which are registered with lexical_lookup_, and will need to be // unregistered when the scope ends. - llvm::DenseSet names = {}; + Set names = {}; // TODO: This likely needs to track things which need to be destructed. }; diff --git a/toolchain/lower/BUILD b/toolchain/lower/BUILD index da6e85214409..61de4b680514 100644 --- a/toolchain/lower/BUILD +++ b/toolchain/lower/BUILD @@ -42,6 +42,7 @@ cc_library( ], deps = [ "//common:check", + "//common:map", "//common:vlog", "//toolchain/base:kind_switch", "//toolchain/sem_ir:entry_point", diff --git a/toolchain/lower/function_context.cpp b/toolchain/lower/function_context.cpp index 22db5b609143..1c708631af38 100644 --- a/toolchain/lower/function_context.cpp +++ b/toolchain/lower/function_context.cpp @@ -21,20 +21,19 @@ FunctionContext::FunctionContext(FileContext& file_context, auto FunctionContext::GetBlock(SemIR::InstBlockId block_id) -> llvm::BasicBlock* { - llvm::BasicBlock*& entry = blocks_[block_id]; - if (!entry) { + auto result = blocks_.Insert(block_id, [&] { llvm::StringRef label_name; if (const auto* inst_namer = file_context_->inst_namer()) { label_name = inst_namer->GetUnscopedLabelFor(block_id); } - entry = llvm::BasicBlock::Create(llvm_context(), label_name, function_); - } - return entry; + return llvm::BasicBlock::Create(llvm_context(), label_name, function_); + }); + return result.value(); } auto FunctionContext::TryToReuseBlock(SemIR::InstBlockId block_id, llvm::BasicBlock* block) -> bool { - if (!blocks_.insert({block_id, block}).second) { + if (!blocks_.Insert(block_id, block).is_inserted()) { return false; } if (block == synthetic_block_) { diff --git a/toolchain/lower/function_context.h b/toolchain/lower/function_context.h index 97cbd5b1f7b5..ed457f45aaa8 100644 --- a/toolchain/lower/function_context.h +++ b/toolchain/lower/function_context.h @@ -5,6 +5,7 @@ #ifndef CARBON_TOOLCHAIN_LOWER_FUNCTION_CONTEXT_H_ #define CARBON_TOOLCHAIN_LOWER_FUNCTION_CONTEXT_H_ +#include "common/map.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" @@ -48,16 +49,15 @@ class FunctionContext { return GetTypeAsValue(); } - auto it = locals_.find(inst_id); - if (it != locals_.end()) { - return it->second; + if (auto result = locals_.Lookup(inst_id)) { + return result.value(); } return file_context_->GetGlobal(inst_id); } // Sets the value for the given instruction. auto SetLocal(SemIR::InstId inst_id, llvm::Value* value) { - bool added = locals_.insert({inst_id, value}).second; + bool added = locals_.Insert(inst_id, value).is_inserted(); CARBON_CHECK(added) << "Duplicate local insert: " << inst_id << " " << sem_ir().insts().Get(inst_id); } @@ -149,14 +149,14 @@ class FunctionContext { llvm::raw_ostream* vlog_stream_; // Maps a function's SemIR::File blocks to lowered blocks. - llvm::DenseMap blocks_; + Map blocks_; // The synthetic block we most recently created. May be null if there is no // such block. llvm::BasicBlock* synthetic_block_ = nullptr; // Maps a function's SemIR::File instructions to lowered values. - llvm::DenseMap locals_; + Map locals_; }; // Declare handlers for each SemIR::File instruction that is not always diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index 73f0bcd1337b..bbd8e67c5b59 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -19,6 +19,7 @@ cc_library( hdrs = ["block_value_store.h"], deps = [ "//common:hashing", + "//common:set", "//toolchain/base:value_store", "//toolchain/base:yaml", "@llvm-project//llvm:Support", @@ -117,6 +118,7 @@ cc_library( "//common:enum_base", "//common:error", "//common:hashing", + "//common:map", "//common:set", "//toolchain/base:kind_switch", "//toolchain/base:value_store", diff --git a/toolchain/sem_ir/bind_name.h b/toolchain/sem_ir/bind_name.h index e132633e3ba2..9328acfbefe5 100644 --- a/toolchain/sem_ir/bind_name.h +++ b/toolchain/sem_ir/bind_name.h @@ -6,6 +6,7 @@ #define CARBON_TOOLCHAIN_SEM_IR_BIND_NAME_H_ #include "common/hashing.h" +#include "common/set.h" #include "toolchain/base/value_store.h" #include "toolchain/sem_ir/ids.h" @@ -17,6 +18,11 @@ struct BindNameInfo : public Printable { << ", index: " << bind_index << "}"; } + friend auto CarbonHashtableEq(const BindNameInfo& lhs, + const BindNameInfo& rhs) -> bool { + return std::memcmp(&lhs, &rhs, sizeof(BindNameInfo)) == 0; + } + // The name. NameId name_id; // The parent scope. @@ -33,44 +39,38 @@ inline auto CarbonHashValue(const BindNameInfo& value, uint64_t seed) return static_cast(hasher); } -// DenseMapInfo for BindNameInfo. -struct BindNameInfoDenseMapInfo { - static auto getEmptyKey() -> BindNameInfo { - return BindNameInfo{.name_id = NameId::Invalid, - .parent_scope_id = NameScopeId::Invalid, - .bind_index = CompileTimeBindIndex( - CompileTimeBindIndex::InvalidIndex - 1)}; - } - static auto getTombstoneKey() -> BindNameInfo { - return BindNameInfo{.name_id = NameId::Invalid, - .parent_scope_id = NameScopeId::Invalid, - .bind_index = CompileTimeBindIndex( - CompileTimeBindIndex::InvalidIndex - 2)}; - } - static auto getHashValue(const BindNameInfo& val) -> unsigned { - return static_cast(HashValue(val)); - } - static auto isEqual(const BindNameInfo& lhs, const BindNameInfo& rhs) - -> bool { - return std::memcmp(&lhs, &rhs, sizeof(BindNameInfo)) == 0; - } -}; - // Value store for BindNameInfo. In addition to the regular ValueStore // functionality, this can provide optional canonical IDs for BindNameInfos. struct BindNameStore : public ValueStore { public: // Convert an ID to a canonical ID. All calls to this with equivalent // `BindNameInfo`s will return the same `BindNameId`. - auto MakeCanonical(BindNameId id) -> BindNameId { - return canonical_ids_.insert({Get(id), id}).first->second; + auto MakeCanonical(BindNameId id) -> BindNameId; + + private: + class KeyContext; + + Set canonical_ids_; +}; + +class BindNameStore::KeyContext : public TranslatingKeyContext { + public: + explicit KeyContext(const BindNameStore* store) : store_(store) {} + + // Note that it is safe to return a `const` reference here as the underlying + // object's lifetime is provided by the `store_`. + auto TranslateKey(BindNameId id) const -> const BindNameInfo& { + return store_->Get(id); } private: - llvm::DenseMap - canonical_ids_; + const BindNameStore* store_; }; +inline auto BindNameStore::MakeCanonical(BindNameId id) -> BindNameId { + return canonical_ids_.Insert(id, KeyContext(this)).key(); +} + } // namespace Carbon::SemIR #endif // CARBON_TOOLCHAIN_SEM_IR_BIND_NAME_H_ diff --git a/toolchain/sem_ir/block_value_store.h b/toolchain/sem_ir/block_value_store.h index 16819ec931d7..c1436ce452e3 100644 --- a/toolchain/sem_ir/block_value_store.h +++ b/toolchain/sem_ir/block_value_store.h @@ -7,8 +7,7 @@ #include -#include "common/hashing.h" -#include "llvm/ADT/DenseMap.h" +#include "common/set.h" #include "toolchain/base/value_store.h" #include "toolchain/base/yaml.h" @@ -51,20 +50,20 @@ 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 [it, added] = canonical_blocks_.insert({{content}, IdT::Invalid}); - if (added) { - auto id = Add(content); - it->first.data = Get(id); - it->second = id; - } - return it->second; + auto result = canonical_blocks_.Insert( + content, [&] { return Add(content); }, KeyContext(this)); + return result.key(); } // Promotes an existing block ID to a canonical block ID, or returns an // existing canonical block ID if the block was already added. The specified // block must not be modified after this point. auto MakeCanonical(IdT id) -> IdT { - return canonical_blocks_.insert({{Get(id)}, id}).first->second; + // Get the content first so that we don't have unnecessary translation of + // the `id` into the content during insertion. + auto result = canonical_blocks_.Insert( + Get(id), [id] { return id; }, KeyContext(this)); + return result.key(); } auto OutputYaml() const -> Yaml::OutputMapping { @@ -95,47 +94,14 @@ class BlockValueStore : public Yaml::Printable> { } // Sets the contents of an empty block to the given content. - auto Set(IdT block_id, llvm::ArrayRef content) -> void { + auto SetContent(IdT block_id, llvm::ArrayRef content) -> void { CARBON_CHECK(Get(block_id).empty()) << "inst block content set more than once"; values_.Get(block_id) = AllocateCopy(content); } private: - // A canonical block, for which we allocate a deduplicated ID. - struct CanonicalBlock { - // This is mutable so we can repoint it at the allocated data if insertion - // succeeds. - mutable llvm::ArrayRef data; - - // See common/hashing.h. - friend auto CarbonHashValue(CanonicalBlock block, uint64_t seed) - -> HashCode { - Hasher hasher(seed); - hasher.HashSizedBytes(block.data); - return static_cast(hasher); - } - }; - - struct CanonicalBlockDenseMapInfo { - // Blocks whose data() points to the start of `SpecialData` are used to - // represent the special "empty" and "tombstone" states. - static constexpr ElementType SpecialData[1] = {ElementType::Invalid}; - static auto getEmptyKey() -> CanonicalBlock { - return CanonicalBlock{ - llvm::ArrayRef(SpecialData, static_cast(0))}; - } - static auto getTombstoneKey() -> CanonicalBlock { - return CanonicalBlock{llvm::ArrayRef(SpecialData, 1)}; - } - static auto getHashValue(CanonicalBlock val) -> unsigned { - return static_cast(HashValue(val)); - } - static auto isEqual(CanonicalBlock lhs, CanonicalBlock rhs) -> bool { - return lhs.data == rhs.data && (lhs.data.data() == SpecialData) == - (rhs.data.data() == SpecialData); - } - }; + class KeyContext; // Allocates an uninitialized array using our slab allocator. auto AllocateUninitialized(std::size_t size) @@ -158,8 +124,21 @@ class BlockValueStore : public Yaml::Printable> { llvm::BumpPtrAllocator* allocator_; ValueStore values_; - llvm::DenseMap - canonical_blocks_; + Set canonical_blocks_; +}; + +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); + } + + private: + const BlockValueStore* store_; }; } // namespace Carbon::SemIR diff --git a/toolchain/sem_ir/constant.cpp b/toolchain/sem_ir/constant.cpp index 53baed13aaa9..80387dbff33b 100644 --- a/toolchain/sem_ir/constant.cpp +++ b/toolchain/sem_ir/constant.cpp @@ -9,8 +9,7 @@ namespace Carbon::SemIR { auto ConstantStore::GetOrAdd(Inst inst, bool is_symbolic) -> ConstantId { - auto [it, added] = map_.insert({inst, ConstantId::Invalid}); - if (added) { + auto result = map_.Insert(inst, [&] { auto inst_id = sem_ir_.insts().AddInNoBlock(LocIdAndInst::NoLoc(inst)); ConstantId const_id = ConstantId::Invalid; if (is_symbolic) { @@ -25,14 +24,13 @@ auto ConstantStore::GetOrAdd(Inst inst, bool is_symbolic) -> ConstantId { } else { const_id = SemIR::ConstantId::ForTemplateConstant(inst_id); } - it->second = const_id; sem_ir_.constant_values().Set(inst_id, const_id); constants_.push_back(inst_id); - } else { - CARBON_CHECK(it->second != ConstantId::Invalid); - CARBON_CHECK(it->second.is_symbolic() == is_symbolic); - } - return it->second; + return const_id; + }); + CARBON_CHECK(result.value() != ConstantId::Invalid); + CARBON_CHECK(result.value().is_symbolic() == is_symbolic); + return result.value(); } } // namespace Carbon::SemIR diff --git a/toolchain/sem_ir/constant.h b/toolchain/sem_ir/constant.h index e45556dcc97e..e437050e16ef 100644 --- a/toolchain/sem_ir/constant.h +++ b/toolchain/sem_ir/constant.h @@ -5,6 +5,7 @@ #ifndef CARBON_TOOLCHAIN_SEM_IR_CONSTANT_H_ #define CARBON_TOOLCHAIN_SEM_IR_CONSTANT_H_ +#include "common/map.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/inst.h" @@ -140,7 +141,7 @@ class ConstantStore { private: File& sem_ir_; - llvm::DenseMap map_; + Map map_; llvm::SmallVector constants_; }; diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 878b5f68ef88..559774328f06 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -217,8 +217,6 @@ struct CompileTimeBindIndex : public IndexBase, constexpr CompileTimeBindIndex CompileTimeBindIndex::Invalid = CompileTimeBindIndex(InvalidIndex); -// Note that InvalidIndex - 1 and InvalidIndex - 2 are used by -// DenseMapInfo. // The ID of a function. struct FunctionId : public IdBase, public Printable { @@ -739,24 +737,4 @@ constexpr LocId LocId::Invalid = LocId(Parse::NodeId::Invalid); } // namespace Carbon::SemIR -// Support use of Id types as DenseMap/DenseSet keys. -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; -template <> -struct llvm::DenseMapInfo - : public Carbon::IndexMapInfo {}; - #endif // CARBON_TOOLCHAIN_SEM_IR_IDS_H_ diff --git a/toolchain/sem_ir/impl.h b/toolchain/sem_ir/impl.h index c0136f1398f4..89ff4e183f50 100644 --- a/toolchain/sem_ir/impl.h +++ b/toolchain/sem_ir/impl.h @@ -5,7 +5,7 @@ #ifndef CARBON_TOOLCHAIN_SEM_IR_IMPL_H_ #define CARBON_TOOLCHAIN_SEM_IR_IMPL_H_ -#include "llvm/ADT/DenseMap.h" +#include "common/map.h" #include "toolchain/sem_ir/ids.h" namespace Carbon::SemIR { @@ -58,13 +58,10 @@ class ImplStore { // `Impl` if none exists. // TODO: Handle parameters. auto LookupOrAdd(TypeId self_id, TypeId constraint_id) -> ImplId { - auto [it, added] = - lookup_.insert({{self_id, constraint_id}, ImplId::Invalid}); - if (added) { - it->second = - values_.Add({.self_id = self_id, .constraint_id = constraint_id}); - } - return it->second; + auto result = lookup_.Insert(std::pair{self_id, constraint_id}, [&]() { + return values_.Add({.self_id = self_id, .constraint_id = constraint_id}); + }); + return result.value(); } // Returns a mutable value for an ID. @@ -82,7 +79,7 @@ class ImplStore { private: ValueStore values_; - llvm::DenseMap, ImplId> lookup_; + Map, ImplId> lookup_; }; } // namespace Carbon::SemIR diff --git a/toolchain/sem_ir/inst.h b/toolchain/sem_ir/inst.h index 286ce935686c..77163b1fcbcb 100644 --- a/toolchain/sem_ir/inst.h +++ b/toolchain/sem_ir/inst.h @@ -139,8 +139,6 @@ class Inst : public Printable { } else { kind_ = TypedInst::Kind.AsInt(); } - CARBON_CHECK(kind_ >= 0) - << "Negative kind values are reserved for DenseMapInfo."; if constexpr (Internal::HasTypeIdMember) { type_id_ = typed_inst.type_id; } @@ -248,14 +246,17 @@ class Inst : public Printable { auto Print(llvm::raw_ostream& out) const -> void; + friend auto operator==(Inst lhs, Inst rhs) -> bool { + return std::memcmp(&lhs, &rhs, sizeof(Inst)) == 0; + } + private: friend class InstTestHelper; - friend struct llvm::DenseMapInfo; // Table mapping instruction kinds to their argument kinds. static const std::pair ArgKindTable[]; - // Raw constructor, used for testing and by DenseMapInfo. + // Raw constructor, used for testing. explicit Inst(InstKind kind, TypeId type_id, int32_t arg0, int32_t arg1) : Inst(kind.AsInt(), type_id, arg0, arg1) {} explicit Inst(int32_t kind, TypeId type_id, int32_t arg0, int32_t arg1) @@ -445,7 +446,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::SetContent(block_id, content); } // Returns the contents of the specified block, or an empty array if the block @@ -464,21 +465,4 @@ inline auto CarbonHashValue(const Inst& value, uint64_t seed) -> HashCode { } // namespace Carbon::SemIR -template <> -struct llvm::DenseMapInfo { - using Inst = Carbon::SemIR::Inst; - static auto getEmptyKey() -> Inst { - return Inst(-1, Carbon::SemIR::TypeId::Invalid, 0, 0); - } - static auto getTombstoneKey() -> Inst { - return Inst(-2, Carbon::SemIR::TypeId::Invalid, 0, 0); - } - static auto getHashValue(const Inst& val) -> unsigned { - return static_cast(Carbon::HashValue(val)); - } - static auto isEqual(const Inst& lhs, const Inst& rhs) -> bool { - return std::memcmp(&lhs, &rhs, sizeof(Inst)) == 0; - } -}; - #endif // CARBON_TOOLCHAIN_SEM_IR_INST_H_ diff --git a/toolchain/sem_ir/name_scope.h b/toolchain/sem_ir/name_scope.h index 6ae8757ab955..6e4ceb581d2f 100644 --- a/toolchain/sem_ir/name_scope.h +++ b/toolchain/sem_ir/name_scope.h @@ -5,6 +5,7 @@ #ifndef CARBON_TOOLCHAIN_SEM_IR_NAME_SCOPE_H_ #define CARBON_TOOLCHAIN_SEM_IR_NAME_SCOPE_H_ +#include "common/map.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/inst.h" @@ -47,11 +48,14 @@ struct NameScope : Printable { // Adds a name to the scope that must not already exist. auto AddRequired(Entry name_entry) -> void { - int index = names.size(); - names.push_back(name_entry); - auto success = name_map.insert({name_entry.name_id, index}).second; - CARBON_CHECK(success) << "Failed to add required name: " - << name_entry.name_id; + auto add_name = [&] { + int index = names.size(); + names.push_back(name_entry); + return index; + }; + auto result = name_map.Insert(name_entry.name_id, add_name); + CARBON_CHECK(result.is_inserted()) + << "Failed to add required name: " << name_entry.name_id; } // Names in the scope. We store both an insertion-ordered vector for iterating @@ -64,7 +68,7 @@ struct NameScope : Printable { // intensive, we can also switch the lookup to a set of indices into the // vector rather than a map from `NameId` to index. llvm::SmallVector names; - llvm::DenseMap name_map = llvm::DenseMap(); + Map name_map; // Scopes extended by this scope. //