Просмотр исходного кода

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 <jperkins@google.com>
Chandler Carruth 1 год назад
Родитель
Сommit
8992d22ab3

+ 1 - 0
toolchain/base/BUILD

@@ -40,6 +40,7 @@ cc_library(
         "//common:check",
         "//common:hashing",
         "//common:ostream",
+        "//common:set",
         "@llvm-project//llvm:Support",
     ],
 )

+ 0 - 26
toolchain/base/index_base.h

@@ -9,7 +9,6 @@
 #include <concepts>
 
 #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<Carbon::MyType>
-//       : public Carbon::IndexMapInfo<Carbon::MyType> {};
-template <typename Index>
-struct IndexMapInfo {
-  static inline auto getEmptyKey() -> Index {
-    return Index(llvm::DenseMapInfo<int32_t>::getEmptyKey());
-  }
-
-  static inline auto getTombstoneKey() -> Index {
-    return Index(llvm::DenseMapInfo<int32_t>::getTombstoneKey());
-  }
-
-  static auto getHashValue(const Index& val) -> unsigned {
-    return llvm::DenseMapInfo<int32_t>::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_

+ 73 - 68
toolchain/base/value_store.h

@@ -8,11 +8,10 @@
 #include <type_traits>
 
 #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<FloatId> {
 };
 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<RealId> {
   using ValueType = Real;
@@ -220,28 +199,25 @@ class ValueStore
 // to later retrieve the value.
 //
 // IdT::ValueType must represent the type being indexed.
-template <typename IdT,
-          typename DenseMapInfoT = llvm::DenseMapInfo<typename IdT::ValueType>>
+template <typename IdT>
 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<IdT>, 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<IdT> values_;
-  llvm::DenseMap<ValueType, IdT, DenseMapInfoT> map_;
+  Set<IdT, /*SmallSize=*/0, KeyContext> set_;
 };
 
+template <typename IdT>
+class CanonicalValueStore<IdT>::KeyContext
+    : public TranslatingKeyContext<KeyContext> {
+ public:
+  explicit KeyContext(llvm::ArrayRef<ValueType> 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<ValueType> values_;
+};
+
+template <typename IdT>
+auto CanonicalValueStore<IdT>::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<StringId>
  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<StringId>
   }
 
   // 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<StringId>
   auto size() const -> size_t { return values_.size(); }
 
  private:
-  llvm::DenseMap<llvm::StringRef, StringId> 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<StringId, /*SmallSize=*/0, KeyContext> set_;
   llvm::SmallVector<llvm::StringRef, 0> values_;
 };
 
