From a735a4e463ee638e299cc3061198bd9d487bebc8 Mon Sep 17 00:00:00 2001 From: Dana Jansens Date: Wed, 5 Feb 2025 15:16:52 -0500 Subject: [PATCH] Make the constant value of AsCompatible match its type (#4881) AsCompatible changes a source instruction's type to a compatible type, so it also needs its constant value to take on the compatible type. Otherwise the type of the instruction and its constant value will differ, which makes moving to the constant value into a lossy transformation. Part of #4868 --- toolchain/check/eval.cpp | 41 ++++++++++++++++++- .../testdata/as/adapter_conversion.carbon | 10 +++-- .../builtin/no_prelude/adapted_type.carbon | 25 ++++++----- .../no_prelude/import_builtin_call.carbon | 25 +++++------ 4 files changed, 72 insertions(+), 29 deletions(-) diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index eeb4f2d2e0a2..a7bb994c56ea 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -1512,6 +1512,10 @@ static auto MakeFacetTypeResult(Context& context, } // Implementation for `TryEvalInst`, wrapping `Context` with `EvalContext`. +// +// Tail call should not be diagnosed as recursion. +// https://github.com/llvm/llvm-project/issues/125724 +// NOLINTNEXTLINE(misc-no-recursion): Tail call. static auto TryEvalInstInContext(EvalContext& eval_context, SemIR::InstId inst_id, SemIR::Inst inst) -> SemIR::ConstantId { @@ -1898,10 +1902,43 @@ static auto TryEvalInstInContext(EvalContext& eval_context, return MakeConstantResult(eval_context.context(), bind, phase); } - // These semantic wrappers don't change the constant value. + // AsCompatible changes the type of the source instruction; its constant + // value, if there is one, needs to be modified to be of the same type. case CARBON_KIND(SemIR::AsCompatible inst): { - return eval_context.GetConstantValue(inst.source_id); + auto value = eval_context.GetConstantValue(inst.source_id); + if (!value.is_constant()) { + return value; + } + + auto from_phase = Phase::Template; + auto value_inst_id = + GetConstantValue(eval_context, inst.source_id, &from_phase); + + auto to_phase = Phase::Template; + auto type_id = GetConstantValue(eval_context, inst.type_id, &to_phase); + + auto value_inst = eval_context.insts().Get(value_inst_id); + value_inst.SetType(type_id); + + if (to_phase >= from_phase) { + // If moving from a template constant value to a symbolic type, the new + // constant value takes on the phase of the new type. We're adding the + // symbolic bit to the new constant value due to the presence of a + // symbolic type. + return MakeConstantResult(eval_context.context(), value_inst, to_phase); + } else { + // If moving from a symbolic constant value to a template type, the new + // constant value has a phase that depends on what is in the value. If + // there is anything symbolic within the value, then it's symbolic. We + // can't easily determine that here without evaluating a new constant + // value. See + // https://github.com/carbon-language/carbon-lang/pull/4881#discussion_r1939961372 + [[clang::musttail]] return TryEvalInstInContext( + eval_context, SemIR::InstId::None, value_inst); + } } + + // These semantic wrappers don't change the constant value. case CARBON_KIND(SemIR::BindAlias typed_inst): { return eval_context.GetConstantValue(typed_inst.value_id); } diff --git a/toolchain/check/testdata/as/adapter_conversion.carbon b/toolchain/check/testdata/as/adapter_conversion.carbon index ce979da67938..c06ca8fc45a7 100644 --- a/toolchain/check/testdata/as/adapter_conversion.carbon +++ b/toolchain/check/testdata/as/adapter_conversion.carbon @@ -377,6 +377,7 @@ var b: B = {.x = 1} as B; // CHECK:STDOUT: %Convert.bound: = bound_method %int_1.5b8, %Convert.197 [template] // CHECK:STDOUT: %Convert.specific_fn: = specific_function %Convert.bound, @Convert.5(%int_32) [template] // CHECK:STDOUT: %int_1.5d2: %i32 = int_value 1 [template] +// CHECK:STDOUT: %int_1.e78: %A = int_value 1 [template] // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: imports { @@ -435,8 +436,8 @@ var b: B = {.x = 1} as B; // CHECK:STDOUT: %.loc8_15.1: %i32 = value_of_initializer %int.convert_checked [template = constants.%int_1.5d2] // CHECK:STDOUT: %.loc8_15.2: %i32 = converted %int_1, %.loc8_15.1 [template = constants.%int_1.5d2] // CHECK:STDOUT: %A.ref: type = name_ref A, file.%A.decl [template = constants.%A] -// CHECK:STDOUT: %.loc8_23.1: %A = as_compatible %.loc8_15.2 [template = constants.%int_1.5d2] -// CHECK:STDOUT: %.loc8_23.2: %A = converted %.loc8_15.2, %.loc8_23.1 [template = constants.%int_1.5d2] +// CHECK:STDOUT: %.loc8_23.1: %A = as_compatible %.loc8_15.2 [template = constants.%int_1.e78] +// CHECK:STDOUT: %.loc8_23.2: %A = converted %.loc8_15.2, %.loc8_23.1 [template = constants.%int_1.e78] // CHECK:STDOUT: %a.ref: %A = name_ref a, file.%a // CHECK:STDOUT: %int_32.loc9: Core.IntLiteral = int_value 32 [template = constants.%int_32] // CHECK:STDOUT: %i32.loc9: type = class_type @Int, @Int(constants.%int_32) [template = constants.%i32] @@ -455,6 +456,7 @@ var b: B = {.x = 1} as B; // CHECK:STDOUT: %C: type = class_type @C [template] // CHECK:STDOUT: %D: type = class_type @D [template] // CHECK:STDOUT: %empty_struct: %empty_struct_type = struct_value () [template] +// CHECK:STDOUT: %D.val: %D = struct_value () [template] // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: imports { @@ -531,8 +533,8 @@ var b: B = {.x = 1} as B; // CHECK:STDOUT: %.loc9_13: %empty_struct_type = struct_literal () // CHECK:STDOUT: %D.ref: type = name_ref D, file.%D.decl [template = constants.%D] // CHECK:STDOUT: %empty_struct: %empty_struct_type = struct_value () [template = constants.%empty_struct] -// CHECK:STDOUT: %.loc9_15.1: %D = as_compatible %empty_struct [template = constants.%empty_struct] -// CHECK:STDOUT: %.loc9_15.2: %D = converted %.loc9_13, %.loc9_15.1 [template = constants.%empty_struct] +// CHECK:STDOUT: %.loc9_15.1: %D = as_compatible %empty_struct [template = constants.%D.val] +// CHECK:STDOUT: %.loc9_15.2: %D = converted %.loc9_13, %.loc9_15.1 [template = constants.%D.val] // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/function/builtin/no_prelude/adapted_type.carbon b/toolchain/check/testdata/function/builtin/no_prelude/adapted_type.carbon index c8ece830142d..ca51499be658 100644 --- a/toolchain/check/testdata/function/builtin/no_prelude/adapted_type.carbon +++ b/toolchain/check/testdata/function/builtin/no_prelude/adapted_type.carbon @@ -56,16 +56,19 @@ fn Int(N: MyIntLiteral) -> type = "int.make_type_signed"; // CHECK:STDOUT: %Int.type: type = fn_type @Int [template] // CHECK:STDOUT: %Int: %Int.type = struct_value () [template] // CHECK:STDOUT: %MyInt32: type = class_type @MyInt32 [template] -// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [template] -// CHECK:STDOUT: %i32.builtin: type = int_type signed, %int_32 [template] +// CHECK:STDOUT: %int_32.be0: Core.IntLiteral = int_value 32 [template] +// CHECK:STDOUT: %int_32.4da: %MyIntLiteral = int_value 32 [template] +// CHECK:STDOUT: %i32.builtin: type = int_type signed, %int_32.4da [template] // CHECK:STDOUT: %Make.type: type = fn_type @Make [template] // CHECK:STDOUT: %Make: %Make.type = struct_value () [template] -// CHECK:STDOUT: %complete_type.f8a: = complete_type_witness %i32.builtin [template] +// CHECK:STDOUT: %complete_type.ec2: = complete_type_witness %i32.builtin [template] // CHECK:STDOUT: %MyAdd.type: type = fn_type @MyAdd [template] // CHECK:STDOUT: %MyAdd: %MyAdd.type = struct_value () [template] // CHECK:STDOUT: %int_1.5b8: Core.IntLiteral = int_value 1 [template] +// CHECK:STDOUT: %int_1.383: %MyIntLiteral = int_value 1 [template] // CHECK:STDOUT: %int_1.d74: %MyInt32 = int_value 1 [template] // CHECK:STDOUT: %int_2.ecc: Core.IntLiteral = int_value 2 [template] +// CHECK:STDOUT: %int_2.a27: %MyIntLiteral = int_value 2 [template] // CHECK:STDOUT: %int_2.eda: %MyInt32 = int_value 2 [template] // CHECK:STDOUT: %int_3: %MyInt32 = int_value 3 [template] // CHECK:STDOUT: } @@ -155,10 +158,10 @@ fn Int(N: MyIntLiteral) -> type = "int.make_type_signed"; // CHECK:STDOUT: // CHECK:STDOUT: class @MyInt32 { // CHECK:STDOUT: %Int.ref: %Int.type = name_ref Int, file.%Int.decl [template = constants.%Int] -// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [template = constants.%int_32] +// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [template = constants.%int_32.be0] // CHECK:STDOUT: %MyIntLiteral.ref: type = name_ref MyIntLiteral, file.%MyIntLiteral.decl [template = constants.%MyIntLiteral] -// CHECK:STDOUT: %.loc13_16.1: %MyIntLiteral = as_compatible %int_32 [template = constants.%int_32] -// CHECK:STDOUT: %.loc13_16.2: %MyIntLiteral = converted %int_32, %.loc13_16.1 [template = constants.%int_32] +// CHECK:STDOUT: %.loc13_16.1: %MyIntLiteral = as_compatible %int_32 [template = constants.%int_32.4da] +// CHECK:STDOUT: %.loc13_16.2: %MyIntLiteral = converted %int_32, %.loc13_16.1 [template = constants.%int_32.4da] // CHECK:STDOUT: %int.make_type_signed: init type = call %Int.ref(%.loc13_16.2) [template = constants.%i32.builtin] // CHECK:STDOUT: %.loc13_32.1: type = value_of_initializer %int.make_type_signed [template = constants.%i32.builtin] // CHECK:STDOUT: %.loc13_32.2: type = converted %int.make_type_signed, %.loc13_32.1 [template = constants.%i32.builtin] @@ -176,7 +179,7 @@ fn Int(N: MyIntLiteral) -> type = "int.make_type_signed"; // CHECK:STDOUT: %return.param.loc15: ref %MyInt32 = out_param runtime_param1 // CHECK:STDOUT: %return.loc15: ref %MyInt32 = return_slot %return.param.loc15 // CHECK:STDOUT: } -// CHECK:STDOUT: %complete_type: = complete_type_witness %i32.builtin [template = constants.%complete_type.f8a] +// CHECK:STDOUT: %complete_type: = complete_type_witness %i32.builtin [template = constants.%complete_type.ec2] // CHECK:STDOUT: complete_type_witness = %complete_type // CHECK:STDOUT: // CHECK:STDOUT: !members: @@ -199,15 +202,15 @@ fn Int(N: MyIntLiteral) -> type = "int.make_type_signed"; // CHECK:STDOUT: %Make.ref.loc22_31: %Make.type = name_ref Make, @MyInt32.%Make.decl [template = constants.%Make] // CHECK:STDOUT: %int_1: Core.IntLiteral = int_value 1 [template = constants.%int_1.5b8] // CHECK:STDOUT: %MyIntLiteral.ref.loc22_42: type = name_ref MyIntLiteral, file.%MyIntLiteral.decl [template = constants.%MyIntLiteral] -// CHECK:STDOUT: %.loc22_39.1: %MyIntLiteral = as_compatible %int_1 [template = constants.%int_1.5b8] -// CHECK:STDOUT: %.loc22_39.2: %MyIntLiteral = converted %int_1, %.loc22_39.1 [template = constants.%int_1.5b8] +// CHECK:STDOUT: %.loc22_39.1: %MyIntLiteral = as_compatible %int_1 [template = constants.%int_1.383] +// CHECK:STDOUT: %.loc22_39.2: %MyIntLiteral = converted %int_1, %.loc22_39.1 [template = constants.%int_1.383] // CHECK:STDOUT: %int.convert_checked.loc22_54: init %MyInt32 = call %Make.ref.loc22_31(%.loc22_39.2) [template = constants.%int_1.d74] // CHECK:STDOUT: %MyInt32.ref.loc22_57: type = name_ref MyInt32, file.%MyInt32.decl [template = constants.%MyInt32] // CHECK:STDOUT: %Make.ref.loc22_64: %Make.type = name_ref Make, @MyInt32.%Make.decl [template = constants.%Make] // CHECK:STDOUT: %int_2: Core.IntLiteral = int_value 2 [template = constants.%int_2.ecc] // CHECK:STDOUT: %MyIntLiteral.ref.loc22_75: type = name_ref MyIntLiteral, file.%MyIntLiteral.decl [template = constants.%MyIntLiteral] -// CHECK:STDOUT: %.loc22_72.1: %MyIntLiteral = as_compatible %int_2 [template = constants.%int_2.ecc] -// CHECK:STDOUT: %.loc22_72.2: %MyIntLiteral = converted %int_2, %.loc22_72.1 [template = constants.%int_2.ecc] +// CHECK:STDOUT: %.loc22_72.1: %MyIntLiteral = as_compatible %int_2 [template = constants.%int_2.a27] +// CHECK:STDOUT: %.loc22_72.2: %MyIntLiteral = converted %int_2, %.loc22_72.1 [template = constants.%int_2.a27] // CHECK:STDOUT: %int.convert_checked.loc22_87: init %MyInt32 = call %Make.ref.loc22_64(%.loc22_72.2) [template = constants.%int_2.eda] // CHECK:STDOUT: %.loc22_54.1: %MyInt32 = value_of_initializer %int.convert_checked.loc22_54 [template = constants.%int_1.d74] // CHECK:STDOUT: %.loc22_54.2: %MyInt32 = converted %int.convert_checked.loc22_54, %.loc22_54.1 [template = constants.%int_1.d74] diff --git a/toolchain/check/testdata/impl/no_prelude/import_builtin_call.carbon b/toolchain/check/testdata/impl/no_prelude/import_builtin_call.carbon index 874afa9c534e..d54a3f7f4089 100644 --- a/toolchain/check/testdata/impl/no_prelude/import_builtin_call.carbon +++ b/toolchain/check/testdata/impl/no_prelude/import_builtin_call.carbon @@ -1049,8 +1049,9 @@ var n: Int(64) = MakeFromClass(FromLiteral(64) as OtherInt); // CHECK:STDOUT: %iN.builtin.9ef: type = int_type signed, %int.convert_checked.b6b [symbolic] // CHECK:STDOUT: %require_complete.c7d: = require_complete_type %iN.builtin.9ef [symbolic] // CHECK:STDOUT: %MakeFromClass.specific_fn.004: = specific_function %MakeFromClass, @MakeFromClass(%N.335) [symbolic] -// CHECK:STDOUT: %ToLiteral.bound.395: = bound_method %int_64.f82, %ToLiteral.ec2 [template] -// CHECK:STDOUT: %MakeFromClass.specific_fn.ad7: = specific_function %MakeFromClass, @MakeFromClass(%int_64.f82) [template] +// CHECK:STDOUT: %int_64.06b: %OtherInt = int_value 64 [template] +// CHECK:STDOUT: %ToLiteral.bound.735: = bound_method %int_64.06b, %ToLiteral.ec2 [template] +// CHECK:STDOUT: %MakeFromClass.specific_fn.61b: = specific_function %MakeFromClass, @MakeFromClass(%int_64.06b) [template] // CHECK:STDOUT: %complete_type.4a1: = complete_type_witness %i64.builtin [template] // CHECK:STDOUT: } // CHECK:STDOUT: @@ -1169,11 +1170,11 @@ var n: Int(64) = MakeFromClass(FromLiteral(64) as OtherInt); // CHECK:STDOUT: %int_64.loc7: Core.IntLiteral = int_value 64 [template = constants.%int_64.fab] // CHECK:STDOUT: %int.convert_checked.loc7: init %i32.builtin = call %FromLiteral.ref.loc7(%int_64.loc7) [template = constants.%int_64.f82] // CHECK:STDOUT: %OtherInt.ref: type = name_ref OtherInt, imports.%Main.OtherInt [template = constants.%OtherInt] -// CHECK:STDOUT: %.loc7_48.1: init %OtherInt = as_compatible %int.convert_checked.loc7 [template = constants.%int_64.f82] -// CHECK:STDOUT: %.loc7_48.2: init %OtherInt = converted %int.convert_checked.loc7, %.loc7_48.1 [template = constants.%int_64.f82] -// CHECK:STDOUT: %.loc7_59.1: %OtherInt = value_of_initializer %.loc7_48.2 [template = constants.%int_64.f82] -// CHECK:STDOUT: %.loc7_59.2: %OtherInt = converted %.loc7_48.2, %.loc7_59.1 [template = constants.%int_64.f82] -// CHECK:STDOUT: %MakeFromClass.specific_fn: = specific_function %MakeFromClass.ref, @MakeFromClass(constants.%int_64.f82) [template = constants.%MakeFromClass.specific_fn.ad7] +// CHECK:STDOUT: %.loc7_48.1: init %OtherInt = as_compatible %int.convert_checked.loc7 [template = constants.%int_64.06b] +// CHECK:STDOUT: %.loc7_48.2: init %OtherInt = converted %int.convert_checked.loc7, %.loc7_48.1 [template = constants.%int_64.06b] +// CHECK:STDOUT: %.loc7_59.1: %OtherInt = value_of_initializer %.loc7_48.2 [template = constants.%int_64.06b] +// CHECK:STDOUT: %.loc7_59.2: %OtherInt = converted %.loc7_48.2, %.loc7_59.1 [template = constants.%int_64.06b] +// CHECK:STDOUT: %MakeFromClass.specific_fn: = specific_function %MakeFromClass.ref, @MakeFromClass(constants.%int_64.06b) [template = constants.%MakeFromClass.specific_fn.61b] // CHECK:STDOUT: %MakeFromClass.call: init %i64.builtin = call %MakeFromClass.specific_fn() // CHECK:STDOUT: assign file.%n.var, %MakeFromClass.call // CHECK:STDOUT: return @@ -1217,15 +1218,15 @@ var n: Int(64) = MakeFromClass(FromLiteral(64) as OtherInt); // CHECK:STDOUT: // CHECK:STDOUT: specific @MakeFromClass(%N) {} // CHECK:STDOUT: -// CHECK:STDOUT: specific @MakeFromClass(constants.%int_64.f82) { -// CHECK:STDOUT: %N => constants.%int_64.f82 -// CHECK:STDOUT: %N.patt => constants.%int_64.f82 -// CHECK:STDOUT: %ToLiteral.bound => constants.%ToLiteral.bound.395 +// CHECK:STDOUT: specific @MakeFromClass(constants.%int_64.06b) { +// CHECK:STDOUT: %N => constants.%int_64.06b +// CHECK:STDOUT: %N.patt => constants.%int_64.06b +// CHECK:STDOUT: %ToLiteral.bound => constants.%ToLiteral.bound.735 // CHECK:STDOUT: %int.convert_checked => constants.%int_64.fab // CHECK:STDOUT: %iN.builtin => constants.%i64.builtin // CHECK:STDOUT: // CHECK:STDOUT: !definition: // CHECK:STDOUT: %require_complete => constants.%complete_type.4a1 -// CHECK:STDOUT: %MakeFromClass.specific_fn => constants.%MakeFromClass.specific_fn.ad7 +// CHECK:STDOUT: %MakeFromClass.specific_fn => constants.%MakeFromClass.specific_fn.61b // CHECK:STDOUT: } // CHECK:STDOUT: