averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-21 12:14:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

[semantic-arc-opts] When performing load [copy] -> load_borrow on classes, do not ignore forwarding uses.

Browse Source 
This is the first of two commits. This commit is a very simple, easily
cherry-pickable fix but does not use the LiveRange infrastructure so that we
handle forwarding uses here. Instead, we just bail if all uses of our load
[copy] are not destroy_value.

In a subsequent commit, I am going to change this to use the LiveRange
infrastructure so we will handle these cases. Sadly doing so doesn't cherry-pick
well. = /.

rdar://58289320
This commit is contained in:
Michael Gottesman
2020-01-03 10:10:16 -08:00
parent 475b5591ed
commit 23f36a059a
2 changed files with 95 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp
View File

@@ -36,6 +36,25 @@ STATISTIC(NumEliminatedInsts, "number of removed instructions");
STATISTIC(NumLoadCopyConvertedToLoadBorrow,
"number of load_copy converted to load_borrow");
//===----------------------------------------------------------------------===//
// Utility
//===----------------------------------------------------------------------===//
static bool isConsumingOwnedUse(Operand *use) {
assert(use->get().getOwnershipKind() == ValueOwnershipKind::Owned);
if (use->isTypeDependent())
return false;
// We know that a copy_value produces an @owned value. Look through all of
// our uses and classify them as either invalidating or not
// invalidating. Make sure that all of the invalidating ones are
// destroy_value since otherwise the live_range is not complete.
auto map = use->getOwnershipKindMap();
auto constraint = map.getLifetimeConstraint(ValueOwnershipKind::Owned);
return constraint == UseLifetimeConstraint::MustBeInvalidated;
}
//===----------------------------------------------------------------------===//
// Live Range Modeling
//===----------------------------------------------------------------------===//
@@ -775,8 +794,21 @@ public:
std::back_inserter(baseEndBorrows));
SmallVector<SILInstruction *, 4> valueDestroys;
llvm::copy(Load->getUsersOfType<DestroyValueInst>(),
std::back_inserter(valueDestroys));
for (auto *use : Load->getUses()) {
// If this load is not consuming, skip it.
if (!isConsumingOwnedUse(use)) {
continue;
}
// Otherwise, if this isn't a destroy_value, we have a consuming use we
// don't understand. Return conservatively that this memory location may
// not be guaranteed.
auto *user = use->getUser();
if (!isa<DestroyValueInst>(user)) {
return answer(true);
}
valueDestroys.emplace_back(user);
}
SmallPtrSet<SILBasicBlock *, 4> visitedBlocks;
LinearLifetimeChecker checker(visitedBlocks, ARCOpt.getDeadEndBlocks());

74
test/SILOptimizer/semantic-arc-opts.sil
View File

