Skip to content

[DCE] Keep outer borrows alive when reborrowing. #39939

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 5 commits into from
Oct 28, 2021
Merged

[DCE] Keep outer borrows alive when reborrowing. #39939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d85ad3
[SILOpt] Add utility to find guaranteed roots.
nate-chandler Oct 26, 2021
3742331
[Test] Removed spurious attribute.
nate-chandler Oct 26, 2021
6b89397
[Test] Improved pattern name.
nate-chandler Oct 27, 2021
30a159b
[NFC] Used already cast value.
nate-chandler Oct 26, 2021
0a18c62
[DCE] Keep outer borrows alive when reborrowing.
nate-chandler Oct 26, 2021
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
5 changes: 5 additions & 0 deletions include/swift/SIL/MemAccessUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,11 @@ SILValue findReferenceRoot(SILValue ref);
/// Find the first owned root of the reference.
SILValue findOwnershipReferenceRoot(SILValue ref);

/// Look through all ownership forwarding instructions to find the values which
/// were originally borrowed.
void findGuaranteedReferenceRoots(SILValue value,
Copy link
Contributor

Choose a reason for hiding this comment

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

This already exists, why are you inventing a new form of this? I don't know why we have the one on line 205 as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They do different things.

Copy link
Contributor

Choose a reason for hiding this comment

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

@nate-chandler you want

bool getAllBorrowIntroducingValues(SILValue value,
SmallVectorImpl &out);

that's in OwnershipUtils.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know of that function. It looks like the behavior is slightly different, but I'll see if it does what is needed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nate and I spoke about this. I have some stuff reliant on this. He is going to in a subsequent commit look into eliminating this and using the already written API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify, I am going to determine whether these should be combined or not.

SmallVectorImpl<SILValue> &roots);

/// Find the aggregate containing the first owned root of the
/// reference. Identical to findOwnershipReferenceRoot, but looks through
/// struct_extract, tuple_extract, etc.
Expand Down
62 changes: 59 additions & 3 deletions lib/SIL/Utils/MemAccessUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
#define DEBUG_TYPE "sil-access-utils"

#include "swift/SIL/MemAccessUtils.h"
#include "swift/SIL/SILModule.h"
#include "swift/SIL/SILUndef.h"
#include "swift/SIL/DynamicCasts.h"
#include "swift/Basic/GraphNodeWorklist.h"
#include "swift/SIL/Consumption.h"
#include "swift/SIL/DynamicCasts.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILModule.h"
#include "swift/SIL/SILUndef.h"
#include "llvm/Support/Debug.h"

using namespace swift;
Expand Down Expand Up @@ -800,6 +801,61 @@ SILValue swift::findOwnershipReferenceRoot(SILValue ref) {
return ref;
}

void swift::findGuaranteedReferenceRoots(SILValue value,
SmallVectorImpl<SILValue> &roots) {
GraphNodeWorklist<SILValue, 4> worklist;
auto addAllOperandsToWorklist = [&worklist](SILInstruction *inst) -> bool {
if (inst->getNumOperands() > 0) {
for (auto operand : inst->getOperandValues()) {
worklist.insert(operand);
}
return true;
}
return false;
};
worklist.initialize(value);
while (auto value = worklist.pop()) {
if (auto *arg = dyn_cast<SILPhiArgument>(value)) {
if (auto *terminator = arg->getSingleTerminator()) {
worklist.insert(terminator->getOperand(arg->getIndex()));
continue;
}
} else if (auto *inst = value->getDefiningInstruction()) {
if (auto *result =
dyn_cast<FirstArgOwnershipForwardingSingleValueInst>(inst)) {
if (result->getNumOperands() > 0) {
worklist.insert(result->getOperand(0));
continue;
}
} else if (auto *result =
dyn_cast<AllArgOwnershipForwardingSingleValueInst>(inst)) {
if (addAllOperandsToWorklist(result)) {
continue;
}
} else if (auto *result = dyn_cast<OwnershipForwardingTermInst>(inst)) {
assert(false && "value defined by a terminator?!");
} else if (auto *result =
dyn_cast<OwnershipForwardingConversionInst>(inst)) {
worklist.insert(result->getConverted());
continue;
} else if (auto *result =
dyn_cast<OwnershipForwardingSelectEnumInstBase>(inst)) {
if (addAllOperandsToWorklist(result)) {
continue;
}
} else if (auto *result =
dyn_cast<OwnershipForwardingMultipleValueInstruction>(
inst)) {
if (addAllOperandsToWorklist(result)) {
continue;
}
}
}
if (value.getOwnershipKind() == OwnershipKind::Guaranteed)
roots.push_back(value);
}
}

