diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 1a60095db2a0..cb34fd36de51 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -103,15 +103,8 @@ auto Context::AddPlaceholderInst(SemIR::ParseNodeAndInst parse_node_and_inst) auto Context::AddConstant(SemIR::Inst inst, bool is_symbolic) -> SemIR::ConstantId { - auto [inst_id, added] = constants().GetOrAdd(inst); - auto const_id = is_symbolic ? SemIR::ConstantId::ForSymbolicConstant(inst_id) - : SemIR::ConstantId::ForTemplateConstant(inst_id); - if (added) { - // TODO: Should `ConstantStore` do this for us? - constant_values().Set(inst_id, const_id); - } - - CARBON_VLOG() << "AddConstantInst: " << inst << "\n"; + auto const_id = constants().GetOrAdd(inst, is_symbolic); + CARBON_VLOG() << "AddConstant: " << inst << "\n"; return const_id; } diff --git a/toolchain/sem_ir/value_stores.cpp b/toolchain/sem_ir/value_stores.cpp index fa8acecc44ca..8b8f2ed0842b 100644 --- a/toolchain/sem_ir/value_stores.cpp +++ b/toolchain/sem_ir/value_stores.cpp @@ -10,30 +10,41 @@ namespace Carbon::SemIR { -auto ConstantStore::GetOrAdd(Inst inst) -> std::pair { +auto ConstantStore::GetOrAdd(Inst inst, bool is_symbolic) -> ConstantId { // Compute the instruction's profile. - ConstantNode node = {.inst = inst, .inst_id = InstId::Invalid}; + ConstantNode node = {.inst = inst, .constant_id = ConstantId::NotConstant}; llvm::FoldingSetNodeID id; node.Profile(id, constants_.getContext()); // Check if we have already created this constant. void* insert_pos; if (ConstantNode* found = constants_.FindNodeOrInsertPos(id, insert_pos)) { - return {found->inst_id, false}; + CARBON_CHECK(found->constant_id.is_constant()) + << "Found non-constant in constant store for " << inst; + CARBON_CHECK(found->constant_id.is_symbolic() == is_symbolic) + << "Mismatch in phase for constant " << inst; + return found->constant_id; } // Create the new inst and insert the new node. - node.inst_id = constants_.getContext()->insts().AddInNoBlock( + auto inst_id = constants_.getContext()->insts().AddInNoBlock( ParseNodeAndInst::Untyped(Parse::NodeId::Invalid, inst)); + auto constant_id = is_symbolic + ? SemIR::ConstantId::ForSymbolicConstant(inst_id) + : SemIR::ConstantId::ForTemplateConstant(inst_id); + node.constant_id = constant_id; constants_.InsertNode(new (*allocator_) ConstantNode(node), insert_pos); - return {node.inst_id, true}; + + // The constant value of any constant instruction is that instruction itself. + constants_.getContext()->constant_values().Set(inst_id, constant_id); + return constant_id; } auto ConstantStore::GetAsVector() const -> llvm::SmallVector { llvm::SmallVector result; result.reserve(constants_.size()); for (const ConstantNode& node : constants_) { - result.push_back(node.inst_id); + result.push_back(node.constant_id.inst_id()); } // For stability, put the results into index order. This happens to also be // insertion order. diff --git a/toolchain/sem_ir/value_stores.h b/toolchain/sem_ir/value_stores.h index c1a495a153dd..2e8af333722e 100644 --- a/toolchain/sem_ir/value_stores.h +++ b/toolchain/sem_ir/value_stores.h @@ -141,9 +141,11 @@ class ConstantStore { : allocator_(&allocator), constants_(&sem_ir) {} // Adds a new constant instruction, or gets the existing constant with this - // value. Returns the instruction ID and a bool indicating whether a new - // instruction was added. - auto GetOrAdd(Inst inst) -> std::pair; + // value. Returns the ID of the constant. + // + // This updates `sem_ir.insts()` and `sem_ir.constant_values()` if the + // constant is new. + auto GetOrAdd(Inst inst, bool is_symbolic) -> ConstantId; // Returns a copy of the constant IDs as a vector, in an arbitrary but // stable order. This should not be used anywhere performance-sensitive. @@ -159,7 +161,7 @@ class ConstantStore { // impact on compile time from that change. struct ConstantNode : llvm::FoldingSetNode { Inst inst; - InstId inst_id; + ConstantId constant_id; auto Profile(llvm::FoldingSetNodeID& id, File* sem_ir) -> void; };