Skip to content

Commit 2e9c904

Browse files
gottesmmgottesmm
committed
[mandatory-inlining] Teach mandatory inlining how to handle begin_borrow instructions.
This was exposed by test/SILOptimizer/diagnostic_constant_propagation.swift. It isn't a pattern in the mandatory inlining tests. I added some tests for it.
1 parent 6a2123f commit 2e9c904

File tree

3 files changed

+125
-21
lines changed

3 files changed

+125
-21
lines changed

‎lib/SILOptimizer/Mandatory/MandatoryInlining.cpp

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
4646
Context.Diags.diagnose(loc, diag, std::forward<U>(args)...);
4747
}
4848

49-
static SILValue stripCopies(SILValue v) {
50-
while (auto *cvi = dyn_cast<CopyValueInst>(v)) {
51-
v = cvi->getOperand();
49+
static SILValue stripCopiesAndBorrows(SILValue v) {
50+
while (isa<CopyValueInst>(v) || isa<BeginBorrowInst>(v)) {
51+
v = cast<SingleValueInstruction>(v)->getOperand(0);
5252
}
5353
return v;
5454
}
@@ -329,7 +329,7 @@ static void cleanupCalleeValue(SILValue calleeValue) {
329329
}
330330
}
331331

332-
calleeValue = stripCopies(calleeValue);
332+
calleeValue = stripCopiesAndBorrows(calleeValue);
333333

334334
// Inline constructor
335335
auto calleeSource = ([&]() -> SILValue {
@@ -339,12 +339,12 @@ static void cleanupCalleeValue(SILValue calleeValue) {
339339
// will delete any uses of the closure, including a
340340
// convert_escape_to_noescape conversion.
341341
if (auto *cfi = dyn_cast<ConvertFunctionInst>(calleeValue))
342-
return stripCopies(cfi->getOperand());
342+
return stripCopiesAndBorrows(cfi->getOperand());
343343

344344
if (auto *cvt = dyn_cast<ConvertEscapeToNoEscapeInst>(calleeValue))
345-
return stripCopies(cvt->getOperand());
345+
return stripCopiesAndBorrows(cvt->getOperand());
346346

347-
return stripCopies(calleeValue);
347+
return stripCopiesAndBorrows(calleeValue);
348348
})();
349349

350350
if (auto *pai = dyn_cast<PartialApplyInst>(calleeSource)) {
@@ -359,7 +359,7 @@ static void cleanupCalleeValue(SILValue calleeValue) {
359359
calleeValue = callee;
360360
}
361361

362-
calleeValue = stripCopies(calleeValue);
362+
calleeValue = stripCopiesAndBorrows(calleeValue);
363363

364364
// Handle function_ref -> convert_function -> partial_apply/thin_to_thick.
365365
if (auto *cfi = dyn_cast<ConvertFunctionInst>(calleeValue)) {
@@ -579,7 +579,7 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
579579

580580
// Then grab a first approximation of our apply by stripping off all copy
581581
// operations.
582-
SILValue CalleeValue = stripCopies(AI.getCallee());
582+
SILValue CalleeValue = stripCopiesAndBorrows(AI.getCallee());
583583

584584
// If after stripping off copy_values, we have a load then see if we the
585585
// function we want to inline has a simple available value through a simple
@@ -588,7 +588,7 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
588588
CalleeValue = getLoadedCalleeValue(li);
589589
if (!CalleeValue)
590590
return nullptr;
591-
CalleeValue = stripCopies(CalleeValue);
591+
CalleeValue = stripCopiesAndBorrows(CalleeValue);
592592
}
593593

594594
// PartialApply/ThinToThick -> ConvertFunction patterns are generated
@@ -599,7 +599,7 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
599599
// a cast.
600600
auto skipFuncConvert = [](SILValue CalleeValue) {
601601
// Skip any copies that we see.
602-
CalleeValue = stripCopies(CalleeValue);
602+
CalleeValue = stripCopiesAndBorrows(CalleeValue);
603603

604604
// We can also allow a thin @escape to noescape conversion as such:
605605
// %1 = function_ref @thin_closure_impl : $@convention(thin) () -> ()
@@ -619,7 +619,7 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
619619
ToCalleeTy->getExtInfo().withNoEscape(false));
620620
if (FromCalleeTy != EscapingCalleeTy)
621621
return CalleeValue;
622-
return stripCopies(ThinToNoescapeCast->getOperand());
622+
return stripCopiesAndBorrows(ThinToNoescapeCast->getOperand());
623623
}
624624

625625
// Ignore mark_dependence users. A partial_apply [stack] uses them to mark
@@ -635,7 +635,7 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
635635

636636
auto *CFI = dyn_cast<ConvertEscapeToNoEscapeInst>(CalleeValue);
637637
if (!CFI)
638-
return stripCopies(CalleeValue);
638+
return stripCopiesAndBorrows(CalleeValue);
639639

640640
// TODO: Handle argument conversion. All the code in this file needs to be
641641
// cleaned up and generalized. The argument conversion handling in
@@ -651,9 +651,9 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
651651
auto EscapingCalleeTy =
652652
ToCalleeTy->getWithExtInfo(ToCalleeTy->getExtInfo().withNoEscape(false));
653653
if (FromCalleeTy != EscapingCalleeTy)
654-
return stripCopies(CalleeValue);
654+
return stripCopiesAndBorrows(CalleeValue);
655655

656-
return stripCopies(CFI->getOperand());
656+
return stripCopiesAndBorrows(CFI->getOperand());
657657
};
658658

659659
// Look through a escape to @noescape conversion.
@@ -666,11 +666,11 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,
666666
// Collect the applied arguments and their convention.
667667
collectPartiallyAppliedArguments(PAI, CapturedArgConventions, FullArgs);
668668

669-
CalleeValue = stripCopies(PAI->getCallee());
669+
CalleeValue = stripCopiesAndBorrows(PAI->getCallee());
670670
IsThick = true;
671671
PartialApply = PAI;
672672
} else if (auto *TTTFI = dyn_cast<ThinToThickFunctionInst>(CalleeValue)) {
673-
CalleeValue = stripCopies(TTTFI->getOperand());
673+
CalleeValue = stripCopiesAndBorrows(TTTFI->getOperand());
674674
IsThick = true;
675675
}
676676