+class CanonicalValueStore<StringId>::KeyContext
+    : public TranslatingKeyContext<KeyContext> {
+ public:
+  explicit KeyContext(llvm::ArrayRef<llvm::StringRef> values)
+      : values_(values) {}
+
+  auto TranslateKey(StringId id) const -> llvm::StringRef {
+    return values_[id.index];
+  }
+
+ private:
+  llvm::ArrayRef<llvm::StringRef> values_;
+};
+
+inline auto CanonicalValueStore<StringId>::Add(llvm::StringRef value)
+    -> StringId {
+  auto make_key = [&] {
+    auto id = static_cast<StringId>(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<StringId>::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<StringId>` 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<StringStoreWrapper<IdT>> {
   CanonicalValueStore<StringId>* values_;
 };
 
-using FloatValueStore = CanonicalValueStore<FloatId, APFloatDenseMapInfo>;
+using FloatValueStore = CanonicalValueStore<FloatId>;
 
 // 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<SharedValueStores> {
 
 }  // namespace Carbon
 
-// Support use of IdentifierId as DenseMap/DenseSet keys.
-// TODO: Remove once NameId is used in checking.
-template <>
-struct llvm::DenseMapInfo<Carbon::IdentifierId>
-    : public Carbon::IndexMapInfo<Carbon::IdentifierId> {};
-// Support use of StringId as DenseMap/DenseSet keys.
-template <>
-struct llvm::DenseMapInfo<Carbon::StringId>
-    : public Carbon::IndexMapInfo<Carbon::StringId> {};
-
 #endif  // CARBON_TOOLCHAIN_BASE_VALUE_STORE_H_

+ 4 - 0
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",

+ 35 - 34
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<PackageImports> package_imports;
 
   // A map of the package names to the outgoing imports above.
-  llvm::DenseMap<IdentifierId, int32_t> package_imports_map;
+  Map<IdentifierId, int32_t> 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<ImportKey, UnitInfo*>& api_map,
-    llvm::DenseMap<ImportKey, Parse::NodeId>* explicit_import_map,
-    UnitInfo& unit_info, Parse::Tree::PackagingNames import) -> void {
+static auto TrackImport(Map<ImportKey, UnitInfo*>& api_map,
+                        Map<ImportKey, Parse::NodeId>* 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<UnitInfo> unit_infos)
-    -> llvm::DenseMap<ImportKey, UnitInfo*> {
-  llvm::DenseMap<ImportKey, UnitInfo*> api_map;
+    llvm::MutableArrayRef<UnitInfo> unit_infos) -> Map<ImportKey, UnitInfo*> {
+  Map<ImportKey, UnitInfo*> 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<Unit> units, bool prelude_import,
     node_converters.push_back(&unit_infos.back().converter);
   }
 
-  llvm::DenseMap<ImportKey, UnitInfo*> api_map =
+  Map<ImportKey, UnitInfo*> api_map =
       BuildApiMapAndDiagnosePackaging(unit_infos);
 
   // Mark down imports for all files.
@@ -1193,7 +1194,7 @@ auto CheckParseTrees(llvm::MutableArrayRef<Unit> units, bool prelude_import,
       TrackImport(api_map, nullptr, unit_info, implicit_names);
     }
 
-    llvm::DenseMap<ImportKey, Parse::NodeId> explicit_import_map;
+    Map<ImportKey, Parse::NodeId> 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.

+ 14 - 17
toolchain/check/context.cpp

@@ -48,12 +48,12 @@ Context::Context(const Lex::TokenizedBuffer& tokens, DiagnosticEmitter& emitter,
       scope_stack_(sem_ir_->identifiers()) {
   // Map the builtin `<error>` 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<SemIR::AnyImportRef>()) {
@@ -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.

+ 2 - 2
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<SemIR::ConstantId, SemIR::TypeId> type_ids_for_type_constants_;
+  Map<SemIR::ConstantId, SemIR::TypeId> type_ids_for_type_constants_;
 
   // The list which will form NodeBlockId::Exports.
   llvm::SmallVector<SemIR::InstId> exports_;

+ 9 - 7
toolchain/check/convert.cpp

@@ -8,6 +8,7 @@
 #include <utility>
 
 #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<SemIR::NameId, int32_t> src_field_indexes;
+  Map<SemIR::NameId, int32_t> 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<SemIR::StructTypeField>(field_id).name_id, i});
-      CARBON_CHECK(added) << "Duplicate field in source structure";
+      auto result = src_field_indexes.Insert(
+          context.insts().GetAs<SemIR::StructTypeField>(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<SemIR::StructTypeField>(
         src_elem_fields[src_field_index]);

+ 11 - 7
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;

+ 2 - 2
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;
 

+ 5 - 4
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<SemIR::NameId, SemIR::InstId> names;
+  Map<SemIR::NameId, SemIR::InstId> names;
   auto& insts = sem_ir.insts();
   for (SemIR::InstId field_inst_id : fields) {
     auto field_inst = insts.GetAs<SemIR::StructTypeField>(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;
     }

+ 21 - 19
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<llvm::function_ref<SemIR::InstId()>> make_import_id)
     -> std::tuple<SemIR::NameScopeId, SemIR::ConstantId, bool> {
   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<SemIR::Namespace>(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<SemIR::NameScopeId, SemIR::NameScopeId>& copied_namespaces,
+    Map<SemIR::NameScopeId, SemIR::NameScopeId>& 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<SemIR::NameScopeId, SemIR::NameScopeId>* copied_namespaces,
+    Map<SemIR::NameScopeId, SemIR::NameScopeId>* 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<SemIR::NameScopeId, SemIR::NameScopeId>& copied_namespaces)
+    Map<SemIR::NameScopeId, SemIR::NameScopeId>& 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<SemIR::NameScopeId> 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<SemIR::NameScopeId, SemIR::NameScopeId> copied_namespaces;
+      Map<SemIR::NameScopeId, SemIR::NameScopeId> copied_namespaces;
 
       auto name_id =
           CopyNameFromImportIR(context, *import_ir.sem_ir, import_name_id);

+ 2 - 3
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;
   }
 }
 

+ 10 - 7
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<int>(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_)

+ 6 - 3
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<SemIR::NameId> names = {};
+    Set<SemIR::NameId> names = {};
 
     // TODO: This likely needs to track things which need to be destructed.
   };

+ 1 - 0
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",

+ 5 - 6
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_) {

+ 6 - 6
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<SemIR::InstBlockId, llvm::BasicBlock*> blocks_;
+  Map<SemIR::InstBlockId, llvm::BasicBlock*> 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<SemIR::InstId, llvm::Value*> locals_;
+  Map<SemIR::InstId, llvm::Value*> locals_;
 };
 
 // Declare handlers for each SemIR::File instruction that is not always

+ 2 - 0
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",

+ 27 - 27
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<BindNameInfo> {
         << ", 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<HashCode>(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<uint64_t>(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<BindNameId> {
  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<BindNameId, /*SmallSize=*/0, KeyContext> canonical_ids_;
+};
+
+class BindNameStore::KeyContext : public TranslatingKeyContext<KeyContext> {
+ 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<BindNameInfo, BindNameId, BindNameInfoDenseMapInfo>
-      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_

+ 26 - 47
toolchain/sem_ir/block_value_store.h

@@ -7,8 +7,7 @@
 
 #include <type_traits>
 
-#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<BlockValueStore<IdT>> {
   // Adds a block or finds an existing canonical block with the given content,
   // and returns an ID to reference it.
   auto AddCanonical(llvm::ArrayRef<ElementType> 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<BlockValueStore<IdT>> {
   }
 
   // Sets the contents of an empty block to the given content.
-  auto Set(IdT block_id, llvm::ArrayRef<ElementType> content) -> void {
+  auto SetContent(IdT block_id, llvm::ArrayRef<ElementType> 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<ElementType> data;
-
-    // See common/hashing.h.
-    friend auto CarbonHashValue(CanonicalBlock block, uint64_t seed)
-        -> HashCode {
-      Hasher hasher(seed);
-      hasher.HashSizedBytes(block.data);
-      return static_cast<HashCode>(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<size_t>(0))};
-    }
-    static auto getTombstoneKey() -> CanonicalBlock {
-      return CanonicalBlock{llvm::ArrayRef(SpecialData, 1)};
-    }
-    static auto getHashValue(CanonicalBlock val) -> unsigned {
-      return static_cast<uint64_t>(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<BlockValueStore<IdT>> {
 
   llvm::BumpPtrAllocator* allocator_;
   ValueStore<IdT> values_;
-  llvm::DenseMap<CanonicalBlock, IdT, CanonicalBlockDenseMapInfo>
-      canonical_blocks_;
+  Set<IdT, /*SmallSize=*/0, KeyContext> canonical_blocks_;
+};
+
+template <typename IdT>
+class BlockValueStore<IdT>::KeyContext
+    : public TranslatingKeyContext<KeyContext> {
+ public:
+  explicit KeyContext(const BlockValueStore* store) : store_(store) {}
+
+  auto TranslateKey(IdT id) const -> llvm::ArrayRef<ElementType> {
+    return store_->Get(id);
+  }
+
+ private:
+  const BlockValueStore* store_;
 };
 
 }  // namespace Carbon::SemIR

+ 6 - 8
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

+ 2 - 1
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<Inst, ConstantId> map_;
+  Map<Inst, ConstantId> map_;
   llvm::SmallVector<InstId, 0> constants_;
 };
 

+ 0 - 22
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<BindNameInfo>.
 
 // The ID of a function.
 struct FunctionId : public IdBase, public Printable<FunctionId> {
@@ -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<Carbon::SemIR::ConstantId>
-    : public Carbon::IndexMapInfo<Carbon::SemIR::ConstantId> {};
-template <>
-struct llvm::DenseMapInfo<Carbon::SemIR::InstBlockId>
-    : public Carbon::IndexMapInfo<Carbon::SemIR::InstBlockId> {};
-template <>
-struct llvm::DenseMapInfo<Carbon::SemIR::InstId>
-    : public Carbon::IndexMapInfo<Carbon::SemIR::InstId> {};
-template <>
-struct llvm::DenseMapInfo<Carbon::SemIR::NameId>
-    : public Carbon::IndexMapInfo<Carbon::SemIR::NameId> {};
-template <>
-struct llvm::DenseMapInfo<Carbon::SemIR::NameScopeId>
-    : public Carbon::IndexMapInfo<Carbon::SemIR::NameScopeId> {};
-template <>
-struct llvm::DenseMapInfo<Carbon::SemIR::TypeId>
-    : public Carbon::IndexMapInfo<Carbon::SemIR::TypeId> {};
-
 #endif  // CARBON_TOOLCHAIN_SEM_IR_IDS_H_

+ 6 - 9
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<ImplId> values_;
-  llvm::DenseMap<std::pair<TypeId, TypeId>, ImplId> lookup_;
+  Map<std::pair<TypeId, TypeId>, ImplId> lookup_;
 };
 
 }  // namespace Carbon::SemIR

+ 6 - 22
toolchain/sem_ir/inst.h

@@ -139,8 +139,6 @@ class Inst : public Printable<Inst> {
     } else {
       kind_ = TypedInst::Kind.AsInt();
     }
-    CARBON_CHECK(kind_ >= 0)
-        << "Negative kind values are reserved for DenseMapInfo.";
     if constexpr (Internal::HasTypeIdMember<TypedInst>) {
       type_id_ = typed_inst.type_id;
     }
@@ -248,14 +246,17 @@ class Inst : public Printable<Inst> {
 
   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<Carbon::SemIR::Inst>;
 
   // Table mapping instruction kinds to their argument kinds.
   static const std::pair<IdKind, IdKind> 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<InstBlockId> {
 
   auto Set(InstBlockId block_id, llvm::ArrayRef<InstId> content) -> void {
     CARBON_CHECK(block_id != InstBlockId::Unreachable);
-    BlockValueStore<InstBlockId>::Set(block_id, content);
+    BlockValueStore<InstBlockId>::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<Carbon::SemIR::Inst> {
-  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<uint64_t>(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_

+ 10 - 6
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<NameScope> {
 
   // 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<NameScope> {
   // 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<Entry> names;
-  llvm::DenseMap<NameId, int> name_map = llvm::DenseMap<NameId, int>();
+  Map<NameId, int> name_map;
 
   // Scopes extended by this scope.
   //