@@ -9,6 +9,10 @@ import Builtin
//////////////////
enum MyNever {}
enum FakeOptional<T> {
case none
case some(T)
}
sil @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
sil @owned_user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
@@ -24,6 +28,7 @@ sil @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
class Klass {}
sil @guaranteed_klass_user : $@convention(thin) (@guaranteed Klass) -> ()
sil @guaranteed_fakeoptional_klass_user : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> ()
struct MyInt {
var value: Builtin.Int32
@@ -40,11 +45,6 @@ struct StructMemberTest {
var t : (Builtin.Int32, AnotherStruct)
}
enum FakeOptional<T> {
case none
case some(T)
}
class ClassLet {
@_hasStorage let aLet: Klass
@_hasStorage var aVar: Klass
@@ -589,6 +589,52 @@ bb0(%x : @owned $ClassLet):
return undef : $()
}
// We do not support this today, but we will once forwarding is ignored when
// checking if the load [copy] is a dead live range.
//
// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase :
// CHECK: load [copy]
// CHECK: } // end sil function 'dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase'
sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase : $@convention(thin) (@owned ClassLet) -> () {
bb0(%x : @owned $ClassLet):
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
%a = begin_borrow %x : $ClassLet
%p = ref_element_addr %a : $ClassLet, #ClassLet.aLetTuple
%v = load [copy] %p : $*(Klass, Klass)
(%v1, %v2) = destructure_tuple %v : $(Klass, Klass)
apply %f(%v1) : $@convention(thin) (@guaranteed Klass) -> ()
apply %f(%v2) : $@convention(thin) (@guaranteed Klass) -> ()
destroy_value %v1 : $Klass
destroy_value %v2 : $Klass
end_borrow %a : $ClassLet
destroy_value %x : $ClassLet
return undef : $()
}
// We do not support this today, but we will once forwarding is ignored when
// checking if the load [copy] is a dead live range.
//
// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase_2 :
// CHECK: load [copy]
// CHECK: } // end sil function 'dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase_2'
sil [ossa] @dont_copy_let_properties_with_borrowed_base_that_dominates_projtestcase_2 : $@convention(thin) (@owned ClassLet) -> () {
bb0(%x : @owned $ClassLet):
%f = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
%a = begin_borrow %x : $ClassLet
%p = ref_element_addr %a : $ClassLet, #ClassLet.aLet
%v = load [copy] %p : $*Klass
%v_cast = unchecked_ref_cast %v : $Klass to $Builtin.NativeObject
apply %f(%v_cast) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
destroy_value %v_cast : $Builtin.NativeObject
end_borrow %a : $ClassLet
destroy_value %x : $ClassLet
return undef : $()
}
// CHECK-LABEL: sil [ossa] @dont_copy_let_properties_with_multi_borrowed_base_that_dominates
// CHECK: [[OUTER:%.*]] = begin_borrow
// CHECK-NEXT: ref_element_addr
@@ -662,7 +708,7 @@ bb0(%x : @owned $ClassLet):
return undef : $()
}
// CHECK-LABEL: sil [ossa] @do_or_dont_copy_let_properties_with_multi_borrowed_base_when_it_dominates
// CHECK-LABEL: sil [ossa] @do_or_dont_copy_let_properties_with_multi_borrowed_base_when_it_dominates_2 :
// CHECK: [[OUTER:%.*]] = begin_borrow
// CHECK-NEXT: ref_element_addr
// CHECK-NEXT: [[INNER:%.*]] = load_borrow
@@ -672,12 +718,15 @@ bb0(%x : @owned $ClassLet):
// CHECK-NEXT: begin_borrow
// CHECK-NEXT: ref_element_addr
// CHECK-NEXT: load [copy]
// CHECK-NEXT: apply
// CHECK-NEXT: end_borrow
// CHECK-NEXT: destroy_value
// CHECK-NEXT: // function_ref
// CHECK-NEXT: function_ref
// CHECK-NEXT: enum
// CHECK-NEXT: apply
// CHECK-NEXT: destroy_value
sil [ossa] @do_or_dont_copy_let_properties_with_multi_borrowed_base_when_it_dominates : $@convention(thin) (@owned ClassLet) -> () {
// CHECK: } // end sil function 'do_or_dont_copy_let_properties_with_multi_borrowed_base_when_it_dominates_2'
sil [ossa] @do_or_dont_copy_let_properties_with_multi_borrowed_base_when_it_dominates_2 : $@convention(thin) (@owned ClassLet) -> () {
bb0(%x : @owned $ClassLet):
%f = function_ref @black_hole : $@convention(thin) (@guaranteed Klass) -> ()
@@ -693,18 +742,17 @@ bb0(%x : @owned $ClassLet):
%b = begin_borrow %x : $ClassLet
%q = ref_element_addr %b : $ClassLet, #ClassLet.aLet
%w = load [copy] %q : $*Klass
%d = begin_borrow %w : $Klass
apply %f(%d) : $@convention(thin) (@guaranteed Klass) -> ()
// End the lifetime of the base object first...
end_borrow %b : $ClassLet
destroy_value %x : $ClassLet
// ...then end the lifetime of the copy.
apply %f(%d) : $@convention(thin) (@guaranteed Klass) -> ()
%f2 = function_ref @guaranteed_fakeoptional_klass_user : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> ()
%w2 = enum $FakeOptional<Klass>, #FakeOptional.some!enumelt.1, %w : $Klass
apply %f2(%w2) : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> ()
end_borrow %d : $Klass
destroy_value %w : $Klass
destroy_value %w2 : $FakeOptional<Klass>
return undef : $()
}