Skip to content

Commit 7544e6c

Browse files
nate-chandlernate-chandler
committed
[SILCombine] Fix apply(convert_func(@guaranteed)).
The operands to the original apply are cast via an ownership forwarding instruction to the appropriate type for the rewritten apply. ``` %converted = convert_function %original to $(NewTy) -> () apply %converted(%operand) ``` -> ``` %cast = cast %operand to $OriginalTy apply %original(%cast) ``` Previously, when an original operand is owned but the new apply does not consume that operand, the newly added cast would consume the original operand (an owned value)--something the original code being replaced did not do. ``` %converted = convert_function %original to $(NewTy) -> () apply %converted(%operand : @guaranteed) // %operand remains available ``` -> ``` %cast = cast %operand to $OriginalTy // consumes %operand! apply %original(%cast : @guaranteed) // %operand is not available! ``` This is incorrect for the complementary reasons that the result of the cast is leaked and any uses of the original operand subsequent to the new apply are uses-after-consume. Here, this is fixed by borrowing the original operand before casting in this case. rdar://142570727
1 parent d8c49df commit 7544e6c

File tree

2 files changed

+184
-8
lines changed

2 files changed

+184
-8
lines changed

‎lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,22 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
173173
auto newOpParamTypes = convertConventions.getParameterSILTypes(context);
174174

175175
llvm::SmallVector<SILValue, 8> Args;
176-
auto convertOp = [&](SILValue Op, SILType OldOpType, SILType NewOpType) {
176+
llvm::SmallVector<BeginBorrowInst *, 8> Borrows;
177+
auto convertOp = [&](SILValue Op, SILType OldOpType, SILType NewOpType,
178+
OperandOwnership ownership) {
177179
// Convert function takes refs to refs, address to addresses, and leaves
178180
// other types alone.
179181
if (OldOpType.isAddress()) {
180182
assert(NewOpType.isAddress() && "Addresses should map to addresses.");
181183
auto UAC = Builder.createUncheckedAddrCast(AI.getLoc(), Op, NewOpType);
182184
Args.push_back(UAC);
183185
} else if (OldOpType.getASTType() != NewOpType.getASTType()) {
186+
if (Op->getOwnershipKind() == OwnershipKind::Owned &&
187+
!ownership.getOwnershipConstraint().isConsuming()) {
188+
auto borrow = Builder.createBeginBorrow(AI.getLoc(), Op);
189+
Op = borrow;
190+
Borrows.push_back(borrow);
191+
}
184192
auto URC =
185193
Builder.createUncheckedForwardingCast(AI.getLoc(), Op, NewOpType);
186194
Args.push_back(URC);
@@ -194,22 +202,30 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
194202
auto newRetI = newOpRetTypes.begin();
195203
auto oldRetI = oldOpRetTypes.begin();
196204

205+
auto getCurrentOperand = [&OpI, &AI]() -> Operand & {
206+
return AI.getInstruction()
207+
->getAllOperands()[OpI + ApplyInst::getArgumentOperandNumber()];
208+
};
209+
197210
for (auto e = newOpRetTypes.end(); newRetI != e;
198211
++OpI, ++newRetI, ++oldRetI) {
199-
convertOp(Ops[OpI], *oldRetI, *newRetI);
212+
convertOp(Ops[OpI], *oldRetI, *newRetI,
213+
getCurrentOperand().getOperandOwnership());
200214
}
201215

202216
if (oldIndirectErrorResultType) {
203217
assert(newIndirectErrorResultType);
204-
convertOp(Ops[OpI], oldIndirectErrorResultType, newIndirectErrorResultType);
218+
convertOp(Ops[OpI], oldIndirectErrorResultType, newIndirectErrorResultType,
219+
getCurrentOperand().getOperandOwnership());
205220
++OpI;
206221
}
207222

208223
auto newParamI = newOpParamTypes.begin();
209224
auto oldParamI = oldOpParamTypes.begin();
210225
for (auto e = newOpParamTypes.end(); newParamI != e;
211226
++OpI, ++newParamI, ++oldParamI) {
212-
convertOp(Ops[OpI], *oldParamI, *newParamI);
227+
convertOp(Ops[OpI], *oldParamI, *newParamI,
228+
getCurrentOperand().getOperandOwnership());
213229
}
214230

215231
// Convert the direct results if they changed.
@@ -249,9 +265,20 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
249265
}
250266

251267
Builder.setInsertionPoint(AI.getInstruction());
252-
return Builder.createTryApply(AI.getLoc(), funcOper, SubstitutionMap(), Args,
253-
normalBB, TAI->getErrorBB(),
254-
TAI->getApplyOptions());
268+
auto *result = Builder.createTryApply(
269+
AI.getLoc(), funcOper, SubstitutionMap(), Args, normalBB,
270+
TAI->getErrorBB(), TAI->getApplyOptions());
271+
if (!Borrows.empty()) {
272+
Builder.setInsertionPoint(&TAI->getErrorBB()->front());
273+
for (auto *borrow : Borrows) {
274+
Builder.createEndBorrow(AI.getLoc(), borrow);
275+
}
276+
Builder.setInsertionPoint(&TAI->getNormalBB()->front());
277+
for (auto *borrow : Borrows) {
278+
Builder.createEndBorrow(AI.getLoc(), borrow);
279+
}
280+
}
281+
return result;
255282
}
256283

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

300+
Builder.setInsertionPoint(AI->getNextInstruction());
301+
for (auto *borrow : Borrows) {
302+
Builder.createEndBorrow(AI.getLoc(), borrow);
303+
}
304+
273305
return result;
274306
}
275307

