Skip to content

Commit

Permalink
Merge pull request swiftlang#78529 from nate-chandler/cherrypick/rele…
Browse files Browse the repository at this point in the history 
…ase/6.1/rdar142570727

6.1: [SILCombine] Fix apply(convert_function(@guaranteed)) with an @owned operand.
  • Loading branch information
@nate-chandler
nate-chandler authored Jan 10, 2025
2 parents 20e30f9 + a9f4a38 commit 2f7b5e4
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 11 deletions.
60 changes: 52 additions & 8 deletions lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,13 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
if (SubstCalleeTy->hasArchetype() || ConvertCalleeTy->hasArchetype())
return nullptr;

// If we converted from a non-throwing to a throwing function which is
// try_apply'd, rewriting would require changing the CFG. Bail for now.
if (!ConvertCalleeTy->hasErrorResult() && isa<TryApplyInst>(AI)) {
assert(SubstCalleeTy->hasErrorResult());
return nullptr;
}

// Ok, we can now perform our transformation. Grab AI's operands and the
// relevant types from the ConvertFunction function type and AI.
Builder.setCurrentDebugScope(AI.getDebugScope());
Expand All @@ -166,14 +173,22 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
auto newOpParamTypes = convertConventions.getParameterSILTypes(context);

llvm::SmallVector<SILValue, 8> Args;
auto convertOp = [&](SILValue Op, SILType OldOpType, SILType NewOpType) {
llvm::SmallVector<BeginBorrowInst *, 8> Borrows;
auto convertOp = [&](SILValue Op, SILType OldOpType, SILType NewOpType,
OperandOwnership ownership) {
// Convert function takes refs to refs, address to addresses, and leaves
// other types alone.
if (OldOpType.isAddress()) {
assert(NewOpType.isAddress() && "Addresses should map to addresses.");
auto UAC = Builder.createUncheckedAddrCast(AI.getLoc(), Op, NewOpType);
Args.push_back(UAC);
} else if (OldOpType.getASTType() != NewOpType.getASTType()) {
if (Op->getOwnershipKind() == OwnershipKind::Owned &&
!ownership.getOwnershipConstraint().isConsuming()) {
auto borrow = Builder.createBeginBorrow(AI.getLoc(), Op);
Op = borrow;
Borrows.push_back(borrow);
}
auto URC =
Builder.createUncheckedForwardingCast(AI.getLoc(), Op, NewOpType);
Args.push_back(URC);
Expand All @@ -187,22 +202,30 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
auto newRetI = newOpRetTypes.begin();
auto oldRetI = oldOpRetTypes.begin();

auto getCurrentOperand = [&OpI, &AI]() -> Operand & {
return AI.getInstruction()
->getAllOperands()[OpI + ApplyInst::getArgumentOperandNumber()];
};

for (auto e = newOpRetTypes.end(); newRetI != e;
++OpI, ++newRetI, ++oldRetI) {
convertOp(Ops[OpI], *oldRetI, *newRetI);
convertOp(Ops[OpI], *oldRetI, *newRetI,
getCurrentOperand().getOperandOwnership());
}

if (oldIndirectErrorResultType) {
assert(newIndirectErrorResultType);
convertOp(Ops[OpI], oldIndirectErrorResultType, newIndirectErrorResultType);
convertOp(Ops[OpI], oldIndirectErrorResultType, newIndirectErrorResultType,
getCurrentOperand().getOperandOwnership());
++OpI;
}

auto newParamI = newOpParamTypes.begin();
auto oldParamI = oldOpParamTypes.begin();
for (auto e = newOpParamTypes.end(); newParamI != e;
++OpI, ++newParamI, ++oldParamI) {
convertOp(Ops[OpI], *oldParamI, *newParamI);
convertOp(Ops[OpI], *oldParamI, *newParamI,
getCurrentOperand().getOperandOwnership());
}

// Convert the direct results if they changed.
Expand Down Expand Up @@ -240,10 +263,22 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,

Builder.createBranch(AI.getLoc(), TAI->getNormalBB(), branchArgs);
}

return Builder.createTryApply(AI.getLoc(), funcOper, SubstitutionMap(), Args,
normalBB, TAI->getErrorBB(),
TAI->getApplyOptions());

Builder.setInsertionPoint(AI.getInstruction());
auto *result = Builder.createTryApply(
AI.getLoc(), funcOper, SubstitutionMap(), Args, normalBB,
TAI->getErrorBB(), TAI->getApplyOptions());
if (!Borrows.empty()) {
Builder.setInsertionPoint(&TAI->getErrorBB()->front());
for (auto *borrow : Borrows) {
Builder.createEndBorrow(AI.getLoc(), borrow);
}
Builder.setInsertionPoint(&TAI->getNormalBB()->front());
for (auto *borrow : Borrows) {
Builder.createEndBorrow(AI.getLoc(), borrow);
}
}
return result;
}

// Match the throwing bit of the underlying function_ref. We assume that if
Expand All @@ -262,6 +297,11 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
Builder.createUncheckedForwardingCast(AI.getLoc(), NAI, oldResultTy);
}

