Skip to content

Commit 4f07b40

Browse files
zygoloidzygoloid
committed
Create a Call instruction directly when building a thunk call.
Don't go through the `PerformCall` machinery a second recursive time -- this is redundant, creates additional unnecessary temporaries, and is in theory wrong because `PerformCall` takes a syntactic argument list (one argument per callee parameter pattern), but we have a call argument list (one argument per callee parameter).
1 parent 53ea894 commit 4f07b40

File tree

2 files changed

+29
-15
lines changed

2 files changed

+29
-15
lines changed

‎toolchain/check/cpp/thunk.cpp‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -669,9 +669,25 @@ auto PerformCppThunkCall(Context& context, SemIR::LocId loc_id,
669669
context.insts().Get(return_slot_id).type_id())),
670670
.lvalue_id = return_slot_id});
671671
thunk_arg_ids.push_back(arg_id);
672+
} else if (return_slot_id.has_value()) {
673+
thunk_arg_ids.push_back(return_slot_id);
672674
}
673675

674-
auto result_id = PerformCall(context, loc_id, thunk_callee_id, thunk_arg_ids);
676+
// Compute the return type of the call to the thunk.
677+
auto thunk_return_type_id =
678+
thunk_function.GetDeclaredReturnType(context.sem_ir());
679+
if (!thunk_return_type_id.has_value()) {
680+
thunk_return_type_id = GetTupleType(context, {});
681+
CARBON_CHECK(thunk_takes_return_address || !return_type_id.has_value());
682+
} else {
683+
CARBON_CHECK(thunk_return_type_id == return_type_id);
684+
}
685+
686+
auto result_id = GetOrAddInst<SemIR::Call>(
687+
context, loc_id,
688+
{.type_id = thunk_return_type_id,
689+
.callee_id = thunk_callee_id,
690+
.args_id = context.inst_blocks().Add(thunk_arg_ids)});
675691