‎test/SILOptimizer/sil_combine_apply_unit.sil

Lines changed: 145 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,28 @@
1-
// RUN: %target-sil-opt -enable-sil-verify-all %s -test-runner | %FileCheck %s
1+
// RUN: %target-sil-opt \
2+
// RUN: -test-runner %s \
3+
// RUN: -module-name Swift \
4+
// RUN: -enable-sil-verify-all \
5+
// RUN: | %FileCheck %s
26

37
import Builtin
48

9+
enum Optional<T> {
10+
case some(T)
11+
case none
12+
}
13+
14+
protocol Error {}
15+
16+
class C {}
17+
518
struct Input {}
619
struct Output {}
720
enum Nunca {}
821

22+
sil @borrowMaybeC : $@convention(thin) (@guaranteed Optional<C>) -> ()
23+
sil @borrowMaybeC2 : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> ()
24+
sil @borrowMaybeCThrowing : $@convention(thin) (@guaranteed Optional<C>) -> (@error Error)
25+
sil @borrowMaybeC2Throwing : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> (@error Error)
926
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>
1027

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

51+
// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__1 : {{.*}} {
52+
// CHECK: bb0([[C:%[^,]+]] :
53+
// CHECK: [[BORROW_MAYBE_C:%[^,]+]] = function_ref @borrowMaybeC
54+
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
55+
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] to $Optional<C>
56+
// CHECK: apply [[BORROW_MAYBE_C]]([[MAYBE_B]])
57+
// CHECK: end_borrow [[B]]
58+
// CHECK: destroy_value [[C]]
59+
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__1'
60+
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__1 : $@convention(thin) (@owned C) -> () {
61+
entry(%c : @owned $C):
62+
%borrowMaybeC = function_ref @borrowMaybeC : $@convention(thin) (@guaranteed Optional<C>) -> ()
63+
%borrowC = convert_function %borrowMaybeC to $@convention(thin) (@guaranteed C) -> ()
64+
%void = apply %borrowC(%c) : $@convention(thin) (@guaranteed C) -> ()
65+
specify_test "sil_combine_instruction %void"
66+
destroy_value %c
67+
%retval = tuple ()
68+
return %retval
69+
}
70+
71+
// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__2 : {{.*}} {
72+
// CHECK: bb0(
73+
// CHECK-SAME: [[C:%[^,]+]] :
74+
// CHECK-SAME: [[C2:%[^,]+]] :
75+
// CHECK-SAME: ):
76+
// CHECK: [[BORROW_MAYBE_C2:%[^,]+]] = function_ref @borrowMaybeC2
77+
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
78+
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] to $Optional<C>
79+
// CHECK: [[B2:%[^,]+]] = begin_borrow [[C2]]
80+
// CHECK: [[MAYBE_B2:%[^,]+]] = unchecked_ref_cast [[B2]] to $Optional<C>
81+
// CHECK: apply [[BORROW_MAYBE_C2]]([[MAYBE_B]], [[MAYBE_B2]])
82+
// CHECK: end_borrow [[B]]
83+
// CHECK: end_borrow [[B2]]
84+
// CHECK: destroy_value [[C]]
85+
// CHECK: destroy_value [[C2]]
86+
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__2'
87+
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__2 : $@convention(thin) (@owned C, @owned C) -> () {
88+
entry(%c : @owned $C, %c2 : @owned $C):
89+
%borrowMaybeC2 = function_ref @borrowMaybeC2 : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> ()
90+
%borrowC2 = convert_function %borrowMaybeC2 to $@convention(thin) (@guaranteed C, @guaranteed C) -> ()
91+
%void = apply %borrowC2(%c, %c2) : $@convention(thin) (@guaranteed C, @guaranteed C) -> ()
92+
specify_test "sil_combine_instruction %void"
93+
destroy_value %c
94+
destroy_value %c2
95+
%retval = tuple ()
96+
return %retval
97+
}
98+
99+
// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__3 : {{.*}} {
100+
// CHECK: bb0([[C:%[^,]+]] :
101+
// CHECK: [[BORROW_MAYBE_C:%[^,]+]] = function_ref @borrowMaybeCThrowing
102+
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
103+
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] to $Optional<C>
104+
// CHECK: try_apply [[BORROW_MAYBE_C]]([[MAYBE_B]])
105+
// CHECK: normal [[SUCCESS:bb[0-9]+]]
106+
// CHECK: error [[FAILURE:bb[0-9]+]]
107+
// CHECK: [[SUCCESS]]
108+
// CHECK: end_borrow [[B]]
109+
// CHECK: destroy_value [[C]]
110+
// CHECK: [[FAILURE]]([[ERROR:%[^,]+]] :
111+
// CHECK: end_borrow [[B]]
112+
// CHECK: destroy_value [[C]]
113+
// CHECK: throw [[ERROR]]
114+
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__3'
115+
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__3 : $@convention(thin) (@owned C) -> (@error Error) {
116+
entry(%c : @owned $C):
117+
%borrowMaybeC = function_ref @borrowMaybeCThrowing : $@convention(thin) (@guaranteed Optional<C>) -> (@error Error)
118+
%borrowC = convert_function %borrowMaybeC to $@convention(thin) (@guaranteed C) -> (@error Error)
119+
specify_test "sil_combine_instruction @instruction"
120+
try_apply %borrowC(%c) : $@convention(thin) (@guaranteed C) -> (@error Error),
121+
normal success,
122+
error failure
123+
124+
success(%void : $()):
125+
destroy_value %c
126+
%retval = tuple ()
127+
return %retval
128+
129+
failure(%error : @owned $Error):
130+
destroy_value %c
131+
throw %error
132+
}
34133