/// Find the first owned aggregate containing the reference, or simply the
/// reference root if no aggregate is found.
///
Expand Down
21 changes: 13 additions & 8 deletions lib/SILOptimizer/Transforms/DeadCodeElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,18 +284,23 @@ void DCE::markLive() {
// Nested borrow handling can be complex in the presence of reborrows.
// So it is not handled currently.
auto *borrowInst = cast<BeginBorrowInst>(&I);
if (borrowInst->getOperand().getOwnershipKind() !=
OwnershipKind::Owned) {
if (borrowInst->getOperand().getOwnershipKind() ==
OwnershipKind::Guaranteed) {
markInstructionLive(borrowInst);
// Visit all end_borrows and mark them live
visitTransitiveEndBorrows(BorrowedValue(borrowInst),
[&](EndBorrowInst *endBorrow) {
markInstructionLive(endBorrow);
});
// Visit the end_borrows of all the borrow scopes that this
// begin_borrow could be borrowing.
SmallVector<SILValue, 4> roots;
findGuaranteedReferenceRoots(borrowInst->getOperand(), roots);
for (auto root : roots) {
visitTransitiveEndBorrows(BorrowedValue(root),
[&](EndBorrowInst *endBorrow) {
markInstructionLive(endBorrow);
});
}
continue;
}
// If not populate reborrowDependencies for this borrow
findReborrowDependencies(cast<BeginBorrowInst>(&I));
findReborrowDependencies(borrowInst);
break;
}
default:
Expand Down
226 changes: 222 additions & 4 deletions test/SILOptimizer/dead_code_elimination_nontrivial_ossa.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ class Klass {

}

enum EitherNoneOrAnyObject {
case none
case any(AnyObject)
}

struct NonTrivialStruct {
var val:Klass
}
Expand Down Expand Up @@ -658,7 +663,7 @@ bb4(%3 : @owned $Klass):
sil [ossa] @looping_borrow : $@convention(thin) () -> () {
entry:
%instance_1 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
%lifetime_1 = begin_borrow [lexical] %instance_1 : $FakeOptional<Klass>
%lifetime_1 = begin_borrow %instance_1 : $FakeOptional<Klass>
br loop_entry(%instance_1 : $FakeOptional<Klass>, %lifetime_1 : $FakeOptional<Klass>)

loop_entry(%18 : @owned $FakeOptional<Klass>, %19 : @guaranteed $FakeOptional<Klass>):
Expand All @@ -684,11 +689,11 @@ exit:
// CHECK: bb0({{%[^,]+}} : $Builtin.Int1, [[INSTANCE_1:%[^,]+]] : @owned $Klass, {{%[^,]+}} : @owned $Klass):
// CHECK: [[LIFETIME_1:%[^,]+]] = begin_borrow [[INSTANCE_1]]
// CHECK: copy_value [[LIFETIME_1]]
// CHECK: cond_br {{%[^,]+}}, [[BASIC_BLOCK1:bb[0-9]+]], [[BASIC_BLOCK2:bb[0-9]+]]
// CHECK: [[BASIC_BLOCK1]]:
// CHECK: cond_br {{%[^,]+}}, [[LEFT:bb[0-9]+]], [[RIGHT:bb[0-9]+]]
// CHECK: [[LEFT]]:
// CHECK: end_borrow [[LIFETIME_1]]
// CHECK: destroy_value [[INSTANCE_1]]
// CHECK: [[BASIC_BLOCK2]]:
// CHECK: [[RIGHT]]:
// CHECK: end_borrow [[LIFETIME_1]]
// CHECK: destroy_value [[INSTANCE_1]]
// CHECK-LABEL: } // end sil function 'add_end_borrow_after_destroy_value'
Expand Down Expand Up @@ -720,3 +725,216 @@ bb3(%original : @owned $Klass, %lifetime : @guaranteed $Klass):
%result = tuple ()
return %result : $()
}

// CHECK-LABEL: sil [ossa] @borrow_none : {{.*}} {
// CHECK: {{bb[0-9]+}}:
// CHECK: [[INSTANCE:%[^,]+]] = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
// CHECK: switch_enum [[INSTANCE]] : $FakeOptional<Klass>, case #FakeOptional.some!enumelt: [[WORK:bb[0-9]+]], case #FakeOptional.none!enumelt: [[TO_EXIT:bb[0-9]+]], forwarding: @guaranteed
// CHECK: [[TO_EXIT]]:
// CHECK: br [[EXIT:bb[0-9]+]]
// CHECK: [[WORK]]([[PAYLOAD:%[^,]+]] : @guaranteed $Klass):
// CHECK: [[LIFETIME:%[^,]+]] = begin_borrow [[PAYLOAD]]
// CHECK: end_borrow [[LIFETIME]]
// CHECK: br [[EXIT]]
// CHECK: [[EXIT]]:
// CHECK: [[RETVAL:%[^,]+]] = tuple ()
// CHECK: return [[RETVAL]] : $()
// CHECK-LABEL: } // end sil function 'borrow_none'
sil [ossa] @borrow_none : $@convention(thin) () -> () {
entry:
%outer_lifetime_1 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
switch_enum %outer_lifetime_1 : $FakeOptional<Klass>, case #FakeOptional.some!enumelt: work, case #FakeOptional.none!enumelt: to_exit, forwarding: @guaranteed
to_exit:
br exit
work(%outer_lifetime_2 : @guaranteed $Klass):
%inner_lifetime_1 = begin_borrow %outer_lifetime_2 : $Klass
end_borrow %inner_lifetime_1 : $Klass
br exit
exit:
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @reborrowed_begin_borrow : {{.*}} {
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $Klass):
// CHECK: [[OUTER_LIFETIME_1:%[^,]+]] = begin_borrow [[INSTANCE]]
// CHECK: br [[EXIT:bb[0-9]+]]([[OUTER_LIFETIME_1]] : $Klass)
// CHECK: [[EXIT]]([[OUTER_LIFETIME_2:%[^,]+]] : @guaranteed $Klass):
// CHECK: end_borrow [[OUTER_LIFETIME_2]]
// CHECK: destroy_value [[INSTANCE]]
// CHECK: [[RETVAL:%[^,]+]] = tuple ()
// CHECK: return [[RETVAL]]
// CHECK-LABEL: } // end sil function 'reborrowed_begin_borrow'
sil [ossa] @reborrowed_begin_borrow : $@convention(thin) (@owned Klass) -> () {
entry(%instance : @owned $Klass):
%outer_lifetime_1 = begin_borrow %instance : $Klass
%inner_lifetime_1 = begin_borrow %outer_lifetime_1 : $Klass
br exit(%outer_lifetime_1 : $Klass, %inner_lifetime_1 : $Klass)
exit(%outer_lifetime_2 : @guaranteed $Klass, %inner_lifetime_2 : @guaranteed $Klass):
end_borrow %inner_lifetime_2 : $Klass
end_borrow %outer_lifetime_2 : $Klass
destroy_value %instance : $Klass
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @reborrowed_guaranteed_phi : {{.*}} {
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $Klass):
// CHECK: [[LIFETIME:%[^,]+]] = begin_borrow [[INSTANCE]]
// CHECK: br [[WORK:bb[0-9]+]]([[LIFETIME]] : $Klass)
// CHECK: [[WORK]]([[LIFETIME_1:%[^,]+]] : @guaranteed $Klass):
// CHECK: br [[EXIT:bb[0-9]+]]([[LIFETIME_1]] : $Klass)
// CHECK: [[EXIT]]([[LIFETIME_2:%[^,]+]] : @guaranteed $Klass):
// CHECK: end_borrow [[LIFETIME_2]]
// CHECK: destroy_value [[INSTANCE]]
// CHECK: [[RETVAL:%[^,]+]] = tuple ()
// CHECK: return [[RETVAL]] : $()
// CHECK-LABEL: } // end sil function 'reborrowed_guaranteed_phi'
sil [ossa] @reborrowed_guaranteed_phi : $@convention(thin) (@owned Klass) -> () {
entry(%instance : @owned $Klass):
%outer_lifetime_1 = begin_borrow %instance : $Klass
br work(%outer_lifetime_1 : $Klass)
work(%outer_lifetime_2 : @guaranteed $Klass):
%inner_lifetime_1 = begin_borrow %outer_lifetime_2 : $Klass
br exit(%outer_lifetime_2 : $Klass, %inner_lifetime_1 : $Klass)
exit(%outer_lifetime_3 : @guaranteed $Klass, %inner_lifetime_2 : @guaranteed $Klass):
end_borrow %inner_lifetime_2 : $Klass
end_borrow %outer_lifetime_3 : $Klass
destroy_value %instance : $Klass
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @reborrow_guaranteed_phi2 : {{.*}} {
// CHECK: {{bb[0-9]+}}([[EITHER:%[^,]+]] : @owned $EitherNoneOrAnyObject):
// CHECK: [[BORROW_EITHER:%[^,]+]] = begin_borrow [[EITHER]]
// CHECK: switch_enum [[BORROW_EITHER]] : $EitherNoneOrAnyObject, case #EitherNoneOrAnyObject.none!enumelt: [[NONE_BLOCK:bb[0-9]+]], case #EitherNoneOrAnyObject.any!enumelt: [[SOME_BLOCK:bb[0-9]+]]
// CHECK: [[SOME_BLOCK]]([[PAYLOAD:%[^,]+]] : @guaranteed $AnyObject):
// CHECK: [[LIFETIME:%[^,]+]] = begin_borrow [[PAYLOAD]]
// CHECK: end_borrow [[LIFETIME]]
// CHECK: end_borrow [[BORROW_EITHER]]
// CHECK: destroy_value [[EITHER]]
// CHECK: br [[EXIT:bb[0-9]+]]
// CHECK: [[NONE_BLOCK]]:
// CHECK: end_borrow [[BORROW_EITHER]]
// CHECK: destroy_value [[EITHER]]
// CHECK: br [[EXIT]]
// CHECK: [[EXIT]]:
// CHECK: [[RETVAL:%[^,]+]] = tuple ()
// CHECK: return [[RETVAL]]
// CHECK-LABEL: } // end sil function 'reborrow_guaranteed_phi2'
sil [ossa] @reborrow_guaranteed_phi2 : $@convention(thin) (@owned EitherNoneOrAnyObject) -> () {
entry(%0 : @owned $EitherNoneOrAnyObject):
%borrow_either = begin_borrow %0 : $EitherNoneOrAnyObject
switch_enum %borrow_either : $EitherNoneOrAnyObject, case #EitherNoneOrAnyObject.none!enumelt: none_block, case #EitherNoneOrAnyObject.any!enumelt: any_block
any_block(%borrow : @guaranteed $AnyObject):
%2 = begin_borrow %borrow : $AnyObject
end_borrow %2 : $AnyObject
end_borrow %borrow_either : $EitherNoneOrAnyObject
destroy_value %0 : $EitherNoneOrAnyObject
br exit
none_block:
end_borrow %borrow_either : $EitherNoneOrAnyObject
destroy_value %0 : $EitherNoneOrAnyObject
br exit
exit:
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @reborrow_load_borrow : {{.*}} {
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : $*Klass):
// CHECK: [[LIFETIME:%[^,]+]] = load_borrow [[INSTANCE]]
// CHECK: br [[BASIC_BLOCK1:bb[0-9]+]]([[LIFETIME]] : $Klass)
// CHECK: [[BASIC_BLOCK1]]([[LIFETIME_2:%[^,]+]] : @guaranteed $Klass):
// CHECK: end_borrow [[LIFETIME_2]]
// CHECK: [[RETVAL:%[^,]+]] = tuple ()
// CHECK: return [[RETVAL]]
// CHECK-LABEL: } // end sil function 'reborrow_load_borrow'
sil [ossa] @reborrow_load_borrow : $@convention(method) (@in Klass) -> () {
bb0(%0 : $*Klass):
%1 = load_borrow %0 : $*Klass
%2 = begin_borrow %1 : $Klass
br bb1(%1 : $Klass, %2 : $Klass)
bb1(%4 : @guaranteed $Klass, %5 : @guaranteed $Klass):
end_borrow %5 : $Klass
end_borrow %4 : $Klass
%8 = tuple ()
return %8 : $()
}

// CHECK-LABEL: sil [ossa] @reborrow_load_borrow2 : {{.*}} {
// CHECK: {{bb[0-9]+}}([[ADDR:%[^,]+]] : $*Klass):
// CHECK: [[LIFETIME:%[^,]+]] = load_borrow [[ADDR]] : $*Klass
// CHECK: br [[EXIT:bb[0-9]+]]([[LIFETIME]] : $Klass)
// CHECK: [[EXIT]]([[LIFETIME_2:%[^,]+]] : @guaranteed $Klass):
// CHECK: end_borrow [[LIFETIME_2]] : $Klass
// CHECK: [[EXIT:%[^,]+]] = tuple ()
// CHECK: return [[EXIT]] : $()
// CHECK-LABEL: } // end sil function 'reborrow_load_borrow2'
sil [ossa] @reborrow_load_borrow2 : $@convention(method) (@in Klass) -> () {
bb0(%0 : $*Klass):
%1 = load_borrow %0 : $*Klass
%2 = begin_borrow %1 : $Klass
br bb1(%2 : $Klass, %1 : $Klass)
bb1(%4 : @guaranteed $Klass, %5 : @guaranteed $Klass):
end_borrow %4 : $Klass
end_borrow %5 : $Klass
%8 = tuple ()
return %8 : $()
}


// CHECK-LABEL: sil [ossa] @borrow_guaranteed_tuple : {{.*}} {
// CHECK: {{bb[0-9]+}}([[INSTANCE_1:%[^,]+]] : @owned $Klass, [[INSTANCE_2:%[^,]+]] : @owned $Klass):
// CHECK: [[LIFETIME_1_1:%[^,]+]] = begin_borrow [[INSTANCE_1]]
// CHECK: [[LIFETIME_2_1:%[^,]+]] = begin_borrow [[INSTANCE_2]]
// CHECK: br [[EXIT:bb[0-9]+]]([[LIFETIME_1_1]] : $Klass, [[LIFETIME_2_1]] : $Klass)
// CHECK: [[EXIT]]([[LIFETIME_1_2:%[^,]+]] : @guaranteed $Klass, [[LIFETIME_2_2:%[^,]+]] : @guaranteed $Klass):
// CHECK: end_borrow [[LIFETIME_2_2]]
// CHECK: destroy_value [[INSTANCE_2]]
// CHECK: end_borrow [[LIFETIME_1_2]]
// CHECK: destroy_value [[INSTANCE_1]]
// CHECK: [[RETVAL:%[^,]+]] = tuple ()
// CHECK: return [[RETVAL]] : $()
// CHECK-LABEL: } // end sil function 'borrow_guaranteed_tuple'
sil [ossa] @borrow_guaranteed_tuple : $@convention(thin) (@owned Klass, @owned Klass) -> () {
entry(%instance_1 : @owned $Klass, %instance_2 : @owned $Klass):
%lifetime_1_1 = begin_borrow %instance_1 : $Klass
%lifetime_2_1 = begin_borrow %instance_2 : $Klass
%tuple = tuple $(Klass, Klass) (%lifetime_1_1, %lifetime_2_1)
%tuple_lifetime_1 = begin_borrow %tuple : $(Klass, Klass)
br exit(%lifetime_1_1 : $Klass, %lifetime_2_1 : $Klass, %tuple_lifetime_1 : $(Klass, Klass))
exit(%lifetime_1_2 : @guaranteed $Klass, %lifetime_2_2 : @guaranteed $Klass, %tuple_lifetime_2 : @guaranteed $(Klass, Klass)):
end_borrow %tuple_lifetime_2 : $(Klass, Klass)
end_borrow %lifetime_2_2 : $Klass
destroy_value %instance_2 : $Klass
end_borrow %lifetime_1_2 : $Klass
destroy_value %instance_1 : $Klass
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @borrow_guaranteed_struct : {{.*}} {
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $Klass):
// CHECK: [[LIFETIME:%[^,]+]] = begin_borrow [[INSTANCE]]
// CHECK: br bb1([[LIFETIME]] : $Klass)
// CHECK: bb1([[LIFETIME_2:%[^,]+]] : @guaranteed $Klass):
// CHECK: end_borrow [[LIFETIME_2]]
// CHECK: destroy_value [[INSTANCE]]
// CHECK: [[RETVAL:%[^,]+]] = tuple ()
// CHECK: return [[RETVAL]] : $()
// CHECK-LABEL: } // end sil function 'borrow_guaranteed_struct'
sil [ossa] @borrow_guaranteed_struct : $@convention(thin) (@owned Klass) -> () {
entry(%instance_1 : @owned $Klass):
%lifetime_1 = begin_borrow %instance_1 : $Klass
%struct = struct $NonTrivialStruct (%lifetime_1 : $Klass)
%struct_lifetime_1 = begin_borrow %struct : $NonTrivialStruct
br exit(%lifetime_1 : $Klass, %struct_lifetime_1 : $NonTrivialStruct)
exit(%lifetime_2 : @guaranteed $Klass, %struct_lifetime_2 : @guaranteed $NonTrivialStruct):
end_borrow %struct_lifetime_2 : $NonTrivialStruct
end_borrow %lifetime_2 : $Klass
destroy_value %instance_1 : $Klass
%retval = tuple ()
return %retval : $()
}