Skip to content

Commit 16e36f0

Browse files
swift-ciswift-ci
authored
Merge pull request #10278 from gottesmm/rdar32625475
2 parents cc11e8b + ac67f33 commit 16e36f0

File tree

2 files changed

+106
-8
lines changed

2 files changed

+106
-8
lines changed

‎lib/SILOptimizer/IPO/CapturePromotion.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -676,13 +676,21 @@ void ClosureCloner::visitLoadInst(LoadInst *LI) {
676676
if (SILValue Val = getProjectBoxMappedVal(SEAI->getOperand())) {
677677
// Loads of a struct_element_addr of an argument get replaced with a
678678
// struct_extract of the new passed in value. The value should be borrowed
679-
// already.
679+
// already, so we can just extract the value.
680680
SILBuilderWithPostProcess<ClosureCloner, 1> B(this, LI);
681681
assert(B.getFunction().hasUnqualifiedOwnership() ||
682682
Val.getOwnershipKind().isTrivialOr(ValueOwnershipKind::Guaranteed));
683-
SILValue V =
683+
Val =
684684
B.emitStructExtract(LI->getLoc(), Val, SEAI->getField(), LI->getType());
685-
ValueMap.insert(std::make_pair(LI, V));
685+
686+
// If we were performing a load [copy], then we need to a perform a copy
687+
// here since when cloning, we do not eliminate the destroy on the copied
688+
// value.
689+
if (LI->getFunction()->hasQualifiedOwnership()
690+
&& LI->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
691+
Val = getBuilder().createCopyValue(LI->getLoc(), Val);
692+
}
693+
ValueMap.insert(std::make_pair(LI, Val));
686694
return;
687695
}
688696
SILCloner<ClosureCloner>::visitLoadInst(LI);
@@ -708,7 +716,7 @@ static bool isNonMutatingLoad(SILInstruction *I) {
708716
static bool
709717
isNonMutatingCapture(SILArgument *BoxArg) {
710718
SmallVector<ProjectBoxInst*, 2> Projections;
711-
719+
712720
// Conservatively do not allow any use of the box argument other than a
713721
// strong_release or projection, since this is the pattern expected from
714722
// SILGen.
@@ -731,7 +739,7 @@ isNonMutatingCapture(SILArgument *BoxArg) {
731739
// TODO: This seems overly limited. Why not projections of tuples and other
732740
// stuff? Also, why not recursive struct elements? This should be a helper
733741
// function that mirrors isNonEscapingUse.
734-
auto checkAddrUse = [](SILInstruction *AddrInst) {
742+
auto isAddrUseMutating = [](SILInstruction *AddrInst) {
735743
if (auto *SEAI = dyn_cast<StructElementAddrInst>(AddrInst)) {
736744
return all_of(SEAI->getUses(),
737745
[](Operand *Op) -> bool {
@@ -743,17 +751,18 @@ isNonMutatingCapture(SILArgument *BoxArg) {
743751
|| isa<MarkFunctionEscapeInst>(AddrInst)
744752
|| isa<EndAccessInst>(AddrInst);
745753
};
754+
746755
for (auto *Projection : Projections) {
747756
for (auto *UseOper : Projection->getUses()) {
748757
if (auto *Access = dyn_cast<BeginAccessInst>(UseOper->getUser())) {
749758
for (auto *AccessUseOper : Access->getUses()) {
750-
if (!checkAddrUse(AccessUseOper->getUser()))
759+
if (!isAddrUseMutating(AccessUseOper->getUser()))
751760
return false;
752761
}
753762
continue;
754763
}
755764

756-
if (!checkAddrUse(UseOper->getUser()))
765+
if (!isAddrUseMutating(UseOper->getUser()))
757766
return false;
758767
}
759768
}

‎test/SILOptimizer/capture_promotion_ownership.sil

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ sil @convert_from_integer_literal : $@convention(thin) (Builtin.Word, @thin Int.
2929
sil @foo_allocating_init : $@convention(thin) (@thick Foo.Type) -> @owned Foo
3030
sil @baz_init : $@convention(thin) (@thin Baz.Type) -> @owned Baz
3131
sil @dummy_func : $@convention(thin) (Int, Int, Int) -> Int
32+
sil @destructured_baz_user : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
3233

3334
// CHECK-LABEL: sil @test_capture_promotion
3435
sil @test_capture_promotion : $@convention(thin) () -> @owned @callee_owned () -> (Int, Builtin.Int64) {
@@ -381,4 +382,92 @@ bb0(%0 : @owned $<τ_0_0> { var τ_0_0 } <Baz>, %1 : @owned $<τ_0_0> { var τ_0
381382
destroy_value %0 : $<τ_0_0> { var τ_0_0 } <Baz>
382383
%9999 = tuple()
383384
return %9999 : $()
384-
}
385+
}
386+
387+
// CHECK-LABEL: sil @test_capture_projection_test : $@convention(thin) () -> @owned @callee_owned () -> () {
388+
sil @test_capture_projection_test : $@convention(thin) () -> @owned @callee_owned () -> () {
389+
bb0:
390+
%0 = function_ref @baz_init : $@convention(thin) (@thin Baz.Type) -> @owned Baz
391+
%1 = metatype $@thin Baz.Type
392+
393+
// CHECK: [[BOX1:%.*]] = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
394+
%2 = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
395+
%3 = project_box %2 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
396+
%4 = apply %0(%1) : $@convention(thin) (@thin Baz.Type) -> @owned Baz
397+
store %4 to [init] %3 : $*Baz
398+
399+
// CHECK: [[BOX2:%.*]] = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
400+
%5 = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
401+
%6 = project_box %5 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
402+
%7 = apply %0(%1) : $@convention(thin) (@thin Baz.Type) -> @owned Baz
403+
store %7 to [init] %6 : $*Baz
404+
405+
// CHECK: [[BOX1_COPY:%.*]] = copy_value [[BOX1]]
406+
// CHECK: [[BOX2_COPY:%.*]] = copy_value [[BOX2]]
407+
// CHECK: [[BOX2_COPY_PB:%.*]] = project_box [[BOX2_COPY]]
408+
// CHECK-NOT: function_ref @closure_indirect_result :
409+
// CHECK: [[SPECIALIZED_FUNC:%.*]] = function_ref @_T023closure_projection_testTf2ni_n : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned Baz) -> ()
410+
// CHECK-NOT: function_ref @closure_projection_test :
411+
// CHECK: [[LOADBAZ:%.*]] = load [copy] [[BOX2_COPY_PB]] : $*Baz
412+
// CHECK: destroy_value [[BOX2_COPY]]
413+
%19 = copy_value %2 : $<τ_0_0> { var τ_0_0 } <Baz>
414+
%20 = copy_value %5 : $<τ_0_0> { var τ_0_0 } <Baz>
415+
%17 = function_ref @closure_projection_test : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned <τ_0_0> { var τ_0_0 } <Baz>) -> ()
416+
417+
// The partial apply has one value argument for each pair of arguments that was
418+
// previously used to capture and pass the variable by reference
419+
// CHECK: {{.*}} = partial_apply [[SPECIALIZED_FUNC]]([[BOX1_COPY]], [[LOADBAZ]])
420+
%21 = partial_apply %17(%19, %20) : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned <τ_0_0> { var τ_0_0 } <Baz>) -> ()
421+
422+
destroy_value %2 : $<τ_0_0> { var τ_0_0 } <Baz>
423+
destroy_value %5 : $<τ_0_0> { var τ_0_0 } <Baz>
424+
425+
return %21 : $@callee_owned () -> ()
426+
}
427+
428+
// CHECK-LABEL: sil @_T023closure_projection_testTf2ni_n : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned Baz) -> () {
429+
// CHECK: bb0([[UNPROMOTED_BAZ:%.*]] : @owned $<τ_0_0> { var τ_0_0 } <Baz>, [[BAZ:%.*]] : @owned $Baz):
430+
// CHECK: [[BORROWED_BAZ:%.*]] = begin_borrow [[BAZ]] : $Baz
431+
// CHECK: [[X:%.*]] = struct_extract [[BORROWED_BAZ]] : $Baz, #Baz.x
432+
// CHECK: [[BAR:%.*]] = struct_extract [[BORROWED_BAZ]] : $Baz, #Baz.bar
433+
// CHECK: [[BAR_COPY:%.*]] = copy_value [[BAR]]
434+
// CHECK: [[BAR2:%.*]] = struct_extract [[BORROWED_BAZ]] : $Baz, #Baz.bar
435+
// CHECK: [[BAR2_COPY:%.*]] = copy_value [[BAR2]]
436+
// CHECK: [[BAR2_COPY_BORROW:%.*]] = begin_borrow [[BAR2_COPY]]
437+
// CHECK: apply {{%.*}}([[BAR_COPY]], [[BAR2_COPY_BORROW]], [[X]])
438+
// CHECK: end_borrow [[BAR2_COPY_BORROW]] from [[BAR2_COPY]]
439+
// CHECK: destroy_value [[BAR2_COPY]]
440+
// CHECK: end_borrow [[BORROWED_BAZ]] from [[BAZ]]
441+
// CHECK: destroy_value [[BAZ]]
442+
// CHECK: } // end sil function '_T023closure_projection_testTf2ni_n'
443+
sil @closure_projection_test : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned <τ_0_0> { var τ_0_0 } <Baz>) -> () {
444+
bb0(%0 : @owned $<τ_0_0> { var τ_0_0 } <Baz>, %1 : @owned $<τ_0_0> { var τ_0_0 } <Baz>):
445+
%0a = project_box %0 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
446+
%2 = struct_element_addr %0a : $*Baz, #Baz.x
447+
%3 = struct_element_addr %0a : $*Baz, #Baz.bar
448+
%4 = load [trivial] %2 : $*Int
449+
%5 = load [take] %3 : $*Bar
450+
%6 = load [copy] %3 : $*Bar
451+
%7 = begin_borrow %6 : $Bar
452+
%8 = function_ref @destructured_baz_user : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
453+
apply %8(%5, %7, %4) : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
454+
end_borrow %7 from %6 : $Bar, $Bar
455+
destroy_value %6 : $Bar
456+
457+
%1a = project_box %1 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
458+
%9 = struct_element_addr %1a : $*Baz, #Baz.x
459+
%10 = struct_element_addr %1a : $*Baz, #Baz.bar
460+
%11 = load [trivial] %9 : $*Int
461+
%12 = load [copy] %10 : $*Bar
462+
%13 = load [copy] %10 : $*Bar
463+
%14 = begin_borrow %13 : $Bar
464+
%15 = function_ref @destructured_baz_user : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
465+
apply %15(%12, %14, %11) : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
466+
end_borrow %14 from %13 : $Bar, $Bar
467+
destroy_value %13 : $Bar
468+
469+
destroy_value %1 : $<τ_0_0> { var τ_0_0 } <Baz>
470+
destroy_value %0 : $<τ_0_0> { var τ_0_0 } <Baz>
471+
%t = tuple()
472+
return %t : $()
473+
}

0 commit comments

Comments
 (0)