Skip to content

[semantic-arc] Eliminate simple recursive copy without borrows involved. #34812

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
Nov 20, 2020
Merged

[semantic-arc] Eliminate simple recursive copy without borrows involved. #34812

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
64 changes: 64 additions & 0 deletions lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "OwnershipPhiOperand.h"
#include "SemanticARCOptVisitor.h"
#include "swift/SIL/LinearLifetimeChecker.h"

using namespace swift;
using namespace swift::semanticarc;
Expand Down Expand Up @@ -450,6 +451,65 @@ bool SemanticARCOptVisitor::tryJoiningCopyValueLiveRangeWithOperand(
return false;
}

/// Given an owned value that is completely enclosed within its parent owned
/// value and is not consumed, eliminate the copy.
bool SemanticARCOptVisitor::tryPerformOwnedCopyValueOptimization(
CopyValueInst *cvi) {
if (ctx.onlyGuaranteedOpts)
return false;

auto originalValue = cvi->getOperand();
if (originalValue.getOwnershipKind() != OwnershipKind::Owned)
return false;

// TODO: Add support for forwarding insts here.
SmallVector<DestroyValueInst *, 8> destroyingUses;
SmallVector<Operand *, 32> allCopyUses;
for (auto *use : cvi->getUses()) {
// First just stash our use so we have /all uses/.
allCopyUses.push_back(use);

// Then if we are not a lifetime ending use, just continue.
if (!use->isLifetimeEnding()) {
continue;
}

// Otherwise, if we have a destroy value lifetime ending use, stash that.
if (auto *dvi = dyn_cast<DestroyValueInst>(use->getUser())) {
destroyingUses.push_back(dvi);
continue;
}

// Otherwise, just bail for now.
return false;
}

// NOTE: We do not actually care if the parent's lifetime ends with
// destroy_values. All we care is that it is lifetime ending and the use isn't
// a forwarding instruction.
SmallVector<Operand *, 8> parentLifetimeEndingUses;
for (auto *origValueUse : originalValue->getUses())
if (origValueUse->isLifetimeEnding() &&
!isa<OwnershipForwardingInst>(origValueUse->getUser()))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... reading this... I think what I wrote here could be made better. I think at this point, the optimization is not dealing with LiveRanges, so the right thing to do is just gather up all consuming uses of the value, ignoring forwarding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*ignoring forwarding I mean just treating forwarding insts as normal consuming instructions.

parentLifetimeEndingUses.push_back(origValueUse);

// Ok, we have an owned value. If we do not have any non-destroying consuming
// uses, see if all of our uses (ignoring destroying uses) are within our
// parent owned value's lifetime.
SmallPtrSet<SILBasicBlock *, 8> visitedBlocks;
LinearLifetimeChecker checker(visitedBlocks, ctx.getDeadEndBlocks());
if (!checker.validateLifetime(originalValue, parentLifetimeEndingUses,
allCopyUses))
return false;

// Ok, we can perform our transform. Eliminate all of our destroy value insts,
// and then RAUW our copy value with our parent value.
while (!destroyingUses.empty())
eraseInstruction(destroyingUses.pop_back_val());
eraseAndRAUWSingleValueInstruction(cvi, cvi->getOperand());
return true;
}

//===----------------------------------------------------------------------===//
// Top Level Entrypoint
//===----------------------------------------------------------------------===//
Expand All @@ -472,5 +532,9 @@ bool SemanticARCOptVisitor::visitCopyValueInst(CopyValueInst *cvi) {
return true;
}

if (tryPerformOwnedCopyValueOptimization(cvi)) {
return true;
}

return false;
}
1 change: 1 addition & 0 deletions lib/SILOptimizer/SemanticARC/SemanticARCOptVisitor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ struct LLVM_LIBRARY_VISIBILITY SemanticARCOptVisitor
bool performGuaranteedCopyValueOptimization(CopyValueInst *cvi);
bool eliminateDeadLiveRangeCopyValue(CopyValueInst *cvi);
bool tryJoiningCopyValueLiveRangeWithOperand(CopyValueInst *cvi);
bool tryPerformOwnedCopyValueOptimization(CopyValueInst *cvi);
};

} // namespace semanticarc
Expand Down
209 changes: 209 additions & 0 deletions test/SILOptimizer/semantic-arc-opts.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,23 @@ sil @owned_user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
sil @get_owned_obj : $@convention(thin) () -> @owned Builtin.NativeObject
sil @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
sil @inout_user : $@convention(thin) (@inout FakeOptional<NativeObjectPair>) -> ()
sil @get_native_object : $@convention(thin) () -> @owned Builtin.NativeObject

struct NativeObjectPair {
var obj1 : Builtin.NativeObject
var obj2 : Builtin.NativeObject
}

sil @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair

struct FakeOptionalNativeObjectPairPair {
var pair1 : FakeOptional<NativeObjectPair>
var pair2 : FakeOptional<NativeObjectPair>
}
sil @inout_user2 : $@convention(thin) (@inout FakeOptionalNativeObjectPairPair) -> ()

sil @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
sil @consume_nativeobject_pair : $@convention(thin) (@owned NativeObjectPair) -> ()

protocol MyFakeAnyObject : Klass {
func myFakeMethod()
Expand Down Expand Up @@ -2868,3 +2872,208 @@ bb3(%result : @owned $FakeOptional<Builtin.NativeObject>):
dealloc_stack %allocStack : $*Builtin.NativeObject
return %result : $FakeOptional<Builtin.NativeObject>
}

// CHECK-LABEL: sil [ossa] @simple_recursive_copy_case : $@convention(thin) () -> () {
// CHECK-NOT: copy_value
// CHECK: } // end sil function 'simple_recursive_copy_case'
sil [ossa] @simple_recursive_copy_case : $@convention(thin) () -> () {
bb0:
%f = function_ref @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair
%pair = apply %f() : $@convention(thin) () -> @owned NativeObjectPair
%1 = copy_value %pair : $NativeObjectPair
%2 = begin_borrow %1 : $NativeObjectPair
%3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
%gUserFun = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %gUserFun(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %2 : $NativeObjectPair
destroy_value %1 : $NativeObjectPair
destroy_value %pair : $NativeObjectPair
%9999 = tuple()
return %9999 : $()
}

// CHECK-LABEL: sil [ossa] @simple_recursive_copy_case_2 : $@convention(thin) () -> () {
// CHECK-NOT: copy_value
// CHECK: } // end sil function 'simple_recursive_copy_case_2'
sil [ossa] @simple_recursive_copy_case_2 : $@convention(thin) () -> () {
bb0:
%f = function_ref @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair
%pair = apply %f() : $@convention(thin) () -> @owned NativeObjectPair
%1 = copy_value %pair : $NativeObjectPair
%2 = begin_borrow %1 : $NativeObjectPair
destroy_value %pair : $NativeObjectPair
%3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
%gUserFun = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %gUserFun(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %2 : $NativeObjectPair
destroy_value %1 : $NativeObjectPair
%9999 = tuple()
return %9999 : $()
}

// We fail in this case since the lifetime of %pair ends too early and our
// joined lifetime analysis is too simplistic to handle this case.
//
// CHECK-LABEL: sil [ossa] @simple_recursive_copy_case_3 : $@convention(thin) () -> () {
// CHECK: copy_value
// CHECK: } // end sil function 'simple_recursive_copy_case_3'
sil [ossa] @simple_recursive_copy_case_3 : $@convention(thin) () -> () {
bb0:
%f = function_ref @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair
%pair = apply %f() : $@convention(thin) () -> @owned NativeObjectPair
%1 = copy_value %pair : $NativeObjectPair
%2 = begin_borrow %1 : $NativeObjectPair
%3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
%gUserFun = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %gUserFun(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %2 : $NativeObjectPair
%consumeFunc = function_ref @consume_nativeobject_pair : $@convention(thin) (@owned NativeObjectPair) -> ()
apply %consumeFunc(%pair) : $@convention(thin) (@owned NativeObjectPair) -> ()
cond_br undef, bb1, bb2

bb1:
(%1a, %1b) = destructure_struct %1 : $NativeObjectPair
%ownedUser = function_ref @owned_user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
apply %ownedUser(%1a) : $@convention(thin) (@owned Builtin.NativeObject) -> ()
apply %ownedUser(%1b) : $@convention(thin) (@owned Builtin.NativeObject) -> ()
br bb3

bb2:
destroy_value %1 : $NativeObjectPair
br bb3

bb3:
%9999 = tuple()
return %9999 : $()
}

// This case fails due to the destructure of our parent object even though the
// lifetimes line up. We don't support destructures here yet.
//
// TODO: Handle this!
//
// CHECK-LABEL: sil [ossa] @simple_recursive_copy_case_4 : $@convention(thin) () -> () {
// CHECK: copy_value
// CHECK: } // end sil function 'simple_recursive_copy_case_4'
sil [ossa] @simple_recursive_copy_case_4 : $@convention(thin) () -> () {
bb0:
%f = function_ref @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair
%pair = apply %f() : $@convention(thin) () -> @owned NativeObjectPair
%1 = copy_value %pair : $NativeObjectPair
%2 = begin_borrow %1 : $NativeObjectPair
%3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
%gUserFun = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %gUserFun(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %2 : $NativeObjectPair
%consumeFunc = function_ref @consume_nativeobject_pair : $@convention(thin) (@owned NativeObjectPair) -> ()
destroy_value %1 : $NativeObjectPair
(%pair1, %pair2) = destructure_struct %pair : $NativeObjectPair
%ownedUser = function_ref @owned_user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
apply %ownedUser(%pair1) : $@convention(thin) (@owned Builtin.NativeObject) -> ()
apply %ownedUser(%pair2) : $@convention(thin) (@owned Builtin.NativeObject) -> ()
%9999 = tuple()
return %9999 : $()
}

// This case fails due to the destructure of our parent object even though the
// lifetimes line up. We don't support destructures here yet.
//
// TODO: Handle this!
//
// CHECK-LABEL: sil [ossa] @simple_recursive_copy_case_5 : $@convention(thin) () -> () {
// CHECK: copy_value
// CHECK: } // end sil function 'simple_recursive_copy_case_5'
sil [ossa] @simple_recursive_copy_case_5 : $@convention(thin) () -> () {
bb0:
%f = function_ref @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair
%pair = apply %f() : $@convention(thin) () -> @owned NativeObjectPair
%1 = copy_value %pair : $NativeObjectPair
%2 = begin_borrow %1 : $NativeObjectPair
%3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
%gUserFun = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %gUserFun(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %2 : $NativeObjectPair
%consumeFunc = function_ref @consume_nativeobject_pair : $@convention(thin) (@owned NativeObjectPair) -> ()
(%pair1, %pair2) = destructure_struct %pair : $NativeObjectPair
%ownedUser = function_ref @owned_user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
destroy_value %1 : $NativeObjectPair
apply %ownedUser(%pair1) : $@convention(thin) (@owned Builtin.NativeObject) -> ()
apply %ownedUser(%pair2) : $@convention(thin) (@owned Builtin.NativeObject) -> ()
%9999 = tuple()
return %9999 : $()
}

// Make sure we do not eliminate copies where only the destroy_value is outside
// of the lifetime of the parent value, but a begin_borrow extends the lifetime
// of the value.
//
// CHECK-LABEL: sil [ossa] @simple_recursive_copy_case_destroying_use_out_of_lifetime : $@convention(thin) () -> () {
// CHECK: copy_value
// CHECK: } // end sil function 'simple_recursive_copy_case_destroying_use_out_of_lifetime'
sil [ossa] @simple_recursive_copy_case_destroying_use_out_of_lifetime : $@convention(thin) () -> () {
bb0:
%f = function_ref @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair
%pair = apply %f() : $@convention(thin) () -> @owned NativeObjectPair
%pairBorrow = begin_borrow %pair : $NativeObjectPair
%3 = struct_extract %pairBorrow : $NativeObjectPair, #NativeObjectPair.obj1
%gUserFun = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %gUserFun(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %pairBorrow : $NativeObjectPair
cond_br undef, bb1, bb2

bb1:
%1 = copy_value %pair : $NativeObjectPair
%2 = begin_borrow %1 : $NativeObjectPair
destroy_value %pair : $NativeObjectPair
%3a = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
apply %gUserFun(%3a) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %2 : $NativeObjectPair
destroy_value %1 : $NativeObjectPair
br bb3

bb2:
destroy_value %pair : $NativeObjectPair
br bb3

bb3:
%9999 = tuple()
return %9999 : $()
}

// Second version of the test that consumes the pair in case we make the
// lifetime joining smart enough to handle the original case.
//
// CHECK-LABEL: sil [ossa] @simple_recursive_copy_case_destroying_use_out_of_lifetime_2 : $@convention(thin) () -> () {
// CHECK: copy_value
// CHECK: } // end sil function 'simple_recursive_copy_case_destroying_use_out_of_lifetime_2'
sil [ossa] @simple_recursive_copy_case_destroying_use_out_of_lifetime_2 : $@convention(thin) () -> () {
bb0:
%f = function_ref @get_object_pair : $@convention(thin) () -> @owned NativeObjectPair
%pair = apply %f() : $@convention(thin) () -> @owned NativeObjectPair
%pairBorrow = begin_borrow %pair : $NativeObjectPair
%3 = struct_extract %pairBorrow : $NativeObjectPair, #NativeObjectPair.obj1
%gUserFun = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
apply %gUserFun(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %pairBorrow : $NativeObjectPair
cond_br undef, bb1, bb2

bb1:
%1 = copy_value %pair : $NativeObjectPair
%2 = begin_borrow %1 : $NativeObjectPair
destroy_value %pair : $NativeObjectPair
%3a = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
apply %gUserFun(%3a) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
end_borrow %2 : $NativeObjectPair
destroy_value %1 : $NativeObjectPair
br bb3

bb2:
%consumePair = function_ref @consume_nativeobject_pair : $@convention(thin) (@owned NativeObjectPair) -> ()
apply %consumePair(%pair) : $@convention(thin) (@owned NativeObjectPair) -> ()
br bb3

bb3:
%9999 = tuple()
return %9999 : $()
}