‎lib/SILOptimizer/Utils/Local.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,9 @@ swift::tryToConcatenateStrings(ApplyInst *AI, SILBuilder &B) {
931931
// Closure Deletion
932932
//===----------------------------------------------------------------------===//
933933

934+
/// NOTE: Instructions with transitive ownership kind are assumed to not keep
935+
/// the underlying closure alive as well. This is meant for instructions only
936+
/// with non-transitive users.
934937
static bool useDoesNotKeepClosureAlive(const SILInstruction *I) {
935938
switch (I->getKind()) {
936939
case SILInstructionKind::StrongRetainInst:
@@ -939,6 +942,7 @@ static bool useDoesNotKeepClosureAlive(const SILInstruction *I) {
939942
case SILInstructionKind::RetainValueInst:
940943
case SILInstructionKind::ReleaseValueInst:
941944
case SILInstructionKind::DebugValueInst:
945+
case SILInstructionKind::EndBorrowInst:
942946
return true;
943947
default:
944948
return false;
@@ -951,9 +955,9 @@ static bool useHasTransitiveOwnership(const SILInstruction *I) {
951955
if (isa<ConvertEscapeToNoEscapeInst>(I))
952956
return true;
953957

954-
// Look through copy_value. It is inert for our purposes, but we need to look
955-
// through it.
956-
return isa<CopyValueInst>(I);
958+
// Look through copy_value, begin_borrow. They are inert for our purposes, but
959+
// we need to look through it.
960+
return isa<CopyValueInst>(I) || isa<BeginBorrowInst>(I);
957961
}
958962

959963
static SILValue createLifetimeExtendedAllocStack(

‎test/SILOptimizer/mandatory_inlining_ownership.sil

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,11 @@ bb2(%5 : @owned $Error):
8383
throw %5 : $Error
8484
}
8585

86-
// Partial Apply copy_value test.
86+
// Partial Apply copy_value, begin_borrow
8787

8888
sil @nativeobject_plus : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
8989
sil @partial_apply_user : $@convention(thin) (@owned @callee_owned (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject) -> ()
90+
sil @partial_apply_user_guaranteed : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject) -> ()
9091

9192
sil [ossa] [transparent] @test_partial_nativeobject_baz : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
9293
bb0(%0 : @owned $Builtin.NativeObject, %1 : @owned $Builtin.NativeObject):
@@ -101,6 +102,12 @@ bb0(%0 : @owned $@callee_owned (@owned Builtin.NativeObject) -> @owned Builtin.N
101102
return %7 : $Builtin.NativeObject
102103
}
103104

105+
sil [ossa] [transparent] @test_partial_nativeobject_bar_guaranteed : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
106+
bb0(%0 : @guaranteed $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, %1 : @owned $Builtin.NativeObject):
107+
%7 = apply %0(%1) : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
108+
return %7 : $Builtin.NativeObject
109+
}
110+
104111
// CHECK-LABEL: sil [transparent] [ossa] @test_partial_nativeobject_foo : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
105112
// CHECK: bb0([[ARG:%.*]] : @owned $Builtin.NativeObject):
106113
// CHECK: [[FN:%.*]] = function_ref @test_partial_nativeobject_baz :
@@ -145,6 +152,99 @@ bb3:
145152
return %13 : $Builtin.NativeObject
146153
}
147154

155+
// Make sure we do not eliminate the actual closure but do the inlining.
156+
//
157+
// CHECK-LABEL: sil [transparent] [ossa] @test_partial_nativeobject_foo_guaranteed_explicit_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
158+
// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed]
159+
// CHECK: [[NATIVEOBJECT_PLUS_FUNC:%.*]] = function_ref @nativeobject_plus : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
160+
// CHECK-NEXT: apply [[NATIVEOBJECT_PLUS_FUNC]](
161+
// CHECK: } // end sil function 'test_partial_nativeobject_foo_guaranteed_explicit_borrow'
162+
sil [transparent] [ossa] @test_partial_nativeobject_foo_guaranteed_explicit_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
163+
bb0(%0 : @owned $Builtin.NativeObject):
164+
%2 = function_ref @test_partial_nativeobject_bar_guaranteed : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
165+
%3 = function_ref @test_partial_nativeobject_baz : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
166+
br bb1
167+
168+
bb1:
169+
%0copy1 = copy_value %0 : $Builtin.NativeObject
170+
%5 = partial_apply [callee_guaranteed] %3(%0copy1) : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
171+
%6 = begin_borrow %5 : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
172+
br bb2
173+
174+
bb2:
175+
%0copy2 = copy_value %0 : $Builtin.NativeObject
176+
%13 = apply %2(%6, %0copy2) : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
177+
end_borrow %6 : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
178+
br bb3
179+
180+
bb3:
181+
%15 = function_ref @partial_apply_user_guaranteed : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject) -> ()
182+
apply %15(%5) : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject) -> ()
183+
destroy_value %5 : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
184+
destroy_value %0 : $Builtin.NativeObject
185+
return %13 : $Builtin.NativeObject
186+
}
187+
188+
// Make sure we do the inlining and delete the closure.
189+
// CHECK-LABEL: sil [transparent] [ossa] @test_partial_nativeobject_foo_guaranteed_explicit_borrow_dead_pa : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
190+
// CHECK-NOT: partial_apply
191+
// CHECK: [[NATIVEOBJECT_PLUS_FUNC:%.*]] = function_ref @nativeobject_plus : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
192+
// CHECK-NEXT: apply [[NATIVEOBJECT_PLUS_FUNC]](
193+
// CHECK-NOT: partial_apply
194+
// CHECK: } // end sil function 'test_partial_nativeobject_foo_guaranteed_explicit_borrow_dead_pa'
195+
sil [transparent] [ossa] @test_partial_nativeobject_foo_guaranteed_explicit_borrow_dead_pa : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
196+
bb0(%0 : @owned $Builtin.NativeObject):
197+
%2 = function_ref @test_partial_nativeobject_bar_guaranteed : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
198+
%3 = function_ref @test_partial_nativeobject_baz : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
199+
br bb1
200+
201+
bb1:
202+
%0copy1 = copy_value %0 : $Builtin.NativeObject
203+
%5 = partial_apply [callee_guaranteed] %3(%0copy1) : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
204+
%6 = begin_borrow %5 : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
205+
br bb2
206+
207+
bb2:
208+
%0copy2 = copy_value %0 : $Builtin.NativeObject
209+
%13 = apply %2(%6, %0copy2) : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
210+
end_borrow %6 : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
211+
br bb3
212+
213+
bb3:
214+
destroy_value %5 : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
215+
destroy_value %0 : $Builtin.NativeObject
216+
return %13 : $Builtin.NativeObject
217+
}
218+
219+
// CHECK-LABEL: sil [transparent] [ossa] @test_partial_nativeobject_foo_guaranteed_implicit_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
220+
// We shouldn't delete the PAI here.
221+
// CHECK: [[PAI:%.*]] = partial_apply [callee_guaranteed]
222+
// CHECK: [[NATIVEOBJECT_PLUS_FUNC:%.*]] = function_ref @nativeobject_plus : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
223+
// CHECK: apply [[NATIVEOBJECT_PLUS_FUNC]](
224+
// CHECK: } // end sil function 'test_partial_nativeobject_foo_guaranteed_implicit_borrow'
225+
sil [transparent] [ossa] @test_partial_nativeobject_foo_guaranteed_implicit_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
226+
bb0(%0 : @owned $Builtin.NativeObject):
227+
%2 = function_ref @test_partial_nativeobject_bar_guaranteed : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
228+
%3 = function_ref @test_partial_nativeobject_baz : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
229+
br bb1
230+
231+
bb1:
232+
%0copy1 = copy_value %0 : $Builtin.NativeObject
233+
%5 = partial_apply [callee_guaranteed] %3(%0copy1) : $@convention(thin) (@owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
234+
br bb2
235+
236+
bb2:
237+
%0copy2 = copy_value %0 : $Builtin.NativeObject
238+
%13 = apply %2(%5, %0copy2) : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject, @owned Builtin.NativeObject) -> @owned Builtin.NativeObject
239+
br bb3
240+
241+
bb3:
242+
%15 = function_ref @partial_apply_user_guaranteed : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject) -> ()
243+
apply %15(%5) : $@convention(thin) (@guaranteed @callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject) -> ()
244+
destroy_value %5 : $@callee_guaranteed (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
245+
destroy_value %0 : $Builtin.NativeObject
246+
return %13 : $Builtin.NativeObject
247+
}
148248

149249
sil [transparent] [ossa] @term_ossa_checked_cast_addr_br_takealways_callee : $@convention(thin) (@owned Builtin.NativeObject) -> () {
150250
bb0(%0 : @owned $Builtin.NativeObject):

0 commit comments

Comments
 (0)