Skip to content

[capture-promotion] Be sure to copy_value a struct_extracted value from a promoted capture even though the value is borrowed. #10278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 15, 2017
Merged

[capture-promotion] Be sure to copy_value a struct_extracted value from a promoted capture even though the value is borrowed. #10278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions lib/SILOptimizer/IPO/CapturePromotion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,13 +676,21 @@ void ClosureCloner::visitLoadInst(LoadInst *LI) {
if (SILValue Val = getProjectBoxMappedVal(SEAI->getOperand())) {
// Loads of a struct_element_addr of an argument get replaced with a
// struct_extract of the new passed in value. The value should be borrowed
// already.
// already, so we can just extract the value.
SILBuilderWithPostProcess<ClosureCloner, 1> B(this, LI);
assert(B.getFunction().hasUnqualifiedOwnership() ||
Val.getOwnershipKind().isTrivialOr(ValueOwnershipKind::Guaranteed));
SILValue V =
Val =
B.emitStructExtract(LI->getLoc(), Val, SEAI->getField(), LI->getType());
ValueMap.insert(std::make_pair(LI, V));

// If we were performing a load [copy], then we need to a perform a copy
// here since when cloning, we do not eliminate the destroy on the copied
// value.
if (LI->getFunction()->hasQualifiedOwnership()
&& LI->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
Val = getBuilder().createCopyValue(LI->getLoc(), Val);
}
ValueMap.insert(std::make_pair(LI, Val));
return;
}
SILCloner<ClosureCloner>::visitLoadInst(LI);
Expand All @@ -708,7 +716,7 @@ static bool isNonMutatingLoad(SILInstruction *I) {
static bool
isNonMutatingCapture(SILArgument *BoxArg) {
SmallVector<ProjectBoxInst*, 2> Projections;

// Conservatively do not allow any use of the box argument other than a
// strong_release or projection, since this is the pattern expected from
// SILGen.
Expand All @@ -731,7 +739,7 @@ isNonMutatingCapture(SILArgument *BoxArg) {
// TODO: This seems overly limited. Why not projections of tuples and other
// stuff? Also, why not recursive struct elements? This should be a helper
// function that mirrors isNonEscapingUse.
auto checkAddrUse = [](SILInstruction *AddrInst) {
auto isAddrUseMutating = [](SILInstruction *AddrInst) {
if (auto *SEAI = dyn_cast<StructElementAddrInst>(AddrInst)) {
return all_of(SEAI->getUses(),
[](Operand *Op) -> bool {
Expand All @@ -743,17 +751,18 @@ isNonMutatingCapture(SILArgument *BoxArg) {
|| isa<MarkFunctionEscapeInst>(AddrInst)
|| isa<EndAccessInst>(AddrInst);
};

for (auto *Projection : Projections) {
for (auto *UseOper : Projection->getUses()) {
if (auto *Access = dyn_cast<BeginAccessInst>(UseOper->getUser())) {
for (auto *AccessUseOper : Access->getUses()) {
if (!checkAddrUse(AccessUseOper->getUser()))
if (!isAddrUseMutating(AccessUseOper->getUser()))
return false;
}
continue;
}

if (!checkAddrUse(UseOper->getUser()))
if (!isAddrUseMutating(UseOper->getUser()))
return false;
}
}
Expand Down
91 changes: 90 additions & 1 deletion test/SILOptimizer/capture_promotion_ownership.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ sil @convert_from_integer_literal : $@convention(thin) (Builtin.Word, @thin Int.
sil @foo_allocating_init : $@convention(thin) (@thick Foo.Type) -> @owned Foo
sil @baz_init : $@convention(thin) (@thin Baz.Type) -> @owned Baz
sil @dummy_func : $@convention(thin) (Int, Int, Int) -> Int
sil @destructured_baz_user : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()

// CHECK-LABEL: sil @test_capture_promotion
sil @test_capture_promotion : $@convention(thin) () -> @owned @callee_owned () -> (Int, Builtin.Int64) {
Expand Down Expand Up @@ -381,4 +382,92 @@ bb0(%0 : @owned $<τ_0_0> { var τ_0_0 } <Baz>, %1 : @owned $<τ_0_0> { var τ_0
destroy_value %0 : $<τ_0_0> { var τ_0_0 } <Baz>
%9999 = tuple()
return %9999 : $()
}
}

// CHECK-LABEL: sil @test_capture_projection_test : $@convention(thin) () -> @owned @callee_owned () -> () {
sil @test_capture_projection_test : $@convention(thin) () -> @owned @callee_owned () -> () {
bb0:
%0 = function_ref @baz_init : $@convention(thin) (@thin Baz.Type) -> @owned Baz
%1 = metatype $@thin Baz.Type

// CHECK: [[BOX1:%.*]] = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
%2 = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
%3 = project_box %2 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
%4 = apply %0(%1) : $@convention(thin) (@thin Baz.Type) -> @owned Baz
store %4 to [init] %3 : $*Baz

// CHECK: [[BOX2:%.*]] = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
%5 = alloc_box $<τ_0_0> { var τ_0_0 } <Baz>
%6 = project_box %5 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
%7 = apply %0(%1) : $@convention(thin) (@thin Baz.Type) -> @owned Baz
store %7 to [init] %6 : $*Baz

// CHECK: [[BOX1_COPY:%.*]] = copy_value [[BOX1]]
// CHECK: [[BOX2_COPY:%.*]] = copy_value [[BOX2]]
// CHECK: [[BOX2_COPY_PB:%.*]] = project_box [[BOX2_COPY]]
// CHECK-NOT: function_ref @closure_indirect_result :
// CHECK: [[SPECIALIZED_FUNC:%.*]] = function_ref @_T023closure_projection_testTf2ni_n : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned Baz) -> ()
// CHECK-NOT: function_ref @closure_projection_test :
// CHECK: [[LOADBAZ:%.*]] = load [copy] [[BOX2_COPY_PB]] : $*Baz
// CHECK: destroy_value [[BOX2_COPY]]
%19 = copy_value %2 : $<τ_0_0> { var τ_0_0 } <Baz>
%20 = copy_value %5 : $<τ_0_0> { var τ_0_0 } <Baz>
%17 = function_ref @closure_projection_test : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned <τ_0_0> { var τ_0_0 } <Baz>) -> ()

// The partial apply has one value argument for each pair of arguments that was
// previously used to capture and pass the variable by reference
// CHECK: {{.*}} = partial_apply [[SPECIALIZED_FUNC]]([[BOX1_COPY]], [[LOADBAZ]])
%21 = partial_apply %17(%19, %20) : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned <τ_0_0> { var τ_0_0 } <Baz>) -> ()

destroy_value %2 : $<τ_0_0> { var τ_0_0 } <Baz>
destroy_value %5 : $<τ_0_0> { var τ_0_0 } <Baz>

return %21 : $@callee_owned () -> ()
}

// CHECK-LABEL: sil @_T023closure_projection_testTf2ni_n : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned Baz) -> () {
// CHECK: bb0([[UNPROMOTED_BAZ:%.*]] : @owned $<τ_0_0> { var τ_0_0 } <Baz>, [[BAZ:%.*]] : @owned $Baz):
// CHECK: [[BORROWED_BAZ:%.*]] = begin_borrow [[BAZ]] : $Baz
// CHECK: [[X:%.*]] = struct_extract [[BORROWED_BAZ]] : $Baz, #Baz.x
// CHECK: [[BAR:%.*]] = struct_extract [[BORROWED_BAZ]] : $Baz, #Baz.bar
// CHECK: [[BAR_COPY:%.*]] = copy_value [[BAR]]
// CHECK: [[BAR2:%.*]] = struct_extract [[BORROWED_BAZ]] : $Baz, #Baz.bar
// CHECK: [[BAR2_COPY:%.*]] = copy_value [[BAR2]]
// CHECK: [[BAR2_COPY_BORROW:%.*]] = begin_borrow [[BAR2_COPY]]
// CHECK: apply {{%.*}}([[BAR_COPY]], [[BAR2_COPY_BORROW]], [[X]])
// CHECK: end_borrow [[BAR2_COPY_BORROW]] from [[BAR2_COPY]]
// CHECK: destroy_value [[BAR2_COPY]]
// CHECK: end_borrow [[BORROWED_BAZ]] from [[BAZ]]
// CHECK: destroy_value [[BAZ]]
// CHECK: } // end sil function '_T023closure_projection_testTf2ni_n'
sil @closure_projection_test : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <Baz>, @owned <τ_0_0> { var τ_0_0 } <Baz>) -> () {
bb0(%0 : @owned $<τ_0_0> { var τ_0_0 } <Baz>, %1 : @owned $<τ_0_0> { var τ_0_0 } <Baz>):
%0a = project_box %0 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
%2 = struct_element_addr %0a : $*Baz, #Baz.x
%3 = struct_element_addr %0a : $*Baz, #Baz.bar
%4 = load [trivial] %2 : $*Int
%5 = load [take] %3 : $*Bar
%6 = load [copy] %3 : $*Bar
%7 = begin_borrow %6 : $Bar
%8 = function_ref @destructured_baz_user : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
apply %8(%5, %7, %4) : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
end_borrow %7 from %6 : $Bar, $Bar
destroy_value %6 : $Bar

%1a = project_box %1 : $<τ_0_0> { var τ_0_0 } <Baz>, 0
%9 = struct_element_addr %1a : $*Baz, #Baz.x
%10 = struct_element_addr %1a : $*Baz, #Baz.bar
%11 = load [trivial] %9 : $*Int
%12 = load [copy] %10 : $*Bar
%13 = load [copy] %10 : $*Bar
%14 = begin_borrow %13 : $Bar
%15 = function_ref @destructured_baz_user : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
apply %15(%12, %14, %11) : $@convention(thin) (@owned Bar, @guaranteed Bar, Int) -> ()
end_borrow %14 from %13 : $Bar, $Bar
destroy_value %13 : $Bar

destroy_value %1 : $<τ_0_0> { var τ_0_0 } <Baz>
destroy_value %0 : $<τ_0_0> { var τ_0_0 } <Baz>
%t = tuple()
return %t : $()
}