Switch nodes to a map, and relabel as locals. (#2861)

The "locals" naming is intended to reflect that I haven't really thought through how globals work, but suspect they're going to end up at least partially separated. Stepping away from the "node" naming feels like it'll reduce confusion, although maybe "values" might be better? (but values seems imperfect in the presence of globals)

The map is the more important part here; I want to avoid anchoring on the prior vector approach.

toolchain/lowering/lowering_context.cpp

@@ -19,7 +19,6 @@ LoweringContext::LoweringContext(llvm::LLVMContext& llvm_context,
       builder_(llvm_context),
       semantics_ir_(&semantics_ir),
       vlog_stream_(vlog_stream),
-      nodes_(semantics_ir_->nodes_size(), nullptr),
       callables_(semantics_ir_->callables_size(), nullptr) {
   CARBON_CHECK(!semantics_ir.has_errors())
       << "Generating LLVM IR from invalid SemanticsIR is unsupported.";
@@ -58,6 +57,8 @@ auto LoweringContext::LowerBlock(SemanticsNodeBlockId block_id) -> void {
 #include "toolchain/semantics/semantics_node_kind.def"
     }
   }
+
+  locals_.clear();
 }
 
 auto LoweringContext::BuildType(SemanticsNodeId node_id) -> llvm::Type* {
@@ -102,8 +103,8 @@ auto LoweringContext::BuildType(SemanticsNodeId node_id) -> llvm::Type* {
   }
 }
 
-auto LoweringContext::GetNodeLoaded(SemanticsNodeId node_id) -> llvm::Value* {
-  auto* value = GetNode(node_id);
+auto LoweringContext::GetLocalLoaded(SemanticsNodeId node_id) -> llvm::Value* {
+  auto* value = GetLocal(node_id);
   if (llvm::isa<llvm::AllocaInst, llvm::GetElementPtrInst>(value)) {
     auto* load_type = GetType(semantics_ir().GetNode(node_id).type_id());
     return builder().CreateLoad(load_type, value);

toolchain/lowering/lowering_context.h

@@ -25,20 +25,21 @@ class LoweringContext {
   // the main execution loop.
   auto Run() -> std::unique_ptr<llvm::Module>;
 
-  // Returns a value for the given node.
-  auto GetNode(SemanticsNodeId node_id) -> llvm::Value* {
-    CARBON_CHECK(nodes_[node_id.index]) << node_id;
-    return nodes_[node_id.index];
+  // Returns a local (versus global) value for the given node.
+  auto GetLocal(SemanticsNodeId node_id) -> llvm::Value* {
+    auto it = locals_.find(node_id);
+    CARBON_CHECK(it != locals_.end()) << "Missing local: " << node_id;
+    return it->second;
   }
 
-  // Returns a value for the given node in loaded state. Loads will only be
-  // inserted on an as-needed basis.
-  auto GetNodeLoaded(SemanticsNodeId node_id) -> llvm::Value*;
+  // Returns a local (versus global) value for the given node in loaded state.
+  // Loads will only be inserted on an as-needed basis.
+  auto GetLocalLoaded(SemanticsNodeId node_id) -> llvm::Value*;
 
   // Sets the value for the given node.
-  auto SetNode(SemanticsNodeId node_id, llvm::Value* value) {
-    CARBON_CHECK(!nodes_[node_id.index]) << node_id;
-    nodes_[node_id.index] = value;
+  auto SetLocal(SemanticsNodeId node_id, llvm::Value* value) {
+    bool added = locals_.insert({node_id, value}).second;
+    CARBON_CHECK(added) << "Duplicate local insert: " << node_id;
   }
 
   // Gets a callable's function.
@@ -92,12 +93,10 @@ class LoweringContext {
   llvm::SmallVector<std::pair<llvm::BasicBlock*, SemanticsNodeBlockId>>
       todo_blocks_;
 
-  // Maps nodes in SemanticsIR to a lowered value. This will have one entry per
-  // node, and will be non-null when lowered. It's expected to be sparse during
-  // execution because while expressions will have entries, statements won't.
-  // TODO: This is transitioning to only track global and local values, rather
-  // than one large map for all nodes.
-  llvm::SmallVector<llvm::Value*> nodes_;
+  // Maps a function's SemanticsIR nodes to lowered values.
+  // TODO: Handle nested scopes. Right now this is just cleared at the end of
+  // every block.
+  llvm::DenseMap<SemanticsNodeId, llvm::Value*> locals_;
 
   // Maps callables to lowered functions. Semantics treats callables as the
   // canonical form of a function, so lowering needs to do the same.

toolchain/lowering/lowering_handle.cpp

@@ -21,8 +21,8 @@ auto LoweringHandleCrossReference(LoweringContext& /*context*/,
 auto LoweringHandleAssign(LoweringContext& context, SemanticsNodeId /*node_id*/,
                           SemanticsNode node) -> void {
   auto [storage_id, value_id] = node.GetAsAssign();
-  context.builder().CreateStore(context.GetNodeLoaded(value_id),
-                                context.GetNode(storage_id));
+  context.builder().CreateStore(context.GetLocalLoaded(value_id),
+                                context.GetLocal(storage_id));
 }
 
 auto LoweringHandleBinaryOperatorAdd(LoweringContext& /*context*/,
@@ -49,11 +49,11 @@ auto LoweringHandleCall(LoweringContext& context, SemanticsNodeId node_id,
   auto* function = context.GetCallable(callable_id);
   std::vector<llvm::Value*> args;
   for (auto ref_id : context.semantics_ir().GetNodeBlock(refs_id)) {
-    args.push_back(context.GetNodeLoaded(ref_id));
+    args.push_back(context.GetLocalLoaded(ref_id));
   }
   auto* value =
       context.builder().CreateCall(function, args, function->getName());
-  context.SetNode(node_id, value);
+  context.SetLocal(node_id, value);
 }
 
 auto LoweringHandleCodeBlock(LoweringContext& /*context*/,
@@ -121,7 +121,7 @@ auto LoweringHandleIntegerLiteral(LoweringContext& context,
   // TODO: This won't offer correct semantics, but seems close enough for now.
   llvm::Value* v =
       llvm::ConstantInt::get(context.builder().getInt32Ty(), i.getSExtValue());
-  context.SetNode(node_id, v);
+  context.SetLocal(node_id, v);
 }
 
 auto LoweringHandleRealLiteral(LoweringContext& context,
@@ -134,8 +134,8 @@ auto LoweringHandleRealLiteral(LoweringContext& context,
       real.mantissa.getSExtValue() *
       std::pow((real.is_decimal ? 10 : 2), real.exponent.getSExtValue());
   llvm::APFloat llvm_val(val);
-  context.SetNode(node_id, llvm::ConstantFP::get(
-                               context.builder().getDoubleTy(), llvm_val));
+  context.SetLocal(node_id, llvm::ConstantFP::get(
+                                context.builder().getDoubleTy(), llvm_val));
 }
 
 auto LoweringHandleReturn(LoweringContext& context, SemanticsNodeId /*node_id*/,
@@ -147,7 +147,7 @@ auto LoweringHandleReturnExpression(LoweringContext& context,
                                     SemanticsNodeId /*node_id*/,
                                     SemanticsNode node) -> void {
   SemanticsNodeId expr_id = node.GetAsReturnExpression();
-  context.builder().CreateRet(context.GetNodeLoaded(expr_id));
+  context.builder().CreateRet(context.GetLocalLoaded(expr_id));
 }
 
 auto LoweringHandleStringLiteral(LoweringContext& /*context*/,
@@ -174,8 +174,8 @@ auto LoweringHandleStructMemberAccess(LoweringContext& context,
           .GetAsStructTypeField());
 
   auto* gep = context.builder().CreateStructGEP(
-      llvm_type, context.GetNode(struct_id), member_index.index, member_name);
-  context.SetNode(node_id, gep);
+      llvm_type, context.GetLocal(struct_id), member_index.index, member_name);
+  context.SetLocal(node_id, gep);
 }
 
 auto LoweringHandleStructType(LoweringContext& /*context*/,
@@ -196,7 +196,7 @@ auto LoweringHandleStructValue(LoweringContext& context,
   auto* llvm_type = context.GetType(node.type_id());
   auto* alloca = context.builder().CreateAlloca(
       llvm_type, /*ArraySize=*/nullptr, "StructLiteralValue");
-  context.SetNode(node_id, alloca);
+  context.SetLocal(node_id, alloca);
 
   auto refs = context.semantics_ir().GetNodeBlock(node.GetAsStructValue());
   // Get type information for member names.
@@ -209,14 +209,14 @@ auto LoweringHandleStructValue(LoweringContext& context,
         context.semantics_ir().GetNode(type_refs[i]).GetAsStructTypeField());
     auto* gep =
         context.builder().CreateStructGEP(llvm_type, alloca, i, member_name);
-    context.builder().CreateStore(context.GetNode(refs[i]), gep);
+    context.builder().CreateStore(context.GetLocal(refs[i]), gep);
   }
 }
 
 auto LoweringHandleStubReference(LoweringContext& context,
                                  SemanticsNodeId node_id, SemanticsNode node)
     -> void {
-  context.SetNode(node_id, context.GetNode(node.GetAsStubReference()));
+  context.SetLocal(node_id, context.GetLocal(node.GetAsStubReference()));
 }
 
 auto LoweringHandleVarStorage(LoweringContext& context, SemanticsNodeId node_id,
@@ -229,7 +229,7 @@ auto LoweringHandleVarStorage(LoweringContext& context, SemanticsNodeId node_id,
   // storage?
   auto* alloca = context.builder().CreateAlloca(context.GetType(node.type_id()),
                                                 /*ArraySize=*/nullptr, "var");
-  context.SetNode(node_id, alloca);
+  context.SetLocal(node_id, alloca);
 }
 
 }  // namespace Carbon