676692
// Produce the result of the call, taking the value from the return storage.
677693
if (thunk_takes_return_address) {

‎toolchain/check/testdata/interop/cpp/function/pointer.carbon‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,9 +1036,8 @@ fn F() {
10361036
// CHECK:STDOUT: %.loc11_14.2: init %Optional.143 = converted %addr.loc11_14.1, %T.binding.as_type.as.ImplicitAs.impl.Convert.call
10371037
// CHECK:STDOUT: %.loc11_14.3: ref %Optional.143 = temporary %.loc11_14.1, %.loc11_14.2
10381038
// CHECK:STDOUT: %.loc11_14.4: %Optional.143 = bind_value %.loc11_14.3
1039-
// CHECK:STDOUT: %.loc11_16.2: ref %Optional.143 = temporary_storage
1040-
// CHECK:STDOUT: %Direct__carbon_thunk.call: init %Optional.143 = call imports.%Direct__carbon_thunk.decl(%.loc11_14.4) to %.loc11_16.2
1041-
// CHECK:STDOUT: %.loc11_16.3: ref %Optional.143 = temporary %.loc11_16.2, %Direct__carbon_thunk.call
1039+
// CHECK:STDOUT: %Direct__carbon_thunk.call: init %Optional.143 = call imports.%Direct__carbon_thunk.decl(%.loc11_14.4) to %.loc11_16.1
1040+
// CHECK:STDOUT: %.loc11_16.2: ref %Optional.143 = temporary %.loc11_16.1, %Direct__carbon_thunk.call
10421041
// CHECK:STDOUT: name_binding_decl {
10431042
// CHECK:STDOUT: %a.patt: %pattern_type.aa5 = value_binding_pattern a [concrete]
10441043
// CHECK:STDOUT: }
@@ -1055,8 +1054,7 @@ fn F() {
10551054
// CHECK:STDOUT: %.loc13_50.2: %S = bind_value %.loc13_50.1
10561055
// CHECK:STDOUT: %.loc13_50.3: ref %S = value_as_ref %.loc13_50.2
10571056
// CHECK:STDOUT: %addr.loc13_58.1: %ptr.5c7 = addr_of %.loc13_50.3
1058-
// CHECK:STDOUT: %.loc13_58.2: ref %Optional.143 = temporary_storage
1059-
// CHECK:STDOUT: %Indirect__carbon_thunk.call: init %Optional.143 = call imports.%Indirect__carbon_thunk.decl(%addr.loc13_58.1) to %.loc13_58.2
1057+
// CHECK:STDOUT: %Indirect__carbon_thunk.call: init %Optional.143 = call imports.%Indirect__carbon_thunk.decl(%addr.loc13_58.1) to %.loc13_58.1
10601058
// CHECK:STDOUT: %.loc13_30.1: type = splice_block %Optional [concrete = constants.%Optional.143] {
10611059
// CHECK:STDOUT: %Core.ref: <namespace> = name_ref Core, imports.%Core [concrete = imports.%Core]
10621060
// CHECK:STDOUT: %Optional.ref: %Optional.type = name_ref Optional, imports.%Core.Optional [concrete = constants.%Optional.generic]
@@ -1067,23 +1065,23 @@ fn F() {
10671065
// CHECK:STDOUT: %.loc13_30.2: %OptionalStorage.type = converted %ptr, %OptionalStorage.facet [concrete = constants.%OptionalStorage.facet]
10681066
// CHECK:STDOUT: %Optional: type = class_type @Optional, @Optional(constants.%OptionalStorage.facet) [concrete = constants.%Optional.143]
10691067
// CHECK:STDOUT: }
1070-
// CHECK:STDOUT: %.loc13_58.3: ref %Optional.143 = temporary %.loc13_58.2, %Indirect__carbon_thunk.call
1071-
// CHECK:STDOUT: %.loc13_58.4: %Optional.143 = bind_value %.loc13_58.3
1072-
// CHECK:STDOUT: %a: %Optional.143 = value_binding a, %.loc13_58.4
1073-
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.bound.loc13_58: <bound method> = bound_method %.loc13_58.3, constants.%DestroyT.binding.as_type.as.Destroy.impl.Op.465
1068+
// CHECK:STDOUT: %.loc13_58.2: ref %Optional.143 = temporary %.loc13_58.1, %Indirect__carbon_thunk.call
1069+
// CHECK:STDOUT: %.loc13_58.3: %Optional.143 = bind_value %.loc13_58.2
1070+
// CHECK:STDOUT: %a: %Optional.143 = value_binding a, %.loc13_58.3
1071+
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.bound.loc13_58: <bound method> = bound_method %.loc13_58.2, constants.%DestroyT.binding.as_type.as.Destroy.impl.Op.465
10741072
// CHECK:STDOUT: <elided>
1075-
// CHECK:STDOUT: %bound_method.loc13_58: <bound method> = bound_method %.loc13_58.3, %DestroyT.binding.as_type.as.Destroy.impl.Op.specific_fn.1
1076-
// CHECK:STDOUT: %addr.loc13_58.2: %ptr.451 = addr_of %.loc13_58.3
1073+
// CHECK:STDOUT: %bound_method.loc13_58: <bound method> = bound_method %.loc13_58.2, %DestroyT.binding.as_type.as.Destroy.impl.Op.specific_fn.1
1074+
// CHECK:STDOUT: %addr.loc13_58.2: %ptr.451 = addr_of %.loc13_58.2
10771075
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.call.loc13_58: init %empty_tuple.type = call %bound_method.loc13_58(%addr.loc13_58.2)
10781076
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.bound.loc13_48: <bound method> = bound_method %.loc13_48.4, constants.%DestroyT.binding.as_type.as.Destroy.impl.Op.016
10791077
// CHECK:STDOUT: <elided>
10801078
// CHECK:STDOUT: %bound_method.loc13_48: <bound method> = bound_method %.loc13_48.4, %DestroyT.binding.as_type.as.Destroy.impl.Op.specific_fn.2
10811079
// CHECK:STDOUT: %addr.loc13_48: %ptr.5c7 = addr_of %.loc13_48.4
10821080
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.call.loc13_48: init %empty_tuple.type = call %bound_method.loc13_48(%addr.loc13_48)
1083-
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.bound.loc11_16: <bound method> = bound_method %.loc11_16.3, constants.%DestroyT.binding.as_type.as.Destroy.impl.Op.465
1081+
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.bound.loc11_16: <bound method> = bound_method %.loc11_16.2, constants.%DestroyT.binding.as_type.as.Destroy.impl.Op.465
10841082
// CHECK:STDOUT: <elided>
1085-
// CHECK:STDOUT: %bound_method.loc11_16: <bound method> = bound_method %.loc11_16.3, %DestroyT.binding.as_type.as.Destroy.impl.Op.specific_fn.3
1086-
// CHECK:STDOUT: %addr.loc11_16: %ptr.451 = addr_of %.loc11_16.3
1083+
// CHECK:STDOUT: %bound_method.loc11_16: <bound method> = bound_method %.loc11_16.2, %DestroyT.binding.as_type.as.Destroy.impl.Op.specific_fn.3
1084+
// CHECK:STDOUT: %addr.loc11_16: %ptr.451 = addr_of %.loc11_16.2
10871085
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.call.loc11_16: init %empty_tuple.type = call %bound_method.loc11_16(%addr.loc11_16)
10881086
// CHECK:STDOUT: %DestroyT.binding.as_type.as.Destroy.impl.Op.bound.loc11_14: <bound method> = bound_method %.loc11_14.3, constants.%DestroyT.binding.as_type.as.Destroy.impl.Op.465
10891087
// CHECK:STDOUT: <elided>

0 commit comments

Comments
 (0)