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. //