Move the mapping from entity name to an imported C++ global variable declaration outside of EntityName (#6211)

bricknerb · web-flow · commit ffefa7711c16 · 2025-10-15T13:33:32.000Z
This would save space for every `EntityName` that is not an imported C++
global variable.
C++ global variables include static data members.
Created `CppGlobalVarId`, `CppGlobalVarKey` and `CppGlobalVar` to allow
having `CanonicalValueStore` that maps `EntityNameId` (which is in
`CppGlobalVarKey` and `CppGlobalVar`) to `ClangDeclId` (which is also in
`CppGlobalVar`).
This is similar to `ClangDeclId`, `ClangDeclKey` and `ClangDecl` .
diff --git a/toolchain/check/context.h b/toolchain/check/context.h
@@ -255,6 +255,9 @@ class Context {
   auto entity_names() -> SemIR::EntityNameStore& {
     return sem_ir().entity_names();
   }
+  auto cpp_global_names() -> SemIR::CppGlobalVarStore& {
+    return sem_ir().cpp_global_vars();
+  }
   auto cpp_overload_sets() -> SemIR::CppOverloadSetStore& {
     return sem_ir().cpp_overload_sets();
   }
diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp
@@ -1853,7 +1853,9 @@ static auto ImportVarDecl(Context& context, SemIR::LocId loc_id,
   SemIR::EntityNameId entity_name_id =
       context.entity_names().AddSymbolicBindingName(
           var_name_id, GetParentNameScopeId(context, var_decl),
-          SemIR::CompileTimeBindIndex::None, false, clang_decl_id);
+          SemIR::CompileTimeBindIndex::None, false);
+  context.cpp_global_names().Add({.key = {.entity_name_id = entity_name_id},
+                                  .clang_decl_id = clang_decl_id});
 
   // Create `BindingPattern` and `VarPattern` in a `NameBindingDecl`.
   context.pattern_block_stack().Push();
diff --git a/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon b/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon
@@ -22,6 +22,7 @@
 // CHECK:STDOUT:   name_scopes:
 // CHECK:STDOUT:     name_scope0:     {inst: inst0000000E, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {}}
 // CHECK:STDOUT:   entity_names:    {}
+// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:       {}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
diff --git a/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon b/toolchain/check/testdata/basics/raw_sem_ir/cpp_interop.carbon
diff --git a/toolchain/check/testdata/basics/raw_sem_ir/multifile.carbon b/toolchain/check/testdata/basics/raw_sem_ir/multifile.carbon
@@ -38,6 +38,7 @@ fn B() {
 // CHECK:STDOUT:   name_scopes:
 // CHECK:STDOUT:     name_scope0:     {inst: inst0000000E, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {name0: inst6000000F}}
 // CHECK:STDOUT:   entity_names:    {}
+// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:
 // CHECK:STDOUT:     function60000000: {name: name0, parent_scope: name_scope0, call_params_id: inst_block_empty, body: [inst_block5]}
 // CHECK:STDOUT:   classes:         {}
@@ -99,7 +100,8 @@ fn B() {
 // CHECK:STDOUT:     name_scope0:     {inst: inst0000000E, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {name1: inst50000010, name0: inst50000011}}
 // CHECK:STDOUT:     name_scope1:     {inst: inst50000010, parent_scope: name_scope0, has_error: false, extended_scopes: [], names: {name1: inst50000016}}
 // CHECK:STDOUT:   entity_names:
-// CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope1, index: -1, is_template: 0, clang_decl_id: clang_decl_id<none>}
+// CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope1, index: -1, is_template: 0}
+// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:
 // CHECK:STDOUT:     function50000000: {name: name0, parent_scope: name_scope0, call_params_id: inst_block_empty, body: [inst_block5]}
 // CHECK:STDOUT:     function50000001: {name: name1, parent_scope: name_scope1}
diff --git a/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon b/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon
@@ -38,6 +38,7 @@ fn B() {
 // CHECK:STDOUT:   name_scopes:
 // CHECK:STDOUT:     name_scope0:     {inst: inst0000000E, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {name0: inst6000000F}}
 // CHECK:STDOUT:   entity_names:    {}
+// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:
 // CHECK:STDOUT:     function60000000: {name: name0, parent_scope: name_scope0, call_params_id: inst_block_empty, body: [inst_block5]}
 // CHECK:STDOUT:   classes:         {}
@@ -118,7 +119,8 @@ fn B() {
 // CHECK:STDOUT:     name_scope0:     {inst: inst0000000E, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {name1: inst50000010, name0: inst50000011}}
 // CHECK:STDOUT:     name_scope1:     {inst: inst50000010, parent_scope: name_scope0, has_error: false, extended_scopes: [], names: {name1: inst50000016}}
 // CHECK:STDOUT:   entity_names:
-// CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope1, index: -1, is_template: 0, clang_decl_id: clang_decl_id<none>}
+// CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope1, index: -1, is_template: 0}
+// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:
 // CHECK:STDOUT:     function50000000: {name: name0, parent_scope: name_scope0, call_params_id: inst_block_empty, body: [inst_block5]}
 // CHECK:STDOUT:     function50000001: {name: name1, parent_scope: name_scope1}
diff --git a/toolchain/check/testdata/basics/raw_sem_ir/one_file.carbon b/toolchain/check/testdata/basics/raw_sem_ir/one_file.carbon
diff --git a/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon b/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon
@@ -28,7 +28,8 @@ fn Foo(n: ()) -> ((), ()) {
 // CHECK:STDOUT:   name_scopes:
 // CHECK:STDOUT:     name_scope0:     {inst: inst0000000E, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {name0: inst60000025}}
 // CHECK:STDOUT:   entity_names:
-// CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope<none>, index: -1, is_template: 0, clang_decl_id: clang_decl_id<none>}
+// CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope<none>, index: -1, is_template: 0}
+// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:
 // CHECK:STDOUT:     function60000000: {name: name0, parent_scope: name_scope0, call_params_id: inst_block9, return_slot_pattern: inst60000020, body: [inst_block12]}
 // CHECK:STDOUT:   classes:         {}
diff --git a/toolchain/driver/testdata/stdin.carbon b/toolchain/driver/testdata/stdin.carbon
@@ -34,6 +34,7 @@
 // CHECK:STDOUT:   name_scopes:
 // CHECK:STDOUT:     name_scope0:     {inst: inst0000000E, parent_scope: name_scope<none>, has_error: false, extended_scopes: [], names: {}}
 // CHECK:STDOUT:   entity_names:    {}
+// CHECK:STDOUT:   cpp_global_vars: {}
 // CHECK:STDOUT:   functions:       {}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
diff --git a/toolchain/lower/BUILD b/toolchain/lower/BUILD
@@ -61,6 +61,7 @@ cc_library(
         "//toolchain/base:kind_switch",
         "//toolchain/parse:tree",
         "//toolchain/sem_ir:absolute_node_id",
+        "//toolchain/sem_ir:clang_decl",
         "//toolchain/sem_ir:diagnostic_loc_converter",
         "//toolchain/sem_ir:entry_point",
         "//toolchain/sem_ir:expr_info",
diff --git a/toolchain/lower/mangler.cpp b/toolchain/lower/mangler.cpp
@@ -9,6 +9,7 @@
 #include "common/raw_string_ostream.h"
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/lower/clang_global_decl.h"
+#include "toolchain/sem_ir/clang_decl.h"
 #include "toolchain/sem_ir/entry_point.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/pattern.h"
@@ -208,15 +209,21 @@ auto Mangler::MangleGlobalVariable(SemIR::InstId pattern_id) -> std::string {
     return std::string();
   }
 
-  auto var_name = sem_ir().entity_names().Get(var_name_id);
-  if (var_name.clang_decl_id.has_value()) {
+  SemIR::CppGlobalVarId cpp_global_var_id =
+      sem_ir().cpp_global_vars().Lookup({.entity_name_id = var_name_id});
+  if (cpp_global_var_id.has_value()) {
+    SemIR::ClangDeclId clang_decl_id =
+        sem_ir().cpp_global_vars().Get(cpp_global_var_id).clang_decl_id;
+    CARBON_CHECK(clang_decl_id.has_value(),
+                 "CppGlobalVar should have a clang_decl_id");
     return MangleCppClang(cast<clang::NamedDecl>(
-        sem_ir().clang_decls().Get(var_name.clang_decl_id).key.decl));
+        sem_ir().clang_decls().Get(clang_decl_id).key.decl));
   }
 
   RawStringOstream os;
   os << "_C";
 
+  auto var_name = sem_ir().entity_names().Get(var_name_id);
   MangleNameId(os, var_name.name_id);
   // TODO: If the variable is private, also include the library name as part of
   // the mangling.
diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD
@@ -87,6 +87,7 @@ cc_library(
         "class.h",
         "constant.h",
         "copy_on_write_block.h",
+        "cpp_global_var.h",
         "cpp_overload_set.h",
         "entity_name.h",
         "entity_with_params_base.h",
diff --git a/toolchain/sem_ir/cpp_global_var.h b/toolchain/sem_ir/cpp_global_var.h
@@ -0,0 +1,71 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_TOOLCHAIN_SEM_IR_CPP_GLOBAL_VAR_H_
+#define CARBON_TOOLCHAIN_SEM_IR_CPP_GLOBAL_VAR_H_
+
+#include "common/hashing.h"
+#include "common/ostream.h"
+#include "toolchain/sem_ir/clang_decl.h"
+#include "toolchain/sem_ir/ids.h"
+
+namespace Carbon::SemIR {
+
+// A key describing a C++ global variable imported into Carbon, identified by
+// its entity name.
+struct CppGlobalVarKey : public Printable<CppGlobalVarKey> {
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "{entity_name_id: " << entity_name_id << "}";
+  }
+
+  // TODO: Use default when `Printable` supports it.
+  friend auto operator==(const CppGlobalVarKey& lhs, const CppGlobalVarKey& rhs)
+      -> bool {
+    return lhs.entity_name_id == rhs.entity_name_id;
+  }
+
+  // The name of the variable.
+  EntityNameId entity_name_id;
+};
+
+// A C++ global variable imported into Carbon. This is used to map the entity
+// name to the Clang declaration so we can use Clang mangling.
+struct CppGlobalVar : public Printable<CppGlobalVar> {
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "{key: " << key << ", clang_decl_id: " << clang_decl_id << "}";
+  }
+
+  // Comparison against `CppGlobalVarKey`, required by `CanonicalValueStore`.
+  friend auto operator==(const CppGlobalVar& lhs, const CppGlobalVarKey& rhs)
+      -> bool {
+    return lhs.key == rhs;
+  }
+  friend auto operator==(const CppGlobalVar& lhs, const CppGlobalVar& rhs)
+      -> bool {
+    return lhs.key == rhs.key;
+  }
+
+  // Hashing for `CppGlobalVar`. See common/hashing.h.
+  friend auto CarbonHashValue(const CppGlobalVar& value, uint64_t seed)
+      -> HashCode {
+    return HashValue(value.key, seed);
+  }
+
+  // The key by which this variable can be looked up.
+  CppGlobalVarKey key;
+
+  // The Clang declaration for this variable, if any.
+  // This is ignored for equality and hashing, since it's always unique for a
+  // given key, in order to store it in `CanonicalValueStore` and allow lookup
+  // by `CppGlobalVarKey`.
+  ClangDeclId clang_decl_id;
+};
+
+// Use the name of a C++ global variable when doing `Lookup` to find an ID.
+using CppGlobalVarStore =
+    CanonicalValueStore<CppGlobalVarId, CppGlobalVarKey, CppGlobalVar>;
+
+}  // namespace Carbon::SemIR
+
+#endif  // CARBON_TOOLCHAIN_SEM_IR_CPP_GLOBAL_VAR_H_
diff --git a/toolchain/sem_ir/entity_name.h b/toolchain/sem_ir/entity_name.h
@@ -17,7 +17,7 @@ struct EntityName : public Printable<EntityName> {
   auto Print(llvm::raw_ostream& out) const -> void {
     out << "{name: " << name_id << ", parent_scope: " << parent_scope_id
         << ", index: " << bind_index_value << ", is_template: " << is_template
-        << ", clang_decl_id: " << clang_decl_id << "}";
+        << "}";
   }
 
   friend auto CarbonHashtableEq(const EntityName& lhs, const EntityName& rhs)
@@ -56,10 +56,6 @@ struct EntityName : public Printable<EntityName> {
   int32_t bind_index_value : 31 = CompileTimeBindIndex::None.index;
   // Whether this binding is a template parameter.
   bool is_template : 1 = false;
-  // For imported C++ global variable names, the Clang decl to use for mangling.
-  // TODO: Move the mapping between global variables and the clang decl to avoid
-  // paying extra memory when the names are not imported from C++.
-  ClangDeclId clang_decl_id = ClangDeclId::None;
 };
 
 // Value store for EntityName. In addition to the regular ValueStore
@@ -68,14 +64,12 @@ struct EntityNameStore : public ValueStore<EntityNameId, EntityName> {
  public:
   // Adds an entity name for a symbolic binding.
   auto AddSymbolicBindingName(NameId name_id, NameScopeId parent_scope_id,
-                              CompileTimeBindIndex bind_index, bool is_template,
-                              ClangDeclId clang_decl_id = ClangDeclId::None)
+                              CompileTimeBindIndex bind_index, bool is_template)
       -> EntityNameId {
     return Add({.name_id = name_id,
                 .parent_scope_id = parent_scope_id,
                 .bind_index_value = bind_index.index,
-                .is_template = is_template,
-                .clang_decl_id = clang_decl_id});
+                .is_template = is_template});
   }
 
   // Convert an `EntityName` to a canonical ID. All calls to this with
diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp
@@ -112,6 +112,7 @@ auto File::OutputYaml(bool include_singletons) const -> Yaml::OutputMapping {
               map.Add("clang_decls", clang_decls_.OutputYaml());
               map.Add("name_scopes", name_scopes_.OutputYaml());
               map.Add("entity_names", entity_names_.OutputYaml());
+              map.Add("cpp_global_vars", cpp_global_vars_.OutputYaml());
               map.Add("functions", functions_.OutputYaml());
               map.Add("classes", classes_.OutputYaml());
               map.Add("generics", generics_.OutputYaml());
@@ -139,6 +140,8 @@ auto File::CollectMemUsage(MemUsage& mem_usage, llvm::StringRef label) const
   mem_usage.Collect(MemUsage::ConcatLabel(label, "allocator_"), allocator_);
   mem_usage.Collect(MemUsage::ConcatLabel(label, "entity_names_"),
                     entity_names_);
+  mem_usage.Collect(MemUsage::ConcatLabel(label, "cpp_global_vars_"),
+                    cpp_global_vars_);
   mem_usage.Collect(MemUsage::ConcatLabel(label, "functions_"), functions_);
   mem_usage.Collect(MemUsage::ConcatLabel(label, "classes_"), classes_);
   mem_usage.Collect(MemUsage::ConcatLabel(label, "interfaces_"), interfaces_);
diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h
@@ -21,6 +21,7 @@
 #include "toolchain/sem_ir/associated_constant.h"
 #include "toolchain/sem_ir/class.h"
 #include "toolchain/sem_ir/constant.h"
+#include "toolchain/sem_ir/cpp_global_var.h"
 #include "toolchain/sem_ir/cpp_overload_set.h"
 #include "toolchain/sem_ir/entity_name.h"
 #include "toolchain/sem_ir/facet_type_info.h"
@@ -154,6 +155,10 @@ class File : public Printable<File> {
 
   auto entity_names() -> EntityNameStore& { return entity_names_; }
   auto entity_names() const -> const EntityNameStore& { return entity_names_; }
+  auto cpp_global_vars() -> CppGlobalVarStore& { return cpp_global_vars_; }
+  auto cpp_global_vars() const -> const CppGlobalVarStore& {
+    return cpp_global_vars_;
+  }
   auto functions() -> FunctionStore& { return functions_; }
   auto functions() const -> const FunctionStore& { return functions_; }
   auto cpp_overload_sets() -> CppOverloadSetStore& {
@@ -301,6 +306,9 @@ class File : public Printable<File> {
   // Storage for EntityNames.
   EntityNameStore entity_names_;
 
+  // For imported C++ global variables, the Clang decl to use for mangling.
+  CppGlobalVarStore cpp_global_vars_;
+
   // Storage for callable objects.
   FunctionStore functions_;
 
diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h
@@ -251,6 +251,13 @@ struct EntityNameId : public IdBase<EntityNameId> {
   using IdBase::IdBase;
 };
 
+// The ID of a C++ global variable.
+struct CppGlobalVarId : public IdBase<CppGlobalVarId> {
+  static constexpr llvm::StringLiteral Label = "cpp_global_var";
+
+  using IdBase::IdBase;
+};
+
 // The index of a compile-time binding. This is the de Bruijn level for the
 // binding -- that is, this is the number of other compile time bindings whose
 // scope encloses this binding.
diff --git a/toolchain/sem_ir/yaml_test.cpp b/toolchain/sem_ir/yaml_test.cpp
@@ -60,6 +60,7 @@ TEST(SemIRTest, Yaml) {
       Pair("clang_decls", Yaml::Mapping(SizeIs(0))),
       Pair("name_scopes", Yaml::Mapping(SizeIs(1))),
       Pair("entity_names", Yaml::Mapping(SizeIs(1))),
+      Pair("cpp_global_vars", Yaml::Mapping(SizeIs(0))),
       Pair("functions", Yaml::Mapping(SizeIs(1))),
       Pair("classes", Yaml::Mapping(SizeIs(0))),
       Pair("generics", Yaml::Mapping(SizeIs(0))),

Original file line number	Diff line number	Diff line change
`@@ -255,6 +255,9 @@ class Context {`
`255`	`255`	`auto entity_names() -> SemIR::EntityNameStore& {`
`256`	`256`	`return sem_ir().entity_names();`
`257`	`257`	`}`
	`258`	`+ auto cpp_global_names() -> SemIR::CppGlobalVarStore& {`
	`259`	`+ return sem_ir().cpp_global_vars();`
	`260`	`+ }`
`258`	`261`	`auto cpp_overload_sets() -> SemIR::CppOverloadSetStore& {`
`259`	`262`	`return sem_ir().cpp_overload_sets();`
`260`	`263`	`}`