Skip to content

Commit ba6c70d

Browse files
nate-chandlernate-chandler
committed
[OSSACompleteLifetime] Recurse into scoped addrs.
Previously, when determining and completing lifetimes of scoped addresses, `computeTransitiveLiveness` was used to determine the liveness used for completing the lifetime. That approach had two problems: (1) The function does not find scope-ending uses of `load_borrow`s. The result was determining that the lifetime of the enclosing `store_borrow` ended before that of the `load_borrow`. (2) The function did not complete lifetimes of values defined within the scoped address whose lifetimes the scoped address had to contain. This was an inconsistency between the handling of scoped addresses and that of values. Here, both are addressed by implementing a `TransitiveAddressWalker` (as `computeTransitiveLiveness`'s callee does) which not only visits existing `end_borrow`s of `load_borrows` but completes them (and other inner guaranteed values or scoped addresses). rdar://141246601
1 parent 0c4917c commit ba6c70d

File tree

3 files changed

+219
-1
lines changed

3 files changed

+219
-1
lines changed

‎lib/SIL/Utils/OSSALifetimeCompletion.cpp

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151

5252
#include "swift/SIL/OSSALifetimeCompletion.h"
5353
#include "swift/Basic/Assertions.h"
54+
#include "swift/SIL/AddressWalker.h"
5455
#include "swift/SIL/BasicBlockUtils.h"
5556
#include "swift/SIL/SILBuilder.h"
5657
#include "swift/SIL/SILFunction.h"
@@ -443,7 +444,47 @@ bool OSSALifetimeCompletion::analyzeAndUpdateLifetime(
443444
ScopedAddressValue scopedAddress, Boundary boundary) {
444445
SmallVector<SILBasicBlock *, 8> discoveredBlocks;
445446
SSAPrunedLiveness liveness(scopedAddress->getFunction(), &discoveredBlocks);
446-
scopedAddress.computeTransitiveLiveness(liveness);
447+
liveness.initializeDef(scopedAddress.value);
448+
449+
struct Walker : TransitiveAddressWalker<Walker> {
450+
OSSALifetimeCompletion &completion;
451+
ScopedAddressValue scopedAddress;
452+
Boundary boundary;
453+
SSAPrunedLiveness &liveness;
454+
Walker(OSSALifetimeCompletion &completion, ScopedAddressValue scopedAddress,
455+
Boundary boundary, SSAPrunedLiveness &liveness)
456+
: completion(completion), scopedAddress(scopedAddress),
457+
boundary(boundary), liveness(liveness) {}
458+
bool visitUse(Operand *use) {
459+
auto *user = use->getUser();
460+
if (scopedAddress.isScopeEndingUse(use)) {
461+
liveness.updateForUse(user, /*lifetimeEnding=*/true);
462+
return true;
463+
}
464+
liveness.updateForUse(user, /*lifetimeEnding=*/false);
465+
for (auto result : user->getResults()) {
466+
auto shouldComplete =
467+
(bool)BorrowedValue(result) || (bool)ScopedAddressValue(result);
468+
if (!shouldComplete)
469+
continue;
470+
auto completed = completion.completeOSSALifetime(result, boundary);
471+
switch (completed) {
472+
case LifetimeCompletion::NoLifetime:
473+
break;
474+
case LifetimeCompletion::AlreadyComplete:
475+
case LifetimeCompletion::WasCompleted:
476+
for (auto *consume : result->getConsumingUses()) {
477+
liveness.updateForUse(consume->getUser(), /*lifetimeEnding=*/false);
478+
}
479+
break;
480+
}
481+
}
482+
return true;
483+
}
484+
};
485+
Walker walker(*this, scopedAddress, boundary, liveness);
486+
std::move(walker).walk(scopedAddress.value);
487+
447488
return endLifetimeAtBoundary(scopedAddress.value, liveness, boundary,
448489
deadEndBlocks);
449490
}

‎test/SILOptimizer/mem2reg_ossa_nontrivial.sil

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ sil [ossa] @get_nontrivialstruct : $@convention(thin) () -> @owned NonTrivialStr
6262
sil [ossa] @get_nontrivialenum : $@convention(thin) () -> @owned NonTrivialEnum
6363
sil [ossa] @get_optionalnontrivialstruct : $@convention(thin) () -> @owned FakeOptional<NonTrivialStruct>
6464
sil [ossa] @take_nontrivialstruct : $@convention(thin) (@owned NonTrivialStruct) -> ()
65+
sil @get : $@convention(thin) () -> @owned FakeOptional<Klass>
66+
sil @use : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> ()
6567

6668
///////////
6769
// Tests //
@@ -1487,3 +1489,63 @@ bb1:
14871489
%t = tuple ()
14881490
return %t : $()
14891491
}
1492+
1493+
// Ensure no verification failure
1494+
sil [ossa] @test_store_enum_value_in_multiple_locs1 : $@convention(thin) () -> () {
1495+
bb0:
1496+
%0 = function_ref @get : $@convention(thin) () -> @owned FakeOptional<Klass>
1497+
%1 = apply %0() : $@convention(thin) () -> @owned FakeOptional<Klass>
1498+
%2 = begin_borrow [lexical] %1
1499+
cond_br undef, bb2, bb1
1500+
1501+
bb1:
1502+
br bb3
1503+
1504+
bb2:
1505+
%5 = alloc_stack [lexical] $FakeOptional<Klass>
1506+
%6 = store_borrow %2 to %5
1507+
cond_br undef, bb8, bb9
1508+
1509+
bb3:
1510+
cond_br undef, bb5, bb4
1511+
1512+
bb4:
1513+
unreachable
1514+
1515+
bb5:
1516+
%11 = alloc_stack $FakeOptional<Klass>
1517+
%12 = store_borrow %2 to %11
1518+
%14 = load_borrow %12
1519+
%15 = begin_borrow [lexical] %14
1520+
%16 = alloc_stack $FakeOptional<Klass>
1521+
%17 = store_borrow %15 to %16
1522+
cond_br undef, bb6, bb7
1523+
1524+
bb6:
1525+
fix_lifetime %17
1526+
end_borrow %17
1527+
dealloc_stack %16
1528+
end_borrow %15
1529+
end_borrow %14
1530+
end_borrow %12
1531+
dealloc_stack %11
1532+
end_borrow %2
1533+
destroy_value %1
1534+
%29 = tuple ()
1535+
return %29
1536+
1537+
bb7:
1538+
end_borrow %17
1539+
dealloc_stack %16
1540+
unreachable
1541+
1542+
bb8:
1543+
end_borrow %6
1544+
dealloc_stack %5
1545+
unreachable
1546+
1547+
bb9:
1548+
end_borrow %6
1549+
dealloc_stack %5
1550+
br bb3
1551+
}

