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,6 +460,16 @@ 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(
@@ -467,47 +477,16 @@ static auto BuildTypeForInst(FileContext& context, SemIR::ArrayType inst)
       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,8 +56,8 @@ 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;
@@ -65,8 +65,18 @@ auto Mangler::MangleInverseQualifiedNameScope(llvm::raw_ostream& os,
                 {.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): {

toolchain/sem_ir/BUILD

@@ -6,14 +6,6 @@ load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
 
 package(default_visibility = ["//visibility:public"])
 
-cc_library(
-    name = "builtin_inst_kind",
-    srcs = ["builtin_inst_kind.cpp"],
-    hdrs = ["builtin_inst_kind.h"],
-    textual_hdrs = ["builtin_inst_kind.def"],
-    deps = ["//common:enum_base"],
-)
-
 cc_library(
     name = "block_value_store",
     hdrs = ["block_value_store.h"],
@@ -27,28 +19,15 @@ cc_library(
 )
 
 cc_library(
-    name = "ids",
+    name = "typed_insts",
+    srcs = [
+        "builtin_inst_kind.cpp",
+        "inst_kind.cpp",
+    ],
     hdrs = [
+        "builtin_inst_kind.h",
         "id_kind.h",
         "ids.h",
-    ],
-    deps = [
-        "//common:check",
-        "//common:ostream",
-        "//toolchain/base:index_base",
-        "//toolchain/base:int",
-        "//toolchain/base:shared_value_stores",
-        "//toolchain/base:value_ids",
-        "//toolchain/diagnostics:diagnostic_emitter",
-        "//toolchain/parse:node_kind",
-        "//toolchain/sem_ir:builtin_inst_kind",
-    ],
-)
-
-cc_library(
-    name = "inst_kind",
-    srcs = ["inst_kind.cpp"],
-    hdrs = [
         "inst_kind.h",
         "typed_insts.h",
     ],
@@ -56,10 +35,12 @@ cc_library(
     deps = [
         "//common:check",
         "//common:enum_base",
+        "//common:ostream",
+        "//toolchain/base:index_base",
         "//toolchain/base:int",
+        "//toolchain/base:value_ids",
+        "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/parse:node_kind",
-        "//toolchain/sem_ir:builtin_inst_kind",
-        "//toolchain/sem_ir:ids",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -70,9 +51,7 @@ cc_library(
     hdrs = ["inst.h"],
     deps = [
         ":block_value_store",
-        ":builtin_inst_kind",
-        ":ids",
-        ":inst_kind",
+        ":typed_insts",
         "//common:check",
         "//common:hashing",
         "//common:ostream",
@@ -122,10 +101,8 @@ cc_library(
     ],
     deps = [
         ":block_value_store",
-        ":builtin_inst_kind",
-        ":ids",
         ":inst",
-        ":inst_kind",
+        ":typed_insts",
         "//common:check",
         "//common:enum_base",
         "//common:error",
@@ -151,8 +128,7 @@ cc_library(
     hdrs = ["stringify_type.h"],
     deps = [
         ":file",
-        ":ids",
-        ":inst_kind",
+        ":typed_insts",
         "//common:check",
         "//toolchain/base:kind_switch",
         "@llvm-project//llvm:Support",
@@ -165,8 +141,7 @@ cc_library(
     hdrs = ["inst_namer.h"],
     deps = [
         ":file",
-        ":ids",
-        ":inst_kind",
+        ":typed_insts",
         "//common:ostream",
         "//toolchain/base:kind_switch",
         "//toolchain/base:shared_value_stores",
@@ -182,9 +157,8 @@ cc_library(
     hdrs = ["formatter.h"],
     deps = [
         ":file",
-        ":ids",
-        ":inst_kind",
         ":inst_namer",
+        ":typed_insts",
         "//common:ostream",
         "//toolchain/base:kind_switch",
         "//toolchain/base:shared_value_stores",
@@ -210,7 +184,7 @@ cc_test(
     srcs = ["typed_insts_test.cpp"],
     deps = [
         ":inst",
-        ":inst_kind",
+        ":typed_insts",
         "//testing/base:gtest_main",
         "@googletest//:gtest",
     ],

toolchain/sem_ir/builtin_inst_kind.cpp

@@ -7,15 +7,15 @@
 namespace Carbon::SemIR {
 
 CARBON_DEFINE_ENUM_CLASS_NAMES(BuiltinInstKind) = {
-#define CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name) \
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) \
   CARBON_ENUM_CLASS_NAME_STRING(Name)
-#include "toolchain/sem_ir/builtin_inst_kind.def"
+#include "toolchain/sem_ir/inst_kind.def"
 };
 
 auto BuiltinInstKind::label() -> llvm::StringRef {
   static constexpr llvm::StringLiteral Labels[] = {
 #define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, Label) Label,
-#include "toolchain/sem_ir/builtin_inst_kind.def"
+#include "toolchain/sem_ir/inst_kind.def"
   };
   return Labels[AsInt()];
 }

toolchain/sem_ir/builtin_inst_kind.def

@@ -1,95 +0,0 @@
-// 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
-//
-// This is an X-macro header. It does not use `#include` guards, and instead is
-// designed to be `#include`ed after the x-macro is defined in order for its
-// inclusion to expand to the desired output. Macro definitions are cleaned up
-// at the end of this file.
-//
-// Supported x-macros are:
-// - CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name)
-//   Used as a fallback if other macros are missing. Used directly by Invalid
-//   only, which is defined last.
-//   - CARBON_SEM_IR_BUILTIN_INST_KIND(Name, Label)
-//     Defines a non-Invalid builtin type. The label is used for stringifying
-//     types.
-//
-// This tree represents the subset relationship between these macros, where if a
-// specific x-macro isn't defined, it'll fall back to the parent macro.
-
-#if !(defined(CARBON_SEM_IR_BUILTIN_INST_KIND_NAME) || \
-      defined(CARBON_SEM_IR_BUILTIN_INST_KIND))
-#error \
-    "Must define CARBON_SEM_IR_BUILTIN_INST_KIND family x-macros to use this file."
-#endif
-
-// If CARBON_SEM_IR_BUILTIN_INST_KIND_NAME is undefined, ignore calls.
-#ifndef CARBON_SEM_IR_BUILTIN_INST_KIND_NAME
-#define CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name)
-#endif
-
-// If CARBON_SEM_IR_BUILTIN_INST_KIND is undefined, delegate calls to
-// CARBON_SEM_IR_BUILTIN_INST_KIND_NAME.
-#ifndef CARBON_SEM_IR_BUILTIN_INST_KIND
-#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) \
-  CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name)
-#endif
-
-// Tracks expressions which are valid as types.
-// This has a deliberately self-referential type.
-CARBON_SEM_IR_BUILTIN_INST_KIND(TypeType, "type")
-
-// Used when a semantic error has been detected, and a SemIR InstId is still
-// required. For example, when there is a type checking issue, this will be used
-// in the type_id. It's typically used as a cue that semantic checking doesn't
-// need to issue further diagnostics.
-CARBON_SEM_IR_BUILTIN_INST_KIND(ErrorInst, "<error>")
-
-// Used for the type of patterns that do not match a fixed type.
-CARBON_SEM_IR_BUILTIN_INST_KIND(AutoType, "auto")
-
-// -----------------------------------------------------------------------------
-// TODO: Below types are all placeholders. While the above may last, the below
-// are expected to need to change in order to better reflect Carbon's design.
-// Keeping distinct placeholders can help find usages for later fixes.
-// -----------------------------------------------------------------------------
-
-// The type of bool literals and branch conditions, bool.
-CARBON_SEM_IR_BUILTIN_INST_KIND(BoolType, "bool")
-
-// An arbitrary-precision integer type, which is used as the type of integer
-// literals and as the parameter type of `Core.Int` and `Core.Float`. This type
-// only provides compile-time operations, and is represented as an empty type at
-// runtime.
-CARBON_SEM_IR_BUILTIN_INST_KIND(IntLiteralType, "Core.IntLiteral")
-
-// The legacy float type. This is currently used for real literals, and is
-// treated as f64. It's separate from `FloatType`, and should change to mirror
-// integers, likely replacing this with a `FloatLiteralType`.
-CARBON_SEM_IR_BUILTIN_INST_KIND(LegacyFloatType, "f64")
-
-// The type of string values and String literals.
-CARBON_SEM_IR_BUILTIN_INST_KIND(StringType, "String")
-
-// The type of bound method values.
-CARBON_SEM_IR_BUILTIN_INST_KIND(BoundMethodType, "<bound method>")
-
-// The type of specific functions.
-CARBON_SEM_IR_BUILTIN_INST_KIND(SpecificFunctionType, "<specific function>")
-
-// The type of namespace and imported package names.
-CARBON_SEM_IR_BUILTIN_INST_KIND(NamespaceType, "<namespace>")
-
-// The type of witnesses.
-CARBON_SEM_IR_BUILTIN_INST_KIND(WitnessType, "<witness>")
-
-// The type of virtual function tables
-CARBON_SEM_IR_BUILTIN_INST_KIND(VtableType, "<vtable>")
-
-// Keep invalid last, so that we can use values as array indices without needing
-// an invalid entry.
-CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Invalid)
-
-#undef CARBON_SEM_IR_BUILTIN_INST_KIND_NAME
-#undef CARBON_SEM_IR_BUILTIN_INST_KIND

toolchain/sem_ir/builtin_inst_kind.h

@@ -12,35 +12,33 @@
 namespace Carbon::SemIR {
 
 CARBON_DEFINE_RAW_ENUM_CLASS(BuiltinInstKind, uint8_t) {
-#define CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name) \
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) \
   CARBON_RAW_ENUM_ENUMERATOR(Name)
-#include "toolchain/sem_ir/builtin_inst_kind.def"
+#include "toolchain/sem_ir/inst_kind.def"
 };
 
 class BuiltinInstKind : public CARBON_ENUM_BASE(BuiltinInstKind) {
  public:
-#define CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name) \
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) \
   CARBON_ENUM_CONSTANT_DECL(Name)
-#include "toolchain/sem_ir/builtin_inst_kind.def"
+#include "toolchain/sem_ir/inst_kind.def"
 
   auto label() -> llvm::StringRef;
 
   // The count of enum values excluding Invalid.
-  //
-  // Note that we *define* this as `constexpr` making it a true compile-time
-  // constant.
-  static const uint8_t ValidCount;
+  static constexpr uint8_t ValidCount = 0
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) +1
+#include "toolchain/sem_ir/inst_kind.def"
+      ;
 
   // Support conversion to and from an int32_t for SemIR instruction storage.
   using EnumBase::AsInt;
   using EnumBase::FromInt;
 };
 
-#define CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name) \
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) \
   CARBON_ENUM_CONSTANT_DEFINITION(BuiltinInstKind, Name)
-#include "toolchain/sem_ir/builtin_inst_kind.def"
-
-constexpr uint8_t BuiltinInstKind::ValidCount = Invalid.AsInt();
+#include "toolchain/sem_ir/inst_kind.def"
 
 static_assert(
     BuiltinInstKind::ValidCount != 0,

toolchain/sem_ir/file.cpp

@@ -44,12 +44,11 @@ File::File(CheckIRId check_ir_id, IdentifierId package_id,
 // a normal type. Every other builtin is a type, including the
 // self-referential TypeType.
 #define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...)                    \
-  insts_.AddInNoBlock(LocIdAndInst::NoLoc<BuiltinInst>(               \
+  insts_.AddInNoBlock(LocIdAndInst::NoLoc<Name>(                      \
       {.type_id = BuiltinInstKind::Name == BuiltinInstKind::ErrorInst \
                       ? TypeId::Error                                 \
-                      : TypeId::TypeType,                             \
-       .builtin_inst_kind = BuiltinInstKind::Name}));
-#include "toolchain/sem_ir/builtin_inst_kind.def"
+                      : TypeId::TypeType}));
+#include "toolchain/sem_ir/inst_kind.def"
   CARBON_CHECK(insts_.size() == BuiltinInstKind::ValidCount,
                "Builtins should produce {0} insts, actual: {1}",
                BuiltinInstKind::ValidCount, insts_.size());
@@ -249,11 +248,14 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory {
       case AssociatedConstantDecl::Kind:
       case AssociatedEntity::Kind:
       case AssociatedEntityType::Kind:
+      case AutoType::Kind:
       case BindSymbolicName::Kind:
       case BindValue::Kind:
       case BlockArg::Kind:
       case BoolLiteral::Kind:
+      case BoolType::Kind:
       case BoundMethod::Kind:
+      case BoundMethodType::Kind:
       case ClassDecl::Kind:
       case ClassType::Kind:
       case CompleteTypeWitness::Kind:
@@ -266,33 +268,37 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory {
       case GenericClassType::Kind:
       case GenericInterfaceType::Kind:
       case ImportDecl::Kind:
+      case IntLiteralType::Kind:
+      case IntType::Kind:
+      case IntValue::Kind:
       case InterfaceDecl::Kind:
       case InterfaceWitness::Kind:
       case InterfaceWitnessAccess::Kind:
-      case IntValue::Kind:
-      case IntType::Kind:
+      case LegacyFloatType::Kind:
+      case NamespaceType::Kind:
       case PointerType::Kind:
       case SpecificFunction::Kind:
+      case SpecificFunctionType::Kind:
       case StringLiteral::Kind:
-      case StructValue::Kind:
+      case StringType::Kind:
       case StructType::Kind:
+      case StructValue::Kind:
       case SymbolicBindingPattern::Kind:
-      case TupleValue::Kind:
       case TupleType::Kind:
+      case TupleValue::Kind:
+      case TypeType::Kind:
       case UnaryOperatorNot::Kind:
       case UnboundElementType::Kind:
       case ValueOfInitializer::Kind:
       case ValueParam::Kind:
       case ValueParamPattern::Kind:
+      case VtableType::Kind:
       case WhereExpr::Kind:
+      case WitnessType::Kind:
         return value_category;
 
-      case CARBON_KIND(BuiltinInst inst): {
-        if (inst.builtin_inst_kind == BuiltinInstKind::ErrorInst) {
-          return ExprCategory::Error;
-        }
-        return value_category;
-      }
+      case ErrorInst::Kind:
+        return ExprCategory::Error;
 
       case CARBON_KIND(BindName inst): {
         // TODO: Don't rely on value_id for expression category, since it may

toolchain/sem_ir/id_kind.h

@@ -116,8 +116,6 @@ class TypeEnum {
 
 // An enum of all the ID types used as instruction operands.
 using IdKind = TypeEnum<
-    // From sem_ir/builtin_inst_kind.h.
-    BuiltinInstKind,
     // From base/value_store.h.
     IntId, RealId, FloatId, StringLiteralValueId,
     // From sem_ir/id.h.

toolchain/sem_ir/ids.h

@@ -39,10 +39,10 @@ struct InstId : public IdBase, public Printable<InstId> {
   // An explicitly invalid ID.
   static const InstId Invalid;
 
-// BuiltinInst IDs.
-#define CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name) \
+// Builtin inst IDs.
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) \
   static const InstId Builtin##Name;
-#include "toolchain/sem_ir/builtin_inst_kind.def"
+#include "toolchain/sem_ir/inst_kind.def"
 
   // The namespace for a `package` expression.
   static const InstId PackageNamespace;
@@ -83,10 +83,10 @@ struct InstId : public IdBase, public Printable<InstId> {
 
 constexpr InstId InstId::Invalid = InstId(InvalidIndex);
 
-#define CARBON_SEM_IR_BUILTIN_INST_KIND_NAME(Name) \
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) \
   constexpr InstId InstId::Builtin##Name =         \
       InstId::ForBuiltin(BuiltinInstKind::Name);
-#include "toolchain/sem_ir/builtin_inst_kind.def"
+#include "toolchain/sem_ir/inst_kind.def"
 
 // An ID of an instruction that is referenced absolutely by another instruction.
 // This should only be used as the type of a field within a typed instruction

toolchain/sem_ir/inst_kind.def

@@ -10,11 +10,84 @@
 // This macro should be defined before including this header:
 // - CARBON_SEM_IR_INST_KIND(Name)
 //   Invoked for each kind of semantic instruction.
+//
+// Temporarily, we have CARBON_SEM_IR_BUILTIN_INST_KIND too:
+//
+// - CARBON_SEM_IR_BUILTIN_INST_KIND(Name, Label)
+//   Defines a non-Invalid builtin type. The label is used for stringifying
+//   types. Falls back to CARBON_SEM_IR_INST_KIND if not defined.
+//
+// TODO: Merge builtin instructions into the standard CARBON_SEM_IR_INST_KIND,
+// tracking the "builtin" annotation separately. This approach is used for
+// legacy compatibility.
+
+// If CARBON_SEM_IR_BUILTIN_INST_KIND is missing, default to
+// CARBON_SEM_IR_INST_KIND. However, if it's provided, make
+// CARBON_SEM_IR_INST_KIND optional. Per the above TODO, this is temporary.
+#ifndef CARBON_SEM_IR_BUILTIN_INST_KIND
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, ...) CARBON_SEM_IR_INST_KIND(Name)
+#else
+#ifndef CARBON_SEM_IR_INST_KIND
+#define CARBON_SEM_IR_INST_KIND(Name)
+#endif
+#endif
 
 #ifndef CARBON_SEM_IR_INST_KIND
 #error "Must define the x-macro to use this file."
 #endif
 
+// Tracks expressions which are valid as types.
+// This has a deliberately self-referential type.
+CARBON_SEM_IR_BUILTIN_INST_KIND(TypeType, "type")
+
+// Used when a semantic error has been detected, and a SemIR InstId is still
+// required. For example, when there is a type checking issue, this will be used
+// in the type_id. It's typically used as a cue that semantic checking doesn't
+// need to issue further diagnostics.
+CARBON_SEM_IR_BUILTIN_INST_KIND(ErrorInst, "<error>")
+
+// Used for the type of patterns that do not match a fixed type.
+CARBON_SEM_IR_BUILTIN_INST_KIND(AutoType, "auto")
+
+// -----------------------------------------------------------------------------
+// TODO: Below CARBON_SEM_IR_BUILTIN_INST_KIND types are all placeholders. While
+// the above may last, the below are expected to need to change in order to
+// better reflect Carbon's design. Keeping distinct placeholders can help find
+// usages for later fixes.
+// -----------------------------------------------------------------------------
+
+// The type of bool literals and branch conditions, bool.
+CARBON_SEM_IR_BUILTIN_INST_KIND(BoolType, "bool")
+
+// An arbitrary-precision integer type, which is used as the type of integer
+// literals and as the parameter type of `Core.Int` and `Core.Float`. This type
+// only provides compile-time operations, and is represented as an empty type at
+// runtime.
+CARBON_SEM_IR_BUILTIN_INST_KIND(IntLiteralType, "Core.IntLiteral")
+
+// The legacy float type. This is currently used for real literals, and is
+// treated as f64. It's separate from `FloatType`, and should change to mirror
+// integers, likely replacing this with a `FloatLiteralType`.
+CARBON_SEM_IR_BUILTIN_INST_KIND(LegacyFloatType, "f64")
+
+// The type of string values and String literals.
+CARBON_SEM_IR_BUILTIN_INST_KIND(StringType, "String")
+
+// The type of bound method values.
+CARBON_SEM_IR_BUILTIN_INST_KIND(BoundMethodType, "<bound method>")
+
+// The type of specific functions.
+CARBON_SEM_IR_BUILTIN_INST_KIND(SpecificFunctionType, "<specific function>")
+
+// The type of namespace and imported package names.
+CARBON_SEM_IR_BUILTIN_INST_KIND(NamespaceType, "<namespace>")
+
+// The type of witnesses.
+CARBON_SEM_IR_BUILTIN_INST_KIND(WitnessType, "<witness>")
+
+// The type of virtual function tables
+CARBON_SEM_IR_BUILTIN_INST_KIND(VtableType, "<vtable>")
+
 // For each instruction kind declared here there is a matching definition in
 // `typed_insts.h`.
 CARBON_SEM_IR_INST_KIND(AdaptDecl)
@@ -41,7 +114,6 @@ CARBON_SEM_IR_INST_KIND(BoundMethod)
 CARBON_SEM_IR_INST_KIND(Branch)
 CARBON_SEM_IR_INST_KIND(BranchIf)
 CARBON_SEM_IR_INST_KIND(BranchWithArg)
-CARBON_SEM_IR_INST_KIND(BuiltinInst)
 CARBON_SEM_IR_INST_KIND(Call)
 CARBON_SEM_IR_INST_KIND(ClassDecl)
 CARBON_SEM_IR_INST_KIND(ClassElementAccess)
@@ -109,3 +181,4 @@ CARBON_SEM_IR_INST_KIND(VarStorage)
 CARBON_SEM_IR_INST_KIND(WhereExpr)
 
 #undef CARBON_SEM_IR_INST_KIND
+#undef CARBON_SEM_IR_BUILTIN_INST_KIND

toolchain/sem_ir/stringify_type.cpp

@@ -339,17 +339,19 @@ auto StringifyTypeExpr(const SemIR::File& outer_sem_ir, InstId outer_inst_id)
       case Assign::Kind:
       case AssociatedConstantDecl::Kind:
       case AssociatedEntity::Kind:
+      case AutoType::Kind:
       case BaseDecl::Kind:
-      case BindingPattern::Kind:
       case BindName::Kind:
       case BindValue::Kind:
+      case BindingPattern::Kind:
       case BlockArg::Kind:
       case BoolLiteral::Kind:
+      case BoolType::Kind:
       case BoundMethod::Kind:
+      case BoundMethodType::Kind:
       case Branch::Kind:
       case BranchIf::Kind:
       case BranchWithArg::Kind:
-      case BuiltinInst::Kind:
       case Call::Kind:
       case ClassDecl::Kind:
       case ClassElementAccess::Kind:
@@ -357,6 +359,7 @@ auto StringifyTypeExpr(const SemIR::File& outer_sem_ir, InstId outer_inst_id)
       case CompleteTypeWitness::Kind:
       case Converted::Kind:
       case Deref::Kind:
+      case ErrorInst::Kind:
       case FieldDecl::Kind:
       case FloatLiteral::Kind:
       case FunctionDecl::Kind:
@@ -365,12 +368,14 @@ auto StringifyTypeExpr(const SemIR::File& outer_sem_ir, InstId outer_inst_id)
       case ImportRefLoaded::Kind:
       case ImportRefUnloaded::Kind:
       case InitializeFrom::Kind:
-      case SpecificConstant::Kind:
+      case IntLiteralType::Kind:
+      case IntValue::Kind:
       case InterfaceDecl::Kind:
       case InterfaceWitness::Kind:
       case InterfaceWitnessAccess::Kind:
-      case IntValue::Kind:
+      case LegacyFloatType::Kind:
       case Namespace::Kind:
+      case NamespaceType::Kind:
       case OutParam::Kind:
       case OutParamPattern::Kind:
       case RequirementEquivalent::Kind:
@@ -380,26 +385,32 @@ auto StringifyTypeExpr(const SemIR::File& outer_sem_ir, InstId outer_inst_id)
       case ReturnExpr::Kind:
       case ReturnSlot::Kind:
       case ReturnSlotPattern::Kind:
+      case SpecificConstant::Kind:
       case SpecificFunction::Kind:
+      case SpecificFunctionType::Kind:
       case SpliceBlock::Kind:
       case StringLiteral::Kind:
+      case StringType::Kind:
       case StructAccess::Kind:
-      case StructLiteral::Kind:
       case StructInit::Kind:
+      case StructLiteral::Kind:
       case StructValue::Kind:
       case SymbolicBindingPattern::Kind:
       case Temporary::Kind:
       case TemporaryStorage::Kind:
       case TupleAccess::Kind:
-      case TupleLiteral::Kind:
       case TupleInit::Kind:
+      case TupleLiteral::Kind:
       case TupleValue::Kind:
+      case TypeType::Kind:
       case UnaryOperatorNot::Kind:
       case ValueAsRef::Kind:
       case ValueOfInitializer::Kind:
       case ValueParam::Kind:
       case ValueParamPattern::Kind:
       case VarStorage::Kind:
+      case VtableType::Kind:
+      case WitnessType::Kind:
         // We don't know how to print this instruction, but it might have a
         // constant value that we can print.
         auto const_inst_id =

toolchain/sem_ir/type.h

@@ -49,13 +49,8 @@ class TypeStore : public Yaml::Printable<TypeStore> {
   // to be a particular kind of instruction.
   template <typename InstT>
   auto GetAs(TypeId type_id) const -> InstT {
-    if constexpr (std::is_same_v<InstT, BuiltinInst>) {
-      return GetAsInst(type_id).As<InstT>();
-    } else {
-      // The type is not a builtin, so no need to check for special values.
-      auto inst_id = constants_->GetInstId(GetConstantId(type_id));
-      return insts_->GetAs<InstT>(inst_id);
-    }
+    auto inst_id = constants_->GetInstId(GetConstantId(type_id));
+    return insts_->GetAs<InstT>(inst_id);
   }
 
   // Returns the instruction used to define the specified type, if it is of a

toolchain/sem_ir/typed_insts.h

@@ -45,6 +45,20 @@
 
 namespace Carbon::SemIR {
 
+// A builtin instruction, corresponding to instructions like
+// InstId::BuiltinTypeType.
+//
+// Builtins don't have a parse node associated with them.
+#define CARBON_SEM_IR_BUILTIN_INST_KIND(Name, Label)                          \
+  struct Name {                                                               \
+    static constexpr auto Kind = InstKind::Name.Define<Parse::InvalidNodeId>( \
+        {.ir_name = Label,                                                    \
+         .is_type = InstIsType::Always,                                       \
+         .constant_kind = InstConstantKind::Always});                         \
+    TypeId type_id;                                                           \
+  };
+#include "toolchain/sem_ir/inst_kind.def"
+
 // An adapted type declaration in a class, of the form `adapt T;`.
 struct AdaptDecl {
   static constexpr auto Kind = InstKind::AdaptDecl.Define<Parse::AdaptDeclId>(
@@ -401,20 +415,6 @@ struct BranchWithArg {
   InstId arg_id;
 };
 
-// A builtin instruction, corresponding to instructions like
-// InstId::BuiltinTypeType.
-struct BuiltinInst {
-  // Builtins don't have a parse node associated with them.
-  static constexpr auto Kind =
-      InstKind::BuiltinInst.Define<Parse::InvalidNodeId>(
-          {.ir_name = "builtin",
-           .is_type = InstIsType::Always,
-           .constant_kind = InstConstantKind::Always});
-
-  TypeId type_id;
-  BuiltinInstKind builtin_inst_kind;
-};
-
 // An abstract `callee(args)` call, where the callee may be a function, but
 // could also be a generic or other callable structure.
 struct Call {