134+
// CHECK-LABEL: sil [ossa] @convert_function__to_optional__owned_as_guaranteed__4 : {{.*}} {
135+
// CHECK: bb0(
136+
// CHECK-SAME: [[C:%[^,]+]] :
137+
// CHECK-SAME: [[C2:%[^,]+]] :
138+
// CHECK-SAME: ):
139+
// CHECK: [[BORROW_MAYBE_C2:%[^,]+]] = function_ref @borrowMaybeC2Throwing
140+
// CHECK: [[B:%[^,]+]] = begin_borrow [[C]]
141+
// CHECK: [[MAYBE_B:%[^,]+]] = unchecked_ref_cast [[B]] to $Optional<C>
142+
// CHECK: [[B2:%[^,]+]] = begin_borrow [[C2]]
143+
// CHECK: [[MAYBE_B2:%[^,]+]] = unchecked_ref_cast [[B2]] to $Optional<C>
144+
// CHECK: try_apply [[BORROW_MAYBE_C2]]([[MAYBE_B]], [[MAYBE_B2]])
145+
// CHECK: normal [[SUCCESS:bb[0-9]+]]
146+
// CHECK: error [[FAILURE:bb[0-9]+]]
147+
// CHECK: [[SUCCESS]]
148+
// CHECK: end_borrow [[B]]
149+
// CHECK: end_borrow [[B2]]
150+
// CHECK: destroy_value [[C]]
151+
// CHECK: destroy_value [[C2]]
152+
// CHECK: [[FAILURE]]([[ERROR:%[^,]+]] :
153+
// CHECK: end_borrow [[B]]
154+
// CHECK: end_borrow [[B2]]
155+
// CHECK: destroy_value [[C]]
156+
// CHECK: destroy_value [[C2]]
157+
// CHECK: throw [[ERROR]]
158+
// CHECK-LABEL: } // end sil function 'convert_function__to_optional__owned_as_guaranteed__4'
159+
sil [ossa] @convert_function__to_optional__owned_as_guaranteed__4 : $@convention(thin) (@owned C, @owned C) -> (@error Error) {
160+
entry(%c : @owned $C, %c2 : @owned $C):
161+
%borrowMaybeC2 = function_ref @borrowMaybeC2Throwing : $@convention(thin) (@guaranteed Optional<C>, @guaranteed Optional<C>) -> (@error Error)
162+
%borrowC2 = convert_function %borrowMaybeC2 to $@convention(thin) (@guaranteed C, @guaranteed C) -> (@error Error)
163+
specify_test "sil_combine_instruction @instruction"
164+
try_apply %borrowC2(%c, %c2) : $@convention(thin) (@guaranteed C, @guaranteed C) -> (@error Error),
165+
normal success,
166+
error failure
167+
168+
success(%void : $()):
169+
destroy_value %c
170+
destroy_value %c2
171+
%retval = tuple ()
172+
return %retval
173+
174+
failure(%error : @owned $Error):
175+
destroy_value %c
176+
destroy_value %c2
177+
throw %error
178+
}

0 commit comments

Comments
 (0)