Builder.setInsertionPoint(AI->getNextInstruction());
for (auto *borrow : Borrows) {
Builder.createEndBorrow(AI.getLoc(), borrow);
}

return result;
}

Expand Down Expand Up @@ -1623,6 +1663,10 @@ SILInstruction *SILCombiner::visitTryApplyInst(TryApplyInst *AI) {
if (isa<PartialApplyInst>(AI->getCallee()))
return nullptr;

if (auto *CFI = dyn_cast<ConvertFunctionInst>(AI->getCallee())) {
return optimizeApplyOfConvertFunctionInst(AI, CFI);
}

// Optimize readonly functions with no meaningful users.
SILFunction *Fn = AI->getReferencedFunctionOrNull();
if (Fn && Fn->getEffectsKind() < EffectsKind::ReleaseNone) {
Expand Down
4 changes: 3 additions & 1 deletion test/SILOptimizer/sil_combine.sil
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,9 @@ bb0(%0 : $*@callee_owned (@in ()) -> @out AnotherClass, %1 : $*AnotherClass):

sil @convertible_result_with_error : $@convention(thin) () -> (@owned AnotherClass, @error Error)

// TODO
// CHECK-LABEL: sil @peephole_convert_function_result_change_with_error : {{.*}} {
// CHECK-NOT: convert_function
// CHECK-LABEL: } // end sil function 'peephole_convert_function_result_change_with_error'
sil @peephole_convert_function_result_change_with_error : $@convention(thin) () -> () {
entry:
%f = function_ref @convertible_result_with_error : $@convention(thin) () -> (@owned AnotherClass, @error Error)
Expand Down
146 changes: 145 additions & 1 deletion test/SILOptimizer/sil_combine_apply_unit.sil
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
// RUN: %target-sil-opt -enable-sil-verify-all %s -test-runner | %FileCheck %s
// RUN: %target-sil-opt \
// RUN: -test-runner %s \
// RUN: -module-name Swift \
// RUN: -enable-sil-verify-all \
// RUN: | %FileCheck %s

import Builtin

enum Optional<T> {
case some(T)
case none
}

protocol Error {}

class C {}

struct Input {}
struct Output {}
enum Nunca {}

sil @borrowMaybeC : $@convention(thin) (@guaranteed Optional<C>) -> ()
sil @borrowMaybeC2 : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> ()
sil @borrowMaybeCThrowing : $@convention(thin) (@guaranteed Optional<C>) -> (@error Error)
sil @borrowMaybeC2Throwing : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> (@error Error)
sil @rdar127452206_callee : $@convention(thin) @Sendable @substituted <τ_0_0, τ_0_1, τ_0_2> (@in_guaranteed τ_0_0) -> (@out τ_0_2, @error_indirect τ_0_1) for <Input, Nunca, Output>

// CHECK-LABEL: sil @rdar127452206 : {{.*}} {
Expand All @@ -31,4 +48,131 @@ entry(%input : $*Input):
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__1 : {{.*}} {
// CHECK: bb0([[C:%[^,]+]] :
// CHECK: [[BORROW_MAYBE_C:%[^,]+]] = function_ref @borrowMaybeC
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] : $C to $Optional<C>
// CHECK: apply [[BORROW_MAYBE_C]]([[MAYBE_B]])
// CHECK: end_borrow [[B]]
// CHECK: destroy_value [[C]]
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__1'
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__1 : $@convention(thin) (@owned C) -> () {
entry(%c : @owned $C):
%borrowMaybeC = function_ref @borrowMaybeC : $@convention(thin) (@guaranteed Optional<C>) -> ()
%borrowC = convert_function %borrowMaybeC : $@convention(thin) (@guaranteed Optional<C>) -> () to $@convention(thin) (@guaranteed C) -> ()
%void = apply %borrowC(%c) : $@convention(thin) (@guaranteed C) -> ()
specify_test "sil_combine_instruction %void"
destroy_value %c : $C
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__2 : {{.*}} {
// CHECK: bb0(
// CHECK-SAME: [[C:%[^,]+]] :
// CHECK-SAME: [[C2:%[^,]+]] :
// CHECK-SAME: ):
// CHECK: [[BORROW_MAYBE_C2:%[^,]+]] = function_ref @borrowMaybeC2
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] : $C to $Optional<C>
// CHECK: [[B2:%[^,]+]] = begin_borrow [[C2]]
// CHECK: [[MAYBE_B2:%[^,]+]] = unchecked_ref_cast [[B2]] : $C to $Optional<C>
// CHECK: apply [[BORROW_MAYBE_C2]]([[MAYBE_B]], [[MAYBE_B2]])
// CHECK: end_borrow [[B]]
// CHECK: end_borrow [[B2]]
// CHECK: destroy_value [[C]]
// CHECK: destroy_value [[C2]]
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__2'
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__2 : $@convention(thin) (@owned C, @owned C) -> () {
entry(%c : @owned $C, %c2 : @owned $C):
%borrowMaybeC2 = function_ref @borrowMaybeC2 : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> ()
%borrowC2 = convert_function %borrowMaybeC2 : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> () to $@convention(thin) (@guaranteed C, @guaranteed C) -> ()
%void = apply %borrowC2(%c, %c2) : $@convention(thin) (@guaranteed C, @guaranteed C) -> ()
specify_test "sil_combine_instruction %void"
destroy_value %c : $C
destroy_value %c2 : $C
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__3 : {{.*}} {
// CHECK: bb0([[C:%[^,]+]] :
// CHECK: [[BORROW_MAYBE_C:%[^,]+]] = function_ref @borrowMaybeCThrowing
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] : $C to $Optional<C>
// CHECK: try_apply [[BORROW_MAYBE_C]]([[MAYBE_B]])
// CHECK: normal [[SUCCESS:bb[0-9]+]]
// CHECK: error [[FAILURE:bb[0-9]+]]
// CHECK: [[SUCCESS]]
// CHECK: end_borrow [[B]]
// CHECK: destroy_value [[C]]
// CHECK: [[FAILURE]]([[ERROR:%[^,]+]] :
// CHECK: end_borrow [[B]]
// CHECK: destroy_value [[C]]
// CHECK: throw [[ERROR]]
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__3'
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__3 : $@convention(thin) (@owned C) -> (@error Error) {
entry(%c : @owned $C):
%borrowMaybeC = function_ref @borrowMaybeCThrowing : $@convention(thin) (@guaranteed Optional<C>) -> (@error Error)
%borrowC = convert_function %borrowMaybeC : $@convention(thin) (@guaranteed Optional<C>) -> (@error Error) to $@convention(thin) (@guaranteed C) -> (@error Error)
specify_test "sil_combine_instruction @instruction"
try_apply %borrowC(%c) : $@convention(thin) (@guaranteed C) -> (@error Error),
normal success,
error failure

success(%void : $()):
destroy_value %c : $C
%retval = tuple ()
return %retval : $()

failure(%error : @owned $Error):
destroy_value %c : $C
throw %error : $Error
}

// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__4 : {{.*}} {
// CHECK: bb0(
// CHECK-SAME: [[C:%[^,]+]] :
// CHECK-SAME: [[C2:%[^,]+]] :
// CHECK-SAME: ):
// CHECK: [[BORROW_MAYBE_C2:%[^,]+]] = function_ref @borrowMaybeC2Throwing
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] : $C to $Optional<C>
// CHECK: [[B2:%[^,]+]] = begin_borrow [[C2]]
// CHECK: [[MAYBE_B2:%[^,]+]] = unchecked_ref_cast [[B2]] : $C to $Optional<C>
// CHECK: try_apply [[BORROW_MAYBE_C2]]([[MAYBE_B]], [[MAYBE_B2]])
// CHECK: normal [[SUCCESS:bb[0-9]+]]
// CHECK: error [[FAILURE:bb[0-9]+]]
// CHECK: [[SUCCESS]]
// CHECK: end_borrow [[B]]
// CHECK: end_borrow [[B2]]
// CHECK: destroy_value [[C]]
// CHECK: destroy_value [[C2]]
// CHECK: [[FAILURE]]([[ERROR:%[^,]+]] :
// CHECK: end_borrow [[B]]
// CHECK: end_borrow [[B2]]
// CHECK: destroy_value [[C]]
// CHECK: destroy_value [[C2]]
// CHECK: throw [[ERROR]]
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__4'
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__4 : $@convention(thin) (@owned C, @owned C) -> (@error Error) {
entry(%c : @owned $C, %c2 : @owned $C):
%borrowMaybeC2 = function_ref @borrowMaybeC2Throwing : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> (@error Error)
%borrowC2 = convert_function %borrowMaybeC2 : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> (@error Error) to $@convention(thin) (@guaranteed C, @guaranteed C) -> (@error Error)
specify_test "sil_combine_instruction @instruction"
try_apply %borrowC2(%c, %c2) : $@convention(thin) (@guaranteed C, @guaranteed C) -> (@error Error),
normal success,
error failure

success(%void : $()):
destroy_value %c : $C
destroy_value %c2 : $C
%retval = tuple ()
return %retval : $()

failure(%error : @owned $Error):
destroy_value %c : $C
destroy_value %c2 : $C
throw %error : $Error
}
4 changes: 3 additions & 1 deletion test/SILOptimizer/sil_combine_ossa.sil
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1745,7 +1745,9 @@ bb0(%0 : $*@callee_owned (@in ()) -> @out AnotherClass, %1 : $*AnotherClass):

sil @convertible_result_with_error : $@convention(thin) () -> (@owned AnotherClass, @error Error)

// TODO
// CHECK-LABEL: sil [ossa] @peephole_convert_function_result_change_with_error : {{.*}} {
// CHECK-NOT: convert_function
// CHECK-LABEL: } // end sil function 'peephole_convert_function_result_change_with_error'
sil [ossa] @peephole_convert_function_result_change_with_error : $@convention(thin) () -> () {
entry:
%f = function_ref @convertible_result_with_error : $@convention(thin) () -> (@owned AnotherClass, @error Error)
Expand Down

0 comments on commit 2f7b5e4

Please sign in to comment.