瀏覽代碼

Represent rewrite constraints in `FacetTypeInfo` with `InstId` not `ConstantId`. (#5281)

This follows the pattern used elsewhere, and allows facet types in eval
blocks to directly reference their operands instead of doing so
indirectly via a `ConstantId` attached to the generic. This prepares us
for making `ConstantId`s always be unattached.

In passing, add a stringified version of the `InstId` to diagnostics in
a couple of places where it seems useful.
Richard Smith 1 年之前
父節點
當前提交
a91752de60

+ 36 - 30
toolchain/check/eval.cpp

@@ -118,8 +118,6 @@ class EvalContext {
         //    GetConstantValueInSpecific.
         // 2) From GetConstantValue(TypeId): for these, we could change
         //    instructions so they store InstIds instead of TypeIds.
-        // 3) From GetConstantFacetTypeInfo: for these, we could store an
-        //    InstId instead of a ConstantId in rewrite_constraints.
         return GetConstantInSpecific(sem_ir(), specific_id_, const_id);
       }
     }
@@ -272,21 +270,6 @@ static auto LatestPhase(Phase a, Phase b) -> Phase {
       std::max(static_cast<uint8_t>(a), static_cast<uint8_t>(b)));
 }
 
-// `where` expressions using `.Self` should not be considered symbolic
-// - `Interface where .Self impls I and .A = bool` -> concrete
-// - `T:! type` ... `Interface where .A = T` -> symbolic, since uses `T` which
-//   is symbolic and not due to `.Self`.
-static auto UpdatePhaseIgnorePeriodSelf(EvalContext& eval_context,
-                                        SemIR::ConstantId constant_id,
-                                        Phase* phase) {
-  Phase constant_phase = GetPhase(eval_context.constant_values(), constant_id);
-  // Since LatestPhase(x, Phase::Concrete) == x, this is equivalent to replacing
-  // Phase::PeriodSelfSymbolic with Phase::Concrete.
-  if (constant_phase != Phase::PeriodSelfSymbolic) {
-    *phase = LatestPhase(*phase, constant_phase);
-  }
-}
-
 // Forms a `constant_id` describing a given evaluation result.
 static auto MakeConstantResult(Context& context, SemIR::Inst inst, Phase phase)
     -> SemIR::ConstantId {
@@ -382,6 +365,30 @@ static auto GetConstantValue(EvalContext& eval_context, SemIR::InstId inst_id,
   return eval_context.constant_values().GetInstId(const_id);
 }
 
+// If the given instruction is constant, returns its constant value. When
+// determining the phase of the result, ignore any dependence on `.Self`.
+//
+// This is used when evaluating facet types, for which `where` expressions using
+// `.Self` should not be considered symbolic
+// - `Interface where .Self impls I and .A = bool` -> concrete
+// - `T:! type` ... `Interface where .A = T` -> symbolic, since uses `T` which
+//   is symbolic and not due to `.Self`.
+static auto GetConstantValueIgnoringPeriodSelf(EvalContext& eval_context,
+                                               SemIR::InstId inst_id,
+                                               Phase* phase) -> SemIR::InstId {
+  if (!inst_id.has_value()) {
+    return SemIR::InstId::None;
+  }
+  auto const_id = eval_context.GetConstantValue(inst_id);
+  Phase constant_phase = GetPhase(eval_context.constant_values(), const_id);
+  // Since LatestPhase(x, Phase::Concrete) == x, this is equivalent to replacing
+  // Phase::PeriodSelfSymbolic with Phase::Concrete.
+  if (constant_phase != Phase::PeriodSelfSymbolic) {
+    *phase = LatestPhase(*phase, constant_phase);
+  }
+  return eval_context.constant_values().GetInstId(const_id);
+}
+
 // Find the instruction that the given instruction instantiates to, and return
 // that.
 static auto GetConstantValue(EvalContext& eval_context,
@@ -608,13 +615,12 @@ static auto GetConstantFacetTypeInfo(EvalContext& eval_context,
   }
   info.rewrite_constraints.reserve(orig.rewrite_constraints.size());
   for (const auto& rewrite : orig.rewrite_constraints) {
-    auto lhs_const_id = eval_context.GetInContext(rewrite.lhs_const_id);
-    auto rhs_const_id = eval_context.GetInContext(rewrite.rhs_const_id);
-    // `where` requirements using `.Self` should not be considered symbolic
-    UpdatePhaseIgnorePeriodSelf(eval_context, lhs_const_id, phase);
-    UpdatePhaseIgnorePeriodSelf(eval_context, rhs_const_id, phase);
-    info.rewrite_constraints.push_back(
-        {.lhs_const_id = lhs_const_id, .rhs_const_id = rhs_const_id});
+    // `where` requirements using `.Self` should not be considered symbolic.
+    auto lhs_id =
+        GetConstantValueIgnoringPeriodSelf(eval_context, rewrite.lhs_id, phase);
+    auto rhs_id =
+        GetConstantValueIgnoringPeriodSelf(eval_context, rewrite.rhs_id, phase);
+    info.rewrite_constraints.push_back({.lhs_id = lhs_id, .rhs_id = rhs_id});
   }
   // TODO: Process other requirements.
   info.other_requirements = orig.other_requirements;
@@ -1887,14 +1893,14 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
       if (auto rewrite =
               eval_context.insts().TryGetAs<SemIR::RequirementRewrite>(
                   inst_id)) {
-        SemIR::ConstantId lhs = eval_context.GetConstantValue(rewrite->lhs_id);
-        SemIR::ConstantId rhs = eval_context.GetConstantValue(rewrite->rhs_id);
         // `where` requirements using `.Self` should not be considered
-        // symbolic
-        UpdatePhaseIgnorePeriodSelf(eval_context, lhs, &phase);
-        UpdatePhaseIgnorePeriodSelf(eval_context, rhs, &phase);
+        // symbolic.
+        auto lhs_id = GetConstantValueIgnoringPeriodSelf(
+            eval_context, rewrite->lhs_id, &phase);
+        auto rhs_id = GetConstantValueIgnoringPeriodSelf(
+            eval_context, rewrite->rhs_id, &phase);
         info.rewrite_constraints.push_back(
-            {.lhs_const_id = lhs, .rhs_const_id = rhs});
+            {.lhs_id = lhs_id, .rhs_id = rhs_id});
       } else if (auto impls =
                      eval_context.insts().TryGetAs<SemIR::RequirementImpls>(
                          inst_id)) {

+ 19 - 14
toolchain/check/facet_type.cpp

@@ -121,8 +121,8 @@ auto InitialFacetTypeImplWitness(
   }
 
   for (auto rewrite : facet_type_info.rewrite_constraints) {
-    auto inst_id = context.constant_values().GetInstId(rewrite.lhs_const_id);
-    auto access = context.insts().GetAs<SemIR::ImplWitnessAccess>(inst_id);
+    auto access =
+        context.insts().GetAs<SemIR::ImplWitnessAccess>(rewrite.lhs_id);
     if (!WitnessAccessMatchesInterface(context, access.witness_id,
                                        interface_to_witness)) {
       continue;
@@ -134,8 +134,8 @@ auto InitialFacetTypeImplWitness(
       // for it to use to attempt recovery.
       continue;
     }
-    auto rewrite_value = rewrite.rhs_const_id;
-    if (rewrite_value == SemIR::ErrorInst::SingletonConstantId) {
+    auto rewrite_value = rewrite.rhs_id;
+    if (rewrite_value == SemIR::ErrorInst::SingletonInstId) {
       table_entry = SemIR::ErrorInst::SingletonInstId;
       continue;
     }
@@ -165,7 +165,7 @@ auto InitialFacetTypeImplWitness(
     }
 
     if (table_entry != SemIR::ImplWitnessTablePlaceholder::SingletonInstId) {
-      if (context.constant_values().Get(table_entry) != rewrite_value) {
+      if (table_entry != rewrite_value) {
         // TODO: Figure out how to print the two different values
         // `const_id` & `rewrite_value` in the diagnostic
         // message.
@@ -196,14 +196,16 @@ auto InitialFacetTypeImplWitness(
       // forming the facet type because the type of the associated constant
       // was symbolic.
       auto converted_inst_id = ConvertToValueOfType(
-          context, context.insts().GetLocId(facet_type_inst_id),
-          context.constant_values().GetInstId(rewrite_value),
+          context, context.insts().GetLocId(facet_type_inst_id), rewrite_value,
           assoc_const_type_id);
-      rewrite_value = context.constant_values().Get(converted_inst_id);
+      // Canonicalize the converted constant value.
+      converted_inst_id =
+          context.constant_values().GetConstantInstId(converted_inst_id);
       // The result of conversion can be non-constant even if the original
       // value was constant.
-      if (!rewrite_value.is_constant() &&
-          rewrite_value != SemIR::ErrorInst::SingletonConstantId) {
+      if (converted_inst_id.has_value()) {
+        rewrite_value = converted_inst_id;
+      } else if (rewrite_value != SemIR::ErrorInst::SingletonInstId) {
         const auto& assoc_const = context.associated_constants().Get(
             assoc_constant_decl->assoc_const_id);
         CARBON_DIAGNOSTIC(
@@ -214,15 +216,18 @@ auto InitialFacetTypeImplWitness(
         context.emitter().Emit(facet_type_inst_id,
                                AssociatedConstantNotConstantAfterConversion,
                                assoc_const.name_id, assoc_const_type_id);
-        rewrite_value = SemIR::ErrorInst::SingletonConstantId;
+        rewrite_value = SemIR::ErrorInst::SingletonInstId;
       }
     }
 
-    auto rewrite_inst_id = context.constant_values().GetInstId(rewrite_value);
+    CARBON_CHECK(rewrite_value ==
+                     context.constant_values().GetConstantInstId(rewrite_value),
+                 "Rewritten value for associated constant is not canonical.");
+
     table_entry = AddInst<SemIR::ImplWitnessAssociatedConstant>(
         context, witness_loc_id,
-        {.type_id = context.insts().Get(rewrite_inst_id).type_id(),
-         .inst_id = rewrite_inst_id});
+        {.type_id = context.insts().Get(rewrite_value).type_id(),
+         .inst_id = rewrite_value});
   }
   return witness_inst_id;
 }

+ 4 - 4
toolchain/check/import_ref.cpp

@@ -2463,8 +2463,8 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
     GetLocalSpecificInterfaceData(resolver, interface);
   }
   for (auto rewrite : import_facet_type_info.rewrite_constraints) {
-    GetLocalConstantId(resolver, rewrite.lhs_const_id);
-    GetLocalConstantId(resolver, rewrite.rhs_const_id);
+    GetLocalConstantInstId(resolver, rewrite.lhs_id);
+    GetLocalConstantInstId(resolver, rewrite.rhs_id);
   }
   if (resolver.HasNewWork()) {
     return ResolveResult::Retry();
@@ -2490,8 +2490,8 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
       import_facet_type_info.rewrite_constraints.size());
   for (auto rewrite : import_facet_type_info.rewrite_constraints) {
     local_facet_type_info.rewrite_constraints.push_back(
-        {.lhs_const_id = GetLocalConstantId(resolver, rewrite.lhs_const_id),
-         .rhs_const_id = GetLocalConstantId(resolver, rewrite.rhs_const_id)});
+        {.lhs_id = GetLocalConstantInstId(resolver, rewrite.lhs_id),
+         .rhs_id = GetLocalConstantInstId(resolver, rewrite.rhs_id)});
   }
   // TODO: Also process the other requirements.
   SemIR::FacetTypeId facet_type_id =

+ 5 - 9
toolchain/check/subst.cpp

@@ -129,12 +129,8 @@ static auto PushOperand(Context& context, Worklist& worklist,
         push_specific(interface.specific_id);
       }
       for (auto rewrite : facet_type_info.rewrite_constraints) {
-        auto lhs_inst_id =
-            context.constant_values().GetInstId(rewrite.lhs_const_id);
-        auto rhs_inst_id =
-            context.constant_values().GetInstId(rewrite.rhs_const_id);
-        worklist.Push(lhs_inst_id);
-        worklist.Push(rhs_inst_id);
+        worklist.Push(rewrite.lhs_id);
+        worklist.Push(rewrite.rhs_id);
       }
       // TODO: Process other requirements as well.
       break;
@@ -242,9 +238,9 @@ static auto PopOperand(Context& context, Worklist& worklist,
           SemIR::FacetTypeInfo::RewriteConstraint::None);
       for (auto& new_constraint :
            llvm::reverse(new_facet_type_info.rewrite_constraints)) {
-        auto rhs_id = context.constant_values().Get(worklist.Pop());
-        auto lhs_id = context.constant_values().Get(worklist.Pop());
-        new_constraint = {.lhs_const_id = lhs_id, .rhs_const_id = rhs_id};
+        auto rhs_id = worklist.Pop();
+        auto lhs_id = worklist.Pop();
+        new_constraint = {.lhs_id = lhs_id, .rhs_id = rhs_id};
       }
       new_facet_type_info.self_impls_constraints.resize(
           old_facet_type_info.self_impls_constraints.size(),

+ 1 - 1
toolchain/check/testdata/impl/use_assoc_const.carbon

@@ -3431,7 +3431,7 @@ fn F() {
 // CHECK:STDOUT:   %T.loc9_14.2: type = bind_symbolic_name T, 0 [symbolic = %T.loc9_14.2 (constants.%T)]
 // CHECK:STDOUT:   %T.patt.loc9_14.2: type = symbolic_binding_pattern T, 0 [symbolic = %T.patt.loc9_14.2 (constants.%T.patt)]
 // CHECK:STDOUT:   %C.loc9_45.2: type = class_type @C, @C(%T.loc9_14.2) [symbolic = %C.loc9_45.2 (constants.%C.f2e)]
-// CHECK:STDOUT:   %Z_where.type: type = facet_type <@Z where constants.%impl.elem0 = %C.loc9_45.2 (constants.%C.f2e)> [symbolic = %Z_where.type (constants.%Z_where.type.b30)]
+// CHECK:STDOUT:   %Z_where.type: type = facet_type <@Z where constants.%impl.elem0 = %C.loc9_45.2> [symbolic = %Z_where.type (constants.%Z_where.type.b30)]
 // CHECK:STDOUT:   %require_complete: <witness> = require_complete_type @impl.%Z_where.type (%Z_where.type.b30) [symbolic = %require_complete (constants.%require_complete)]
 // CHECK:STDOUT:   %impl_witness: <witness> = impl_witness (file.%impl_witness_assoc_constant), @impl(%T.loc9_14.2) [symbolic = %impl_witness (constants.%impl_witness.017)]
 // CHECK:STDOUT:

+ 1 - 1
toolchain/check/testdata/where_expr/dot_self_index.carbon

@@ -193,7 +193,7 @@ fn G(U: Empty(i32) where .A = i32*) {
 // CHECK:STDOUT:   %.Self.as_wit.iface0.loc18_34.2: <witness> = facet_access_witness %.Self.2, element0 [symbolic = %.Self.as_wit.iface0.loc18_34.2 (constants.%.Self.as_wit.iface0.663)]
 // CHECK:STDOUT:   %impl.elem0.loc18_34.2: type = impl_witness_access %.Self.as_wit.iface0.loc18_34.2, element0 [symbolic = %impl.elem0.loc18_34.2 (constants.%impl.elem0.d0b)]
 // CHECK:STDOUT:   %ptr.loc18_40.2: type = ptr_type %T.loc18_6.2 [symbolic = %ptr.loc18_40.2 (constants.%ptr.79f)]
-// CHECK:STDOUT:   %Empty_where.type: type = facet_type <@Empty, @Empty(%T.loc18_6.2) where %impl.elem0.loc18_34.2 (constants.%impl.elem0.d0b) = %ptr.loc18_40.2 (constants.%ptr.79f)> [symbolic = %Empty_where.type (constants.%Empty_where.type.5a8)]
+// CHECK:STDOUT:   %Empty_where.type: type = facet_type <@Empty, @Empty(%T.loc18_6.2) where %impl.elem0.loc18_34.2 = %ptr.loc18_40.2> [symbolic = %Empty_where.type (constants.%Empty_where.type.5a8)]
 // CHECK:STDOUT:   %V.loc18_43.2: type = bind_symbolic_name V, 1 [symbolic = %V.loc18_43.2 (constants.%V)]
 // CHECK:STDOUT:   %V.patt.loc18_43.2: type = symbolic_binding_pattern V, 1 [symbolic = %V.patt.loc18_43.2 (constants.%V.patt)]
 // CHECK:STDOUT:

+ 33 - 33
toolchain/check/testdata/where_expr/equal_rewrite.carbon

@@ -111,7 +111,7 @@ fn Calls() {
   // CHECK:STDERR:
   Equal(bool);
 
-  // CHECK:STDERR: fail_import_rewrites.carbon:[[@LINE+8]]:3: error: cannot convert type `i32` into type implementing `A where .(A.C) = () and .(A.B) = bool` [ConversionFailureTypeToFacet]
+  // CHECK:STDERR: fail_import_rewrites.carbon:[[@LINE+8]]:3: error: cannot convert type `i32` into type implementing `A where .(A.B) = bool and .(A.C) = ()` [ConversionFailureTypeToFacet]
   // CHECK:STDERR:   NestedRewrite(i32);
   // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~
   // CHECK:STDERR: fail_import_rewrites.carbon:[[@LINE-17]]:1: in import [InImport]
@@ -179,13 +179,13 @@ interface E {
 // Testing how these types get stringified in error messages.
 
 // TODO: This should be a compile-time binding, once that is supported.
-// CHECK:STDERR: fail_type_does_not_implement_where.carbon:[[@LINE+4]]:42: error: cannot convert type `f64` into type implementing `E where .(E.G) = () and .(E.F) = bool` [ConversionFailureTypeToFacet]
+// CHECK:STDERR: fail_type_does_not_implement_where.carbon:[[@LINE+4]]:42: error: cannot convert type `f64` into type implementing `E where .(E.F) = bool and .(E.G) = ()` [ConversionFailureTypeToFacet]
 // CHECK:STDERR: let H: (E where .F = bool and .G = ()) = f64;
 // CHECK:STDERR:                                          ^~~
 // CHECK:STDERR:
 let H: (E where .F = bool and .G = ()) = f64;
 
-// CHECK:STDERR: fail_type_does_not_implement_where.carbon:[[@LINE+4]]:45: error: cannot convert type `bool` into type implementing `E where .(E.G) = i32 and .(E.F) = {}` [ConversionFailureTypeToFacet]
+// CHECK:STDERR: fail_type_does_not_implement_where.carbon:[[@LINE+4]]:45: error: cannot convert type `bool` into type implementing `E where .(E.F) = {} and .(E.G) = i32` [ConversionFailureTypeToFacet]
 // CHECK:STDERR: let J: ((E where .F = {}) where .G = i32) = bool;
 // CHECK:STDERR:                                             ^~~~
 // CHECK:STDERR:
@@ -308,9 +308,9 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %.Self.as_wit.iface0.d15: <witness> = facet_access_witness %.Self.80b, element0 [symbolic_self]
 // CHECK:STDOUT:   %A.facet.271: %A.type = facet_value %.Self.as_type.ec1, (%.Self.as_wit.iface0.d15) [symbolic_self]
 // CHECK:STDOUT:   %impl.elem1: type = impl_witness_access %.Self.as_wit.iface0.d15, element1 [symbolic_self]
-// CHECK:STDOUT:   %A_where.type.248: type = facet_type <@A where %impl.elem1 = %empty_tuple.type and %impl.elem0 = bool> [concrete]
-// CHECK:STDOUT:   %D: %A_where.type.248 = bind_symbolic_name D, 0 [symbolic]
-// CHECK:STDOUT:   %D.patt: %A_where.type.248 = symbolic_binding_pattern D, 0 [symbolic]
+// CHECK:STDOUT:   %A_where.type.0fe: type = facet_type <@A where %impl.elem0 = bool and %impl.elem1 = %empty_tuple.type> [concrete]
+// CHECK:STDOUT:   %D: %A_where.type.0fe = bind_symbolic_name D, 0 [symbolic]
+// CHECK:STDOUT:   %D.patt: %A_where.type.0fe = symbolic_binding_pattern D, 0 [symbolic]
 // CHECK:STDOUT:   %NestedRewrite.type: type = fn_type @NestedRewrite [concrete]
 // CHECK:STDOUT:   %NestedRewrite: %NestedRewrite.type = struct_value () [concrete]
 // CHECK:STDOUT: }
@@ -332,9 +332,9 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %A.decl: type = interface_decl @A [concrete = constants.%A.type] {} {}
 // CHECK:STDOUT:   %NestedRewrite.decl: %NestedRewrite.type = fn_decl @NestedRewrite [concrete = constants.%NestedRewrite] {
-// CHECK:STDOUT:     %D.patt.loc9_18.1: %A_where.type.248 = symbolic_binding_pattern D, 0 [symbolic = %D.patt.loc9_18.2 (constants.%D.patt)]
+// CHECK:STDOUT:     %D.patt.loc9_18.1: %A_where.type.0fe = symbolic_binding_pattern D, 0 [symbolic = %D.patt.loc9_18.2 (constants.%D.patt)]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %.loc9_42.1: type = splice_block %.loc9_42.2 [concrete = constants.%A_where.type.248] {
+// CHECK:STDOUT:     %.loc9_42.1: type = splice_block %.loc9_42.2 [concrete = constants.%A_where.type.0fe] {
 // CHECK:STDOUT:       %A.ref: type = name_ref A, file.%A.decl [concrete = constants.%A.type]
 // CHECK:STDOUT:       %.Self.1: %A.type = bind_symbolic_name .Self [symbolic_self = constants.%.Self.3ca]
 // CHECK:STDOUT:       %.Self.ref.loc9_31: %A.type = name_ref .Self, %.Self.1 [symbolic_self = constants.%.Self.3ca]
@@ -358,11 +358,11 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:       %impl.elem1: type = impl_witness_access %.Self.as_wit.iface0.loc9_48, element1 [symbolic_self = constants.%impl.elem1]
 // CHECK:STDOUT:       %.loc9_54.1: %empty_tuple.type = tuple_literal ()
 // CHECK:STDOUT:       %.loc9_54.2: type = converted %.loc9_54.1, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type]
-// CHECK:STDOUT:       %.loc9_42.2: type = where_expr %.Self.2 [concrete = constants.%A_where.type.248] {
+// CHECK:STDOUT:       %.loc9_42.2: type = where_expr %.Self.2 [concrete = constants.%A_where.type.0fe] {
 // CHECK:STDOUT:         requirement_rewrite %impl.elem1, %.loc9_54.2
 // CHECK:STDOUT:       }
 // CHECK:STDOUT:     }
-// CHECK:STDOUT:     %D.loc9_18.1: %A_where.type.248 = bind_symbolic_name D, 0 [symbolic = %D.loc9_18.2 (constants.%D)]
+// CHECK:STDOUT:     %D.loc9_18.1: %A_where.type.0fe = bind_symbolic_name D, 0 [symbolic = %D.loc9_18.2 (constants.%D)]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -390,11 +390,11 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   assoc_const C:! type;
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: generic fn @NestedRewrite(%D.loc9_18.1: %A_where.type.248) {
-// CHECK:STDOUT:   %D.loc9_18.2: %A_where.type.248 = bind_symbolic_name D, 0 [symbolic = %D.loc9_18.2 (constants.%D)]
-// CHECK:STDOUT:   %D.patt.loc9_18.2: %A_where.type.248 = symbolic_binding_pattern D, 0 [symbolic = %D.patt.loc9_18.2 (constants.%D.patt)]
+// CHECK:STDOUT: generic fn @NestedRewrite(%D.loc9_18.1: %A_where.type.0fe) {
+// CHECK:STDOUT:   %D.loc9_18.2: %A_where.type.0fe = bind_symbolic_name D, 0 [symbolic = %D.loc9_18.2 (constants.%D)]
+// CHECK:STDOUT:   %D.patt.loc9_18.2: %A_where.type.0fe = symbolic_binding_pattern D, 0 [symbolic = %D.patt.loc9_18.2 (constants.%D.patt)]
 // CHECK:STDOUT:
-// CHECK:STDOUT:   fn(%D.patt.loc9_18.1: %A_where.type.248);
+// CHECK:STDOUT:   fn(%D.patt.loc9_18.1: %A_where.type.0fe);
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @B(constants.%Self) {}
@@ -656,7 +656,7 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %impl.elem1: type = impl_witness_access %.Self.as_wit.iface0, element1 [symbolic_self]
 // CHECK:STDOUT:   %Bool.type: type = fn_type @Bool [concrete]
 // CHECK:STDOUT:   %Bool: %Bool.type = struct_value () [concrete]
-// CHECK:STDOUT:   %J_where.type: type = facet_type <@J where %impl.elem1 = bool and %impl.elem0 = %empty_tuple.type> [concrete]
+// CHECK:STDOUT:   %J_where.type: type = facet_type <@J where %impl.elem0 = %empty_tuple.type and %impl.elem1 = bool> [concrete]
 // CHECK:STDOUT:   %M: %J_where.type = bind_symbolic_name M, 0 [symbolic]
 // CHECK:STDOUT:   %M.patt: %J_where.type = symbolic_binding_pattern M, 0 [symbolic]
 // CHECK:STDOUT:   %Alphabetical.type: type = fn_type @Alphabetical [concrete]
@@ -1200,9 +1200,9 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %.Self.80b: %A_where.type.bcb = bind_symbolic_name .Self [symbolic_self]
 // CHECK:STDOUT:   %.Self.as_wit.iface0.d15: <witness> = facet_access_witness %.Self.80b, element0 [symbolic_self]
 // CHECK:STDOUT:   %impl.elem1: type = impl_witness_access %.Self.as_wit.iface0.d15, element1 [symbolic_self]
-// CHECK:STDOUT:   %A_where.type.248: type = facet_type <@A where %impl.elem1 = %empty_tuple.type and %impl.elem0.696 = bool> [concrete]
-// CHECK:STDOUT:   %D.patt: %A_where.type.248 = symbolic_binding_pattern D, 0 [symbolic]
-// CHECK:STDOUT:   %D: %A_where.type.248 = bind_symbolic_name D, 0 [symbolic]
+// CHECK:STDOUT:   %A_where.type.0fe: type = facet_type <@A where %impl.elem0.696 = bool and %impl.elem1 = %empty_tuple.type> [concrete]
+// CHECK:STDOUT:   %D.patt: %A_where.type.0fe = symbolic_binding_pattern D, 0 [symbolic]
+// CHECK:STDOUT:   %D: %A_where.type.0fe = bind_symbolic_name D, 0 [symbolic]
 // CHECK:STDOUT:   %int_32: Core.IntLiteral = int_value 32 [concrete]
 // CHECK:STDOUT:   %i32: type = class_type @Int, @Int(%int_32) [concrete]
 // CHECK:STDOUT: }
@@ -1227,7 +1227,7 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %Main.import_ref.30f = import_ref Main//nested_rewrites, loc6_8, unloaded
 // CHECK:STDOUT:   %Main.B = import_ref Main//nested_rewrites, B, unloaded
 // CHECK:STDOUT:   %Main.C = import_ref Main//nested_rewrites, C, unloaded
-// CHECK:STDOUT:   %Main.import_ref.705: %A_where.type.248 = import_ref Main//nested_rewrites, loc9_18, loaded [symbolic = @NestedRewrite.%D (constants.%D)]
+// CHECK:STDOUT:   %Main.import_ref.3bd: %A_where.type.0fe = import_ref Main//nested_rewrites, loc9_18, loaded [symbolic = @NestedRewrite.%D (constants.%D)]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -1276,11 +1276,11 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   fn(%T.patt.1: %N_where.type);
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: generic fn @NestedRewrite(imports.%Main.import_ref.705: %A_where.type.248) [from "nested_rewrites.carbon"] {
-// CHECK:STDOUT:   %D: %A_where.type.248 = bind_symbolic_name D, 0 [symbolic = %D (constants.%D)]
-// CHECK:STDOUT:   %D.patt.2: %A_where.type.248 = symbolic_binding_pattern D, 0 [symbolic = %D.patt.2 (constants.%D.patt)]
+// CHECK:STDOUT: generic fn @NestedRewrite(imports.%Main.import_ref.3bd: %A_where.type.0fe) [from "nested_rewrites.carbon"] {
+// CHECK:STDOUT:   %D: %A_where.type.0fe = bind_symbolic_name D, 0 [symbolic = %D (constants.%D)]
+// CHECK:STDOUT:   %D.patt.2: %A_where.type.0fe = symbolic_binding_pattern D, 0 [symbolic = %D.patt.2 (constants.%D.patt)]
 // CHECK:STDOUT:
-// CHECK:STDOUT:   fn(%D.patt.1: %A_where.type.248);
+// CHECK:STDOUT:   fn(%D.patt.1: %A_where.type.0fe);
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @Equal.1(constants.%T) {
@@ -1575,7 +1575,7 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %empty_tuple.type: type = tuple_type () [concrete]
 // CHECK:STDOUT:   %Bool: %Bool.type = struct_value () [concrete]
 // CHECK:STDOUT:   %impl.elem1.7a9: type = impl_witness_access %.Self.as_wit.iface0.7f0, element1 [symbolic_self]
-// CHECK:STDOUT:   %E_where.type.281: type = facet_type <@E where %impl.elem1.7a9 = %empty_tuple.type and %impl.elem0 = bool> [concrete]
+// CHECK:STDOUT:   %E_where.type.f28: type = facet_type <@E where %impl.elem0 = bool and %impl.elem1.7a9 = %empty_tuple.type> [concrete]
 // CHECK:STDOUT:   %int_64: Core.IntLiteral = int_value 64 [concrete]
 // CHECK:STDOUT:   %Float.type: type = fn_type @Float [concrete]
 // CHECK:STDOUT:   %Float: %Float.type = struct_value () [concrete]
@@ -1588,7 +1588,7 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %impl.elem1.bae: type = impl_witness_access %.Self.as_wit.iface0.b26, element1 [symbolic_self]
 // CHECK:STDOUT:   %int_32: Core.IntLiteral = int_value 32 [concrete]
 // CHECK:STDOUT:   %i32: type = class_type @Int, @Int(%int_32) [concrete]
-// CHECK:STDOUT:   %E_where.type.d43: type = facet_type <@E where %impl.elem1.bae = %i32 and %impl.elem0 = %empty_struct_type> [concrete]
+// CHECK:STDOUT:   %E_where.type.7f5: type = facet_type <@E where %impl.elem0 = %empty_struct_type and %impl.elem1.bae = %i32> [concrete]
 // CHECK:STDOUT:   %E_where.type.ca8: type = facet_type <@E where %impl.elem0 = %impl.elem1.7a9> [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -1613,9 +1613,9 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %E.decl: type = interface_decl @E [concrete = constants.%E.type] {} {}
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %H.patt: %E_where.type.281 = binding_pattern H
+// CHECK:STDOUT:     %H.patt: %E_where.type.f28 = binding_pattern H
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %.loc15_11.1: type = splice_block %.loc15_11.2 [concrete = constants.%E_where.type.281] {
+// CHECK:STDOUT:   %.loc15_11.1: type = splice_block %.loc15_11.2 [concrete = constants.%E_where.type.f28] {
 // CHECK:STDOUT:     %E.ref.loc15: type = name_ref E, %E.decl [concrete = constants.%E.type]
 // CHECK:STDOUT:     %.Self.1: %E.type = bind_symbolic_name .Self [symbolic_self = constants.%.Self.da5]
 // CHECK:STDOUT:     %.Self.ref.loc15_17: %E.type = name_ref .Self, %.Self.1 [symbolic_self = constants.%.Self.da5]
@@ -1635,16 +1635,16 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:     %impl.elem1.loc15: type = impl_witness_access %.Self.as_wit.iface0.loc15_31, element1 [symbolic_self = constants.%impl.elem1.7a9]
 // CHECK:STDOUT:     %.loc15_37.1: %empty_tuple.type = tuple_literal ()
 // CHECK:STDOUT:     %.loc15_37.2: type = converted %.loc15_37.1, constants.%empty_tuple.type [concrete = constants.%empty_tuple.type]
-// CHECK:STDOUT:     %.loc15_11.2: type = where_expr %.Self.1 [concrete = constants.%E_where.type.281] {
+// CHECK:STDOUT:     %.loc15_11.2: type = where_expr %.Self.1 [concrete = constants.%E_where.type.f28] {
 // CHECK:STDOUT:       requirement_rewrite %impl.elem0.loc15, %.loc15_22.2
 // CHECK:STDOUT:       requirement_rewrite %impl.elem1.loc15, %.loc15_37.2
 // CHECK:STDOUT:     }
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %H: %E_where.type.281 = bind_name H, <error>
+// CHECK:STDOUT:   %H: %E_where.type.f28 = bind_name H, <error>
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %J.patt: %E_where.type.d43 = binding_pattern J
+// CHECK:STDOUT:     %J.patt: %E_where.type.7f5 = binding_pattern J
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %.loc21_27.1: type = splice_block %.loc21_27.2 [concrete = constants.%E_where.type.d43] {
+// CHECK:STDOUT:   %.loc21_27.1: type = splice_block %.loc21_27.2 [concrete = constants.%E_where.type.7f5] {
 // CHECK:STDOUT:     %E.ref.loc21: type = name_ref E, %E.decl [concrete = constants.%E.type]
 // CHECK:STDOUT:     %.Self.2: %E.type = bind_symbolic_name .Self [symbolic_self = constants.%.Self.da5]
 // CHECK:STDOUT:     %.Self.ref.loc21_18: %E.type = name_ref .Self, %.Self.2 [symbolic_self = constants.%.Self.da5]
@@ -1667,11 +1667,11 @@ let K: (E where .F = .Self.G) = bool;
 // CHECK:STDOUT:     %impl.elem1.loc21: type = impl_witness_access %.Self.as_wit.iface0.loc21_33, element1 [symbolic_self = constants.%impl.elem1.bae]
 // CHECK:STDOUT:     %int_32: Core.IntLiteral = int_value 32 [concrete = constants.%int_32]
 // CHECK:STDOUT:     %i32: type = class_type @Int, @Int(constants.%int_32) [concrete = constants.%i32]
-// CHECK:STDOUT:     %.loc21_27.2: type = where_expr %.Self.3 [concrete = constants.%E_where.type.d43] {
+// CHECK:STDOUT:     %.loc21_27.2: type = where_expr %.Self.3 [concrete = constants.%E_where.type.7f5] {
 // CHECK:STDOUT:       requirement_rewrite %impl.elem1.loc21, %i32
 // CHECK:STDOUT:     }
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %J: %E_where.type.d43 = bind_name J, <error>
+// CHECK:STDOUT:   %J: %E_where.type.7f5 = bind_name J, <error>
 // CHECK:STDOUT:   name_binding_decl {
 // CHECK:STDOUT:     %K.patt: %E_where.type.ca8 = binding_pattern K
 // CHECK:STDOUT:   }

+ 2 - 2
toolchain/sem_ir/dump.cpp

@@ -129,8 +129,8 @@ LLVM_DUMP_METHOD auto Dump(const File& file, FacetTypeId facet_type_id)
   }
   for (auto rewrite : facet_type.rewrite_constraints) {
     out << "\n"
-        << "  - " << DumpConstantSummary(file, rewrite.lhs_const_id) << "\n"
-        << "  - " << DumpConstantSummary(file, rewrite.rhs_const_id);
+        << "  - " << DumpInstSummary(file, rewrite.lhs_id) << "\n"
+        << "  - " << DumpInstSummary(file, rewrite.rhs_id);
   }
   if (auto identified_id =
           file.identified_facet_types().TryGetId(facet_type_id);

+ 3 - 3
toolchain/sem_ir/facet_type_info.cpp

@@ -22,8 +22,8 @@ static auto ImplsLess(const FacetTypeInfo::ImplsConstraint& lhs,
 // Canonically ordered by the numerical ids.
 static auto RewriteLess(const FacetTypeInfo::RewriteConstraint& lhs,
                         const FacetTypeInfo::RewriteConstraint& rhs) -> bool {
-  return std::tie(lhs.lhs_const_id.index, lhs.rhs_const_id.index) <
-         std::tie(rhs.lhs_const_id.index, rhs.rhs_const_id.index);
+  return std::tie(lhs.lhs_id.index, lhs.rhs_id.index) <
+         std::tie(rhs.lhs_id.index, rhs.rhs_id.index);
 }
 
 // Canonically ordered by the numerical ids.
@@ -147,7 +147,7 @@ auto FacetTypeInfo::Print(llvm::raw_ostream& out) const -> void {
     out << outer_sep << "rewrites: ";
     llvm::ListSeparator sep;
     for (RewriteConstraint req : rewrite_constraints) {
-      out << sep << req.lhs_const_id << "=" << req.rhs_const_id;
+      out << sep << req.lhs_id << "=" << req.rhs_id;
     }
   }
 

+ 4 - 4
toolchain/sem_ir/facet_type_info.h

@@ -35,8 +35,8 @@ struct FacetTypeInfo : Printable<FacetTypeInfo> {
 
   // Rewrite constraints of the form `.T = U`
   struct RewriteConstraint {
-    ConstantId lhs_const_id;
-    ConstantId rhs_const_id;
+    InstId lhs_id;
+    InstId rhs_id;
 
     static const RewriteConstraint None;
 
@@ -78,8 +78,8 @@ struct FacetTypeInfo : Printable<FacetTypeInfo> {
 };
 
 constexpr FacetTypeInfo::RewriteConstraint
-    FacetTypeInfo::RewriteConstraint::None = {.lhs_const_id = ConstantId::None,
-                                              .rhs_const_id = ConstantId::None};
+    FacetTypeInfo::RewriteConstraint::None = {.lhs_id = InstId::None,
+                                              .rhs_id = InstId::None};
 
 struct IdentifiedFacetType {
   using RequiredInterface = SpecificInterface;

+ 2 - 2
toolchain/sem_ir/formatter.cpp

@@ -1276,9 +1276,9 @@ class FormatterImpl {
       }
       for (auto rewrite : info.rewrite_constraints) {
         out_ << and_sep;
-        FormatConstant(rewrite.lhs_const_id);
+        FormatArg(rewrite.lhs_id);
         out_ << " = ";
-        FormatConstant(rewrite.rhs_const_id);
+        FormatArg(rewrite.rhs_id);
       }
       if (info.other_requirements) {
         out_ << and_sep << "TODO";

+ 1 - 5
toolchain/sem_ir/stringify_type.cpp

@@ -305,11 +305,7 @@ class Stringifier {
       if (some_where) {
         step_stack_->PushString(" and");
       }
-      auto lhs_const_id =
-          sem_ir_->constant_values().GetInstId(rewrite.lhs_const_id);
-      auto rhs_const_id =
-          sem_ir_->constant_values().GetInstId(rewrite.rhs_const_id);
-      step_stack_->Push(" ", lhs_const_id, " = ", rhs_const_id);
+      step_stack_->Push(" ", rewrite.lhs_id, " = ", rewrite.rhs_id);
       some_where = true;
     }
     if (!facet_type_info.self_impls_constraints.empty()) {