Adds per-builtin instructions, removing BuiltinInst (#4556)

Adds per-builtin instructions, removing `BuiltinInst`. This collapses
`builtin_inst_kind.def` into `inst_kind.def` so that we have a single
place for all macro uses. I still want to remove `BuiltinInstKind`, but
it's something I think is better separated from the `BuiltinInst`
removal.

I'm collapsing the build targets `ids` and `inst_kind` into one because
they both have links to builtin kind information now. It's hard to
separate without a cycle. I'm using the `typed_insts` name because that
seems like the actual most significant thing there, and more interesting
relative to the `inst` target.

toolchain/check/BUILD

@@ -82,12 +82,10 @@ cc_library(
         "//toolchain/parse:node_kind",
         "//toolchain/parse:tree",
         "//toolchain/parse:tree_node_diagnostic_converter",
-        "//toolchain/sem_ir:builtin_inst_kind",
         "//toolchain/sem_ir:file",
         "//toolchain/sem_ir:formatter",
-        "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
-        "//toolchain/sem_ir:inst_kind",
+        "//toolchain/sem_ir:typed_insts",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -131,12 +129,10 @@ cc_library(
         "//toolchain/parse:node_kind",
         "//toolchain/parse:tree",
         "//toolchain/parse:tree_node_diagnostic_converter",
-        "//toolchain/sem_ir:builtin_inst_kind",
         "//toolchain/sem_ir:entry_point",
         "//toolchain/sem_ir:file",
-        "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
-        "//toolchain/sem_ir:inst_kind",
+        "//toolchain/sem_ir:typed_insts",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -164,8 +160,8 @@ cc_library(
         "//common:ostream",
         "//common:vlog",
         "//toolchain/sem_ir:file",
-        "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
+        "//toolchain/sem_ir:typed_insts",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -180,9 +176,8 @@ cc_library(
         "//toolchain/base:kind_switch",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/sem_ir:file",
-        "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
-        "//toolchain/sem_ir:inst_kind",
+        "//toolchain/sem_ir:typed_insts",
     ],
 )
 
@@ -194,9 +189,8 @@ cc_library(
         ":context",
         "//common:check",
         "//toolchain/sem_ir:file",
-        "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
-        "//toolchain/sem_ir:inst_kind",
+        "//toolchain/sem_ir:typed_insts",
     ],
 )
 
@@ -210,7 +204,7 @@ cc_library(
         "//common:vlog",
         "//toolchain/parse:node_kind",
         "//toolchain/parse:tree",
-        "//toolchain/sem_ir:ids",
+        "//toolchain/sem_ir:typed_insts",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -223,9 +217,8 @@ cc_library(
         ":context",
         "//common:check",
         "//toolchain/parse:node_kind",
-        "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
-        "//toolchain/sem_ir:inst_kind",
+        "//toolchain/sem_ir:typed_insts",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -248,7 +241,7 @@ cc_library(
         "//toolchain/base:value_ids",
         "//toolchain/base:value_store",
         "//toolchain/sem_ir:file",
-        "//toolchain/sem_ir:ids",
+        "//toolchain/sem_ir:typed_insts",
         "@llvm-project//llvm:Support",
     ],
 )

toolchain/check/context.cpp

@@ -1000,31 +1000,24 @@ class TypeCompleter {
     return value_rep;
   }
 
+  template <typename InstT>
+    requires(InstT::Kind.template IsAnyOf<
+             SemIR::AutoType, SemIR::BoolType, SemIR::BoundMethodType,
+             SemIR::ErrorInst, SemIR::IntLiteralType, SemIR::LegacyFloatType,
+             SemIR::NamespaceType, SemIR::SpecificFunctionType, SemIR::TypeType,
+             SemIR::VtableType, SemIR::WitnessType>())
+  auto BuildValueReprForInst(SemIR::TypeId type_id, InstT /*inst*/) const
+      -> SemIR::ValueRepr {
+    return MakeCopyValueRepr(type_id);
+  }
+
   auto BuildValueReprForInst(SemIR::TypeId type_id,
-                             SemIR::BuiltinInst builtin) const
+                             SemIR::StringType /*inst*/) const
       -> SemIR::ValueRepr {
-    switch (builtin.builtin_inst_kind) {
-      case SemIR::BuiltinInstKind::TypeType:
-      case SemIR::BuiltinInstKind::AutoType:
-      case SemIR::BuiltinInstKind::ErrorInst:
-      case SemIR::BuiltinInstKind::Invalid:
-      case SemIR::BuiltinInstKind::BoolType:
-      case SemIR::BuiltinInstKind::IntLiteralType:
-      case SemIR::BuiltinInstKind::LegacyFloatType:
-      case SemIR::BuiltinInstKind::NamespaceType:
-      case SemIR::BuiltinInstKind::BoundMethodType:
-      case SemIR::BuiltinInstKind::WitnessType:
-      case SemIR::BuiltinInstKind::SpecificFunctionType:
-      case SemIR::BuiltinInstKind::VtableType:
-        return MakeCopyValueRepr(type_id);
-
-      case SemIR::BuiltinInstKind::StringType:
-        // TODO: Decide on string value semantics. This should probably be a
-        // custom value representation carrying a pointer and size or
-        // similar.
-        return MakePointerValueRepr(type_id);
-    }
-    llvm_unreachable("All builtin kinds were handled above");
+    // TODO: Decide on string value semantics. This should probably be a
+    // custom value representation carrying a pointer and size or
+    // similar.
+    return MakePointerValueRepr(type_id);
   }
 
   auto BuildStructOrTupleValueRepr(size_t num_elements,
@@ -1340,7 +1333,6 @@ auto Context::GetAssociatedEntityType(SemIR::TypeId interface_type_id,
 }
 
 auto Context::GetBuiltinType(SemIR::BuiltinInstKind kind) -> SemIR::TypeId {
-  CARBON_CHECK(kind != SemIR::BuiltinInstKind::Invalid);
   auto type_id = GetTypeIdForTypeInst(SemIR::InstId::ForBuiltin(kind));
   // To keep client code simpler, complete builtin types before returning them.
   bool complete = TryToCompleteType(type_id);

toolchain/check/eval.cpp

@@ -1402,7 +1402,18 @@ static auto TryEvalInstInContext(EvalContext& eval_context,
     case SemIR::TupleInit::Kind:
       return RebuildInitAsValue(eval_context, inst, SemIR::TupleValue::Kind);
 
-    case SemIR::BuiltinInst::Kind:
+    case SemIR::AutoType::Kind:
+    case SemIR::BoolType::Kind:
+    case SemIR::BoundMethodType::Kind:
+    case SemIR::ErrorInst::Kind:
+    case SemIR::IntLiteralType::Kind:
+    case SemIR::LegacyFloatType::Kind:
+    case SemIR::NamespaceType::Kind:
+    case SemIR::SpecificFunctionType::Kind:
+    case SemIR::StringType::Kind:
+    case SemIR::TypeType::Kind:
+    case SemIR::VtableType::Kind:
+    case SemIR::WitnessType::Kind:
       // Builtins are always template constants.
       return MakeConstantResult(eval_context.context(), inst, Phase::Template);
 

toolchain/check/testdata/basics/builtin_insts.carbon

@@ -32,18 +32,18 @@
 // CHECK:STDOUT:   type_blocks:
 // CHECK:STDOUT:     type_block0:     {}
 // CHECK:STDOUT:   insts:
-// CHECK:STDOUT:     instTypeType:    {kind: BuiltinInst, arg0: TypeType, type: typeTypeType}
-// CHECK:STDOUT:     instErrorInst:   {kind: BuiltinInst, arg0: ErrorInst, type: typeError}
-// CHECK:STDOUT:     instAutoType:    {kind: BuiltinInst, arg0: AutoType, type: typeTypeType}
-// CHECK:STDOUT:     instBoolType:    {kind: BuiltinInst, arg0: BoolType, type: typeTypeType}
-// CHECK:STDOUT:     instIntLiteralType: {kind: BuiltinInst, arg0: IntLiteralType, type: typeTypeType}
-// CHECK:STDOUT:     instLegacyFloatType: {kind: BuiltinInst, arg0: LegacyFloatType, type: typeTypeType}
-// CHECK:STDOUT:     instStringType:  {kind: BuiltinInst, arg0: StringType, type: typeTypeType}
-// CHECK:STDOUT:     instBoundMethodType: {kind: BuiltinInst, arg0: BoundMethodType, type: typeTypeType}
-// CHECK:STDOUT:     instSpecificFunctionType: {kind: BuiltinInst, arg0: SpecificFunctionType, type: typeTypeType}
-// CHECK:STDOUT:     instNamespaceType: {kind: BuiltinInst, arg0: NamespaceType, type: typeTypeType}
-// CHECK:STDOUT:     instWitnessType: {kind: BuiltinInst, arg0: WitnessType, type: typeTypeType}
-// CHECK:STDOUT:     instVtableType:  {kind: BuiltinInst, arg0: VtableType, type: typeTypeType}
+// CHECK:STDOUT:     instTypeType:    {kind: TypeType, type: typeTypeType}
+// CHECK:STDOUT:     instErrorInst:   {kind: ErrorInst, type: typeError}
+// CHECK:STDOUT:     instAutoType:    {kind: AutoType, type: typeTypeType}
+// CHECK:STDOUT:     instBoolType:    {kind: BoolType, type: typeTypeType}
+// CHECK:STDOUT:     instIntLiteralType: {kind: IntLiteralType, type: typeTypeType}
+// CHECK:STDOUT:     instLegacyFloatType: {kind: LegacyFloatType, type: typeTypeType}
+// CHECK:STDOUT:     instStringType:  {kind: StringType, type: typeTypeType}
+// CHECK:STDOUT:     instBoundMethodType: {kind: BoundMethodType, type: typeTypeType}
+// CHECK:STDOUT:     instSpecificFunctionType: {kind: SpecificFunctionType, type: typeTypeType}
+// CHECK:STDOUT:     instNamespaceType: {kind: NamespaceType, type: typeTypeType}
+// CHECK:STDOUT:     instWitnessType: {kind: WitnessType, type: typeTypeType}
+// CHECK:STDOUT:     instVtableType:  {kind: VtableType, type: typeTypeType}
 // CHECK:STDOUT:     'inst+0':          {kind: Namespace, arg0: name_scope0, arg1: inst<invalid>, type: type(instNamespaceType)}
 // CHECK:STDOUT:   constant_values:
 // CHECK:STDOUT:     instTypeType:    templateConstant(instTypeType)

toolchain/docs/adding_features.md

@@ -293,9 +293,10 @@ If the resulting SemIR needs a new instruction:
         where types don't need to be distinct per-entity. This is rare, but
         used, for example, when an expression implicitly uses a value as part of
         SemIR evaluation or as part of desugaring. We have builtin types for
-        bound methods, namespaces, witnesses, among others. These are defined in
-        [`sem_ir/builtin_inst_kind.def`](/toolchain/sem_ir/builtin_inst_kind.def).
-        To get a type id for one of these builtin types, use something like
+        bound methods, namespaces, witnesses, among others. These are
+        constructed as a special-case in
+        [`File` construction](/toolchain/sem_ir/file.cpp). To get a type id for
+        one of these builtin types, use something like
         `context.GetBuiltinType(SemIR::BuiltinInstKind::WitnessType)`, as in:
 
         ```
@@ -334,7 +335,7 @@ need custom formatting, then a `FormatInstRHS` overload can be implemented
 instead.
 
 If the resulting SemIR needs a new built-in, add it to
-[`sem_ir/builtin_inst_kind.def`](/toolchain/sem_ir/builtin_inst_kind.def).
+[`File` construction](/toolchain/sem_ir/file.cpp).
 
 ### SemIR typed instruction metadata implementation
 

toolchain/lower/BUILD

@@ -51,10 +51,9 @@ cc_library(
         "//toolchain/check:sem_ir_diagnostic_converter",
         "//toolchain/sem_ir:entry_point",
         "//toolchain/sem_ir:file",
-        "//toolchain/sem_ir:ids",
         "//toolchain/sem_ir:inst",
-        "//toolchain/sem_ir:inst_kind",
         "//toolchain/sem_ir:inst_namer",
+        "//toolchain/sem_ir:typed_insts",
         "@llvm-project//llvm:Core",
         "@llvm-project//llvm:Support",
         "@llvm-project//llvm:TransformUtils",

toolchain/lower/file_context.cpp

@@ -460,54 +460,33 @@ auto FileContext::BuildDISubprogram(const SemIR::Function& function,
       llvm::DISubprogram::SPFlagDefinition);
 }
 
+// BuildTypeForInst is used to construct types for FileContext::BuildType below.
+// Implementations return the LLVM type for the instruction. This first overload
+// is the fallback handler for non-type instructions.
+template <typename InstT>
+  requires(InstT::Kind.is_type() == SemIR::InstIsType::Never)
+static auto BuildTypeForInst(FileContext& /*context*/, InstT inst)
+    -> llvm::Type* {
+  CARBON_FATAL("Cannot use inst as type: {0}", inst);
+}
+
 static auto BuildTypeForInst(FileContext& context, SemIR::ArrayType inst)
     -> llvm::Type* {
   return llvm::ArrayType::get(
       context.GetType(inst.element_type_id),
       context.sem_ir().GetArrayBoundValue(inst.bound_id));
 }
 
-static auto BuildTypeForInst(FileContext& context, SemIR::BuiltinInst inst)
+static auto BuildTypeForInst(FileContext& /*context*/, SemIR::AutoType inst)
     -> llvm::Type* {
-  switch (inst.builtin_inst_kind) {
-    case SemIR::BuiltinInstKind::Invalid:
-    case SemIR::BuiltinInstKind::AutoType:
-      CARBON_FATAL("Unexpected builtin type in lowering: {0}", inst);
-    case SemIR::BuiltinInstKind::ErrorInst:
-      // This is a complete type but uses of it should never be lowered.
-      return nullptr;
-    case SemIR::BuiltinInstKind::TypeType:
-      return context.GetTypeType();
-    case SemIR::BuiltinInstKind::LegacyFloatType:
-      return llvm::Type::getDoubleTy(context.llvm_context());
-    case SemIR::BuiltinInstKind::BoolType:
-      // TODO: We may want to have different representations for `bool`
-      // storage
-      // (`i8`) versus for `bool` values (`i1`).
-      return llvm::Type::getInt1Ty(context.llvm_context());
-    case SemIR::BuiltinInstKind::SpecificFunctionType:
-    case SemIR::BuiltinInstKind::StringType:
-      // TODO: Decide how we want to represent `StringType`.
-      return llvm::PointerType::get(context.llvm_context(), 0);
-    case SemIR::BuiltinInstKind::BoundMethodType:
-    case SemIR::BuiltinInstKind::IntLiteralType:
-    case SemIR::BuiltinInstKind::NamespaceType:
-    case SemIR::BuiltinInstKind::WitnessType:
-      // Return an empty struct as a placeholder.
-      return llvm::StructType::get(context.llvm_context());
-    case SemIR::BuiltinInstKind::VtableType:
-      return llvm::Type::getVoidTy(context.llvm_context());
-  }
+  CARBON_FATAL("Unexpected builtin type in lowering: {0}", inst);
 }
 
-// BuildTypeForInst is used to construct types for FileContext::BuildType below.
-// Implementations return the LLVM type for the instruction. This first overload
-// is the fallback handler for non-type instructions.
-template <typename InstT>
-  requires(InstT::Kind.is_type() == SemIR::InstIsType::Never)
-static auto BuildTypeForInst(FileContext& /*context*/, InstT inst)
+static auto BuildTypeForInst(FileContext& context, SemIR::BoolType /*inst*/)
     -> llvm::Type* {
-  CARBON_FATAL("Cannot use inst as type: {0}", inst);
+  // TODO: We may want to have different representations for `bool` storage
+  // (`i8`) versus for `bool` values (`i1`).
+  return llvm::Type::getInt1Ty(context.llvm_context());
 }
 
 static auto BuildTypeForInst(FileContext& context, SemIR::ClassType inst)
@@ -524,6 +503,12 @@ static auto BuildTypeForInst(FileContext& context, SemIR::ConstType inst)
   return context.GetType(inst.inner_id);
 }
 
+static auto BuildTypeForInst(FileContext& /*context*/,
+                             SemIR::ErrorInst /*inst*/) -> llvm::Type* {
+  // This is a complete type but uses of it should never be lowered.
+  return nullptr;
+}
+
 static auto BuildTypeForInst(FileContext& context, SemIR::FloatType /*inst*/)
     -> llvm::Type* {
   // TODO: Handle different sizes.
@@ -540,6 +525,11 @@ static auto BuildTypeForInst(FileContext& context, SemIR::IntType inst)
       context.sem_ir().ints().Get(width->int_id).getZExtValue());
 }
 
+static auto BuildTypeForInst(FileContext& context,
+                             SemIR::LegacyFloatType /*inst*/) -> llvm::Type* {
+  return llvm::Type::getDoubleTy(context.llvm_context());
+}
+
 static auto BuildTypeForInst(FileContext& context, SemIR::PointerType /*inst*/)
     -> llvm::Type* {
   return llvm::PointerType::get(context.llvm_context(), /*AddressSpace=*/0);
@@ -571,6 +561,35 @@ static auto BuildTypeForInst(FileContext& context, SemIR::TupleType inst)
   return llvm::StructType::get(context.llvm_context(), subtypes);
 }
 
+static auto BuildTypeForInst(FileContext& context, SemIR::TypeType /*inst*/)
+    -> llvm::Type* {
+  return context.GetTypeType();
+}
+
+static auto BuildTypeForInst(FileContext& context, SemIR::VtableType /*inst*/)
+    -> llvm::Type* {
+  return llvm::Type::getVoidTy(context.llvm_context());
+}
+
+template <typename InstT>
+  requires(InstT::Kind.template IsAnyOf<SemIR::SpecificFunctionType,
+                                        SemIR::StringType>())
+static auto BuildTypeForInst(FileContext& context, InstT /*inst*/)
+    -> llvm::Type* {
+  // TODO: Decide how we want to represent `StringType`.
+  return llvm::PointerType::get(context.llvm_context(), 0);
+}
+
+template <typename InstT>
+  requires(InstT::Kind
+               .template IsAnyOf<SemIR::BoundMethodType, SemIR::IntLiteralType,
+                                 SemIR::NamespaceType, SemIR::WitnessType>())
+static auto BuildTypeForInst(FileContext& context, InstT /*inst*/)
+    -> llvm::Type* {
+  // Return an empty struct as a placeholder.
+  return llvm::StructType::get(context.llvm_context());
+}
+
 template <typename InstT>
   requires(InstT::Kind.template IsAnyOf<
            SemIR::AssociatedEntityType, SemIR::FacetType, SemIR::FunctionType,

toolchain/lower/mangler.cpp

@@ -56,17 +56,27 @@ auto Mangler::MangleInverseQualifiedNameScope(llvm::raw_ostream& os,
         names_to_render.push_back(
             {.name_scope_id = interface.scope_id, .prefix = ':'});
 
-        CARBON_KIND_SWITCH(insts().Get(constant_values().GetConstantInstId(
-                               impl.self_id))) {
+        auto self_inst_id = constant_values().GetConstantInstId(impl.self_id);
+        CARBON_KIND_SWITCH(insts().Get(self_inst_id)) {
           case CARBON_KIND(SemIR::ClassType class_type): {
             auto next_name_scope_id =
                 sem_ir().classes().Get(class_type.class_id).scope_id;
             names_to_render.push_back(
                 {.name_scope_id = next_name_scope_id, .prefix = '\0'});
             break;
           }
-          case CARBON_KIND(SemIR::BuiltinInst builtin_inst): {
-            os << builtin_inst.builtin_inst_kind.label();
+          case SemIR::AutoType::Kind:
+          case SemIR::BoolType::Kind:
+          case SemIR::BoundMethodType::Kind:
+          case SemIR::IntLiteralType::Kind:
+          case SemIR::LegacyFloatType::Kind:
+          case SemIR::NamespaceType::Kind:
+          case SemIR::SpecificFunctionType::Kind:
+          case SemIR::StringType::Kind:
+          case SemIR::TypeType::Kind:
+          case SemIR::VtableType::Kind:
+          case SemIR::WitnessType::Kind: {
+            os << self_inst_id.builtin_inst_kind().label();
             break;
           }
           case CARBON_KIND(SemIR::IntType int_type): {