‎test/SILOptimizer/ossa_lifetime_completion.sil

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,90 @@ die:
817817
unreachable
818818
}
819819

820+
// CHECK-LABEL: begin running test {{.*}} on load_borrow_of_store_borrow_1: ossa_lifetime_completion
821+
// CHECK-LABEL: sil [ossa] @load_borrow_of_store_borrow_1 : {{.*}} {
822+
// CHECK: [[TOKEN:%[^,]+]] = store_borrow
823+
// CHECK: [[LOAD:%[^,]+]] = load_borrow [[TOKEN]]
824+
// CHECK: [[BORROW:%[^,]+]] = begin_borrow [[LOAD]]
825+
// CHECK: cond_br undef, {{bb[0-9]+}}, [[DIE:bb[0-9]+]]
826+
// CHECK: [[DIE]]:
827+
// CHECK-NEXT: end_borrow [[BORROW]]
828+
// CHECK-NEXT: end_borrow [[LOAD]]
829+
// CHECK-NEXT: end_borrow [[TOKEN]]
830+
// CHECK-NEXT: unreachable
831+
// CHECK-LABEL: } // end sil function 'load_borrow_of_store_borrow_1'
832+
// CHECK-LABEL: end running test {{.*}} on load_borrow_of_store_borrow_1: ossa_lifetime_completion
833+
sil [ossa] @load_borrow_of_store_borrow_1 : $@convention(thin) (@guaranteed C) -> () {
834+
entry(%instance : @guaranteed $C):
835+
specify_test "ossa_lifetime_completion %token availability"
836+
%addr = alloc_stack $C
837+
%token = store_borrow %instance to %addr
838+
%load = load_borrow %token
839+
%borrow = begin_borrow %load
840+
cond_br undef, exit, die
841+
842+
exit:
843+
end_borrow %borrow
844+
end_borrow %load
845+
end_borrow %token
846+
dealloc_stack %addr
847+
%retval = tuple ()
848+
return %retval : $()
849+
die:
850+
unreachable
851+
}
852+
853+
// CHECK-LABEL: begin running test {{.*}} on load_borrow_of_store_borrow_2: ossa_lifetime_completion
854+
// CHECK-LABEL: sil [ossa] @load_borrow_of_store_borrow_2 : {{.*}} {
855+
// CHECK: [[TOKEN:%[^,]+]] = store_borrow
856+
// CHECK: [[LOAD:%[^,]+]] = load_borrow [[TOKEN]]
857+
// CHECK: [[BORROW:%[^,]+]] = begin_borrow [[LOAD]]
858+
// CHECK: cond_br undef, {{bb[0-9]+}}, [[DIE:bb[0-9]+]]
859+
// CHECK: [[DIE]]:
860+
// CHECK-NEXT: end_borrow [[BORROW]]
861+
// CHECK-NEXT: end_borrow [[LOAD]]
862+
// CHECK-NEXT: end_borrow [[TOKEN]]
863+
// CHECK-NEXT: unreachable
864+
// CHECK-LABEL: } // end sil function 'load_borrow_of_store_borrow_2'
865+
// CHECK-LABEL: end running test {{.*}} on load_borrow_of_store_borrow_2: ossa_lifetime_completion
866+
sil [ossa] @load_borrow_of_store_borrow_2 : $@convention(thin) (@guaranteed C) -> () {
867+
entry(%instance : @guaranteed $C):
868+
specify_test "ossa_lifetime_completion %token availability"
869+
%addr = alloc_stack $C
870+
%token = store_borrow %instance to %addr
871+
%load = load_borrow %token
872+
%borrow = begin_borrow %load
873+
cond_br undef, exit, die
874+
875+
exit:
876+
end_borrow %borrow
877+
end_borrow %load
878+
end_borrow %token
879+
dealloc_stack %addr
880+
%retval = tuple ()
881+
return %retval : $()
882+
die:
883+
end_borrow %borrow
884+
end_borrow %load
885+
unreachable
886+
}
887+
888+
sil [ossa] @load_borrow_1 : $@convention(thin) (@in_guaranteed C) -> () {
889+
entry(%addr : $*C):
890+
specify_test "ossa_lifetime_completion %load availability"
891+
%load = load_borrow %addr
892+
%borrow = begin_borrow %load
893+
cond_br undef, exit, die
894+
895+
exit:
896+
end_borrow %borrow
897+
end_borrow %load
898+
%retval = tuple ()
899+
return %retval : $()
900+
die:
901+
unreachable
902+
}
903+
820904
// CHECK-LABEL: begin running test {{.*}} on begin_access: ossa_lifetime_completion
821905
// CHECK-LABEL: sil [ossa] @begin_access : {{.*}} {
822906
// CHECK: [[TOKEN:%[^,]+]] = begin_access
@@ -841,3 +925,34 @@ exit:
841925
die:
842926
unreachable
843927
}
928+
929+
// CHECK-LABEL: begin running test {{.*}} on load_borrow_of_begin_access: ossa_lifetime_completion
930+
// CHECK-LABEL: sil [ossa] @load_borrow_of_begin_access : {{.*}} {
931+
// CHECK: [[ACCESS:%[^,]+]] = begin_access
932+
// CHECK: [[LOAD:%[^,]+]] = load_borrow [[ACCESS]]
933+
// CHECK: [[BORROW:%[^,]+]] = begin_borrow [[LOAD]]
934+
// CHECK: cond_br undef, {{bb[0-9]+}}, [[DIE:bb[0-9]+]]
935+
// CHECK: [[DIE]]:
936+
// CHECK-NEXT: end_borrow [[BORROW]]
937+
// CHECK-NEXT: end_borrow [[LOAD]]
938+
// CHECK-NEXT: end_access [[ACCESS]]
939+
// CHECK-NEXT: unreachable
940+
// CHECK-LABEL: } // end sil function 'load_borrow_of_begin_access'
941+
// CHECK-LABEL: end running test {{.*}} on load_borrow_of_begin_access: ossa_lifetime_completion
942+
sil [ossa] @load_borrow_of_begin_access : $@convention(thin) (@in_guaranteed C) -> () {
943+
entry(%addr : $*C):
944+
specify_test "ossa_lifetime_completion %access availability"
945+
%access = begin_access [static] [read] %addr : $*C
946+
%load = load_borrow %access
947+
%borrow = begin_borrow %load
948+
cond_br undef, exit, die
949+
950+
exit:
951+
end_borrow %borrow
952+
end_borrow %load
953+
end_access %access : $*C
954+
%retval = tuple ()
955+
return %retval : $()
956+
die:
957+
unreachable
958+
}

0 commit comments

Comments
 (0)