From 0c4917ce7d5bc75752704a20ca0539639fda718d Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Fri, 13 Dec 2024 12:22:31 -0800 Subject: [PATCH 1/3] [NFC] OSSACompleteLifetime: Extracted method. In preparation to expand it. --- include/swift/SIL/OSSALifetimeCompletion.h | 1 + lib/SIL/Utils/OSSALifetimeCompletion.cpp | 14 ++++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/include/swift/SIL/OSSALifetimeCompletion.h b/include/swift/SIL/OSSALifetimeCompletion.h index 6749f5d03250c..062724ee5613c 100644 --- a/include/swift/SIL/OSSALifetimeCompletion.h +++ b/include/swift/SIL/OSSALifetimeCompletion.h @@ -134,6 +134,7 @@ class OSSALifetimeCompletion { protected: bool analyzeAndUpdateLifetime(SILValue value, Boundary boundary); + bool analyzeAndUpdateLifetime(ScopedAddressValue value, Boundary boundary); }; //===----------------------------------------------------------------------===// diff --git a/lib/SIL/Utils/OSSALifetimeCompletion.cpp b/lib/SIL/Utils/OSSALifetimeCompletion.cpp index 08943b8337a14..be5e5e2c14c5a 100644 --- a/lib/SIL/Utils/OSSALifetimeCompletion.cpp +++ b/lib/SIL/Utils/OSSALifetimeCompletion.cpp @@ -439,16 +439,22 @@ static bool endLifetimeAtBoundary(SILValue value, return changed; } +bool OSSALifetimeCompletion::analyzeAndUpdateLifetime( + ScopedAddressValue scopedAddress, Boundary boundary) { + SmallVector discoveredBlocks; + SSAPrunedLiveness liveness(scopedAddress->getFunction(), &discoveredBlocks); + scopedAddress.computeTransitiveLiveness(liveness); + return endLifetimeAtBoundary(scopedAddress.value, liveness, boundary, + deadEndBlocks); +} + /// End the lifetime of \p value at unreachable instructions. /// /// Returns true if any new instructions were created to complete the lifetime. bool OSSALifetimeCompletion::analyzeAndUpdateLifetime(SILValue value, Boundary boundary) { if (auto scopedAddress = ScopedAddressValue(value)) { - SmallVector discoveredBlocks; - SSAPrunedLiveness liveness(value->getFunction(), &discoveredBlocks); - scopedAddress.computeTransitiveLiveness(liveness); - return endLifetimeAtBoundary(value, liveness, boundary, deadEndBlocks); + return analyzeAndUpdateLifetime(scopedAddress, boundary); } // Called for inner borrows, inner adjacent reborrows, inner reborrows, and From ba6c70db4be80bac3e9c490b655df8caab98e58b Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Fri, 13 Dec 2024 14:26:17 -0800 Subject: [PATCH 2/3] [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 --- lib/SIL/Utils/OSSALifetimeCompletion.cpp | 43 ++++++- test/SILOptimizer/mem2reg_ossa_nontrivial.sil | 62 ++++++++++ .../SILOptimizer/ossa_lifetime_completion.sil | 115 ++++++++++++++++++ 3 files changed, 219 insertions(+), 1 deletion(-) diff --git a/lib/SIL/Utils/OSSALifetimeCompletion.cpp b/lib/SIL/Utils/OSSALifetimeCompletion.cpp index be5e5e2c14c5a..296fa0163558a 100644 --- a/lib/SIL/Utils/OSSALifetimeCompletion.cpp +++ b/lib/SIL/Utils/OSSALifetimeCompletion.cpp @@ -51,6 +51,7 @@ #include "swift/SIL/OSSALifetimeCompletion.h" #include "swift/Basic/Assertions.h" +#include "swift/SIL/AddressWalker.h" #include "swift/SIL/BasicBlockUtils.h" #include "swift/SIL/SILBuilder.h" #include "swift/SIL/SILFunction.h" @@ -443,7 +444,47 @@ bool OSSALifetimeCompletion::analyzeAndUpdateLifetime( ScopedAddressValue scopedAddress, Boundary boundary) { SmallVector discoveredBlocks; SSAPrunedLiveness liveness(scopedAddress->getFunction(), &discoveredBlocks); - scopedAddress.computeTransitiveLiveness(liveness); + liveness.initializeDef(scopedAddress.value); + + struct Walker : TransitiveAddressWalker { + OSSALifetimeCompletion &completion; + ScopedAddressValue scopedAddress; + Boundary boundary; + SSAPrunedLiveness &liveness; + Walker(OSSALifetimeCompletion &completion, ScopedAddressValue scopedAddress, + Boundary boundary, SSAPrunedLiveness &liveness) + : completion(completion), scopedAddress(scopedAddress), + boundary(boundary), liveness(liveness) {} + bool visitUse(Operand *use) { + auto *user = use->getUser(); + if (scopedAddress.isScopeEndingUse(use)) { + liveness.updateForUse(user, /*lifetimeEnding=*/true); + return true; + } + liveness.updateForUse(user, /*lifetimeEnding=*/false); + for (auto result : user->getResults()) { + auto shouldComplete = + (bool)BorrowedValue(result) || (bool)ScopedAddressValue(result); + if (!shouldComplete) + continue; + auto completed = completion.completeOSSALifetime(result, boundary); + switch (completed) { + case LifetimeCompletion::NoLifetime: + break; + case LifetimeCompletion::AlreadyComplete: + case LifetimeCompletion::WasCompleted: + for (auto *consume : result->getConsumingUses()) { + liveness.updateForUse(consume->getUser(), /*lifetimeEnding=*/false); + } + break; + } + } + return true; + } + }; + Walker walker(*this, scopedAddress, boundary, liveness); + std::move(walker).walk(scopedAddress.value); + return endLifetimeAtBoundary(scopedAddress.value, liveness, boundary, deadEndBlocks); } diff --git a/test/SILOptimizer/mem2reg_ossa_nontrivial.sil b/test/SILOptimizer/mem2reg_ossa_nontrivial.sil index 322364766b4b9..4bae535d1bd4a 100644 --- a/test/SILOptimizer/mem2reg_ossa_nontrivial.sil +++ b/test/SILOptimizer/mem2reg_ossa_nontrivial.sil @@ -62,6 +62,8 @@ sil [ossa] @get_nontrivialstruct : $@convention(thin) () -> @owned NonTrivialStr sil [ossa] @get_nontrivialenum : $@convention(thin) () -> @owned NonTrivialEnum sil [ossa] @get_optionalnontrivialstruct : $@convention(thin) () -> @owned FakeOptional sil [ossa] @take_nontrivialstruct : $@convention(thin) (@owned NonTrivialStruct) -> () +sil @get : $@convention(thin) () -> @owned FakeOptional +sil @use : $@convention(thin) (@guaranteed FakeOptional) -> () /////////// // Tests // @@ -1487,3 +1489,63 @@ bb1: %t = tuple () return %t : $() } + +// Ensure no verification failure +sil [ossa] @test_store_enum_value_in_multiple_locs1 : $@convention(thin) () -> () { +bb0: + %0 = function_ref @get : $@convention(thin) () -> @owned FakeOptional + %1 = apply %0() : $@convention(thin) () -> @owned FakeOptional + %2 = begin_borrow [lexical] %1 + cond_br undef, bb2, bb1 + +bb1: + br bb3 + +bb2: + %5 = alloc_stack [lexical] $FakeOptional + %6 = store_borrow %2 to %5 + cond_br undef, bb8, bb9 + +bb3: + cond_br undef, bb5, bb4 + +bb4: + unreachable + +bb5: + %11 = alloc_stack $FakeOptional + %12 = store_borrow %2 to %11 + %14 = load_borrow %12 + %15 = begin_borrow [lexical] %14 + %16 = alloc_stack $FakeOptional + %17 = store_borrow %15 to %16 + cond_br undef, bb6, bb7 + +bb6: + fix_lifetime %17 + end_borrow %17 + dealloc_stack %16 + end_borrow %15 + end_borrow %14 + end_borrow %12 + dealloc_stack %11 + end_borrow %2 + destroy_value %1 + %29 = tuple () + return %29 + +bb7: + end_borrow %17 + dealloc_stack %16 + unreachable + +bb8: + end_borrow %6 + dealloc_stack %5 + unreachable + +bb9: + end_borrow %6 + dealloc_stack %5 + br bb3 +} diff --git a/test/SILOptimizer/ossa_lifetime_completion.sil b/test/SILOptimizer/ossa_lifetime_completion.sil index 8cfc0d90dd9d1..1b281f8608626 100644 --- a/test/SILOptimizer/ossa_lifetime_completion.sil +++ b/test/SILOptimizer/ossa_lifetime_completion.sil @@ -817,6 +817,90 @@ die: unreachable } +// CHECK-LABEL: begin running test {{.*}} on load_borrow_of_store_borrow_1: ossa_lifetime_completion +// CHECK-LABEL: sil [ossa] @load_borrow_of_store_borrow_1 : {{.*}} { +// CHECK: [[TOKEN:%[^,]+]] = store_borrow +// CHECK: [[LOAD:%[^,]+]] = load_borrow [[TOKEN]] +// CHECK: [[BORROW:%[^,]+]] = begin_borrow [[LOAD]] +// CHECK: cond_br undef, {{bb[0-9]+}}, [[DIE:bb[0-9]+]] +// CHECK: [[DIE]]: +// CHECK-NEXT: end_borrow [[BORROW]] +// CHECK-NEXT: end_borrow [[LOAD]] +// CHECK-NEXT: end_borrow [[TOKEN]] +// CHECK-NEXT: unreachable +// CHECK-LABEL: } // end sil function 'load_borrow_of_store_borrow_1' +// CHECK-LABEL: end running test {{.*}} on load_borrow_of_store_borrow_1: ossa_lifetime_completion +sil [ossa] @load_borrow_of_store_borrow_1 : $@convention(thin) (@guaranteed C) -> () { +entry(%instance : @guaranteed $C): + specify_test "ossa_lifetime_completion %token availability" + %addr = alloc_stack $C + %token = store_borrow %instance to %addr + %load = load_borrow %token + %borrow = begin_borrow %load + cond_br undef, exit, die + +exit: + end_borrow %borrow + end_borrow %load + end_borrow %token + dealloc_stack %addr + %retval = tuple () + return %retval : $() +die: + unreachable +} + +// CHECK-LABEL: begin running test {{.*}} on load_borrow_of_store_borrow_2: ossa_lifetime_completion +// CHECK-LABEL: sil [ossa] @load_borrow_of_store_borrow_2 : {{.*}} { +// CHECK: [[TOKEN:%[^,]+]] = store_borrow +// CHECK: [[LOAD:%[^,]+]] = load_borrow [[TOKEN]] +// CHECK: [[BORROW:%[^,]+]] = begin_borrow [[LOAD]] +// CHECK: cond_br undef, {{bb[0-9]+}}, [[DIE:bb[0-9]+]] +// CHECK: [[DIE]]: +// CHECK-NEXT: end_borrow [[BORROW]] +// CHECK-NEXT: end_borrow [[LOAD]] +// CHECK-NEXT: end_borrow [[TOKEN]] +// CHECK-NEXT: unreachable +// CHECK-LABEL: } // end sil function 'load_borrow_of_store_borrow_2' +// CHECK-LABEL: end running test {{.*}} on load_borrow_of_store_borrow_2: ossa_lifetime_completion +sil [ossa] @load_borrow_of_store_borrow_2 : $@convention(thin) (@guaranteed C) -> () { +entry(%instance : @guaranteed $C): + specify_test "ossa_lifetime_completion %token availability" + %addr = alloc_stack $C + %token = store_borrow %instance to %addr + %load = load_borrow %token + %borrow = begin_borrow %load + cond_br undef, exit, die + +exit: + end_borrow %borrow + end_borrow %load + end_borrow %token + dealloc_stack %addr + %retval = tuple () + return %retval : $() +die: + end_borrow %borrow + end_borrow %load + unreachable +} + +sil [ossa] @load_borrow_1 : $@convention(thin) (@in_guaranteed C) -> () { +entry(%addr : $*C): + specify_test "ossa_lifetime_completion %load availability" + %load = load_borrow %addr + %borrow = begin_borrow %load + cond_br undef, exit, die + +exit: + end_borrow %borrow + end_borrow %load + %retval = tuple () + return %retval : $() +die: + unreachable +} + // CHECK-LABEL: begin running test {{.*}} on begin_access: ossa_lifetime_completion // CHECK-LABEL: sil [ossa] @begin_access : {{.*}} { // CHECK: [[TOKEN:%[^,]+]] = begin_access @@ -841,3 +925,34 @@ exit: die: unreachable } + +// CHECK-LABEL: begin running test {{.*}} on load_borrow_of_begin_access: ossa_lifetime_completion +// CHECK-LABEL: sil [ossa] @load_borrow_of_begin_access : {{.*}} { +// CHECK: [[ACCESS:%[^,]+]] = begin_access +// CHECK: [[LOAD:%[^,]+]] = load_borrow [[ACCESS]] +// CHECK: [[BORROW:%[^,]+]] = begin_borrow [[LOAD]] +// CHECK: cond_br undef, {{bb[0-9]+}}, [[DIE:bb[0-9]+]] +// CHECK: [[DIE]]: +// CHECK-NEXT: end_borrow [[BORROW]] +// CHECK-NEXT: end_borrow [[LOAD]] +// CHECK-NEXT: end_access [[ACCESS]] +// CHECK-NEXT: unreachable +// CHECK-LABEL: } // end sil function 'load_borrow_of_begin_access' +// CHECK-LABEL: end running test {{.*}} on load_borrow_of_begin_access: ossa_lifetime_completion +sil [ossa] @load_borrow_of_begin_access : $@convention(thin) (@in_guaranteed C) -> () { +entry(%addr : $*C): + specify_test "ossa_lifetime_completion %access availability" + %access = begin_access [static] [read] %addr : $*C + %load = load_borrow %access + %borrow = begin_borrow %load + cond_br undef, exit, die + +exit: + end_borrow %borrow + end_borrow %load + end_access %access : $*C + %retval = tuple () + return %retval : $() +die: + unreachable +} From 1726b321b05157546b14a8afcbacfde3cf6f7663 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Mon, 16 Dec 2024 07:40:06 -0800 Subject: [PATCH 3/3] [Test] Backport SIL tests to 6.1. On main type annotations are not always required but on 6.1 they are. --- test/SILOptimizer/mem2reg_ossa_nontrivial.sil | 44 ++++++++--------- .../SILOptimizer/ossa_lifetime_completion.sil | 48 +++++++++---------- 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/test/SILOptimizer/mem2reg_ossa_nontrivial.sil b/test/SILOptimizer/mem2reg_ossa_nontrivial.sil index 4bae535d1bd4a..55400685629a7 100644 --- a/test/SILOptimizer/mem2reg_ossa_nontrivial.sil +++ b/test/SILOptimizer/mem2reg_ossa_nontrivial.sil @@ -1495,7 +1495,7 @@ sil [ossa] @test_store_enum_value_in_multiple_locs1 : $@convention(thin) () -> ( bb0: %0 = function_ref @get : $@convention(thin) () -> @owned FakeOptional %1 = apply %0() : $@convention(thin) () -> @owned FakeOptional - %2 = begin_borrow [lexical] %1 + %2 = begin_borrow [lexical] %1 : $FakeOptional cond_br undef, bb2, bb1 bb1: @@ -1503,7 +1503,7 @@ bb1: bb2: %5 = alloc_stack [lexical] $FakeOptional - %6 = store_borrow %2 to %5 + %6 = store_borrow %2 to %5 : $*FakeOptional cond_br undef, bb8, bb9 bb3: @@ -1514,38 +1514,38 @@ bb4: bb5: %11 = alloc_stack $FakeOptional - %12 = store_borrow %2 to %11 - %14 = load_borrow %12 - %15 = begin_borrow [lexical] %14 + %12 = store_borrow %2 to %11 : $*FakeOptional + %14 = load_borrow %12 : $*FakeOptional + %15 = begin_borrow [lexical] %14 : $FakeOptional %16 = alloc_stack $FakeOptional - %17 = store_borrow %15 to %16 + %17 = store_borrow %15 to %16 : $*FakeOptional cond_br undef, bb6, bb7 bb6: - fix_lifetime %17 - end_borrow %17 - dealloc_stack %16 - end_borrow %15 - end_borrow %14 - end_borrow %12 - dealloc_stack %11 - end_borrow %2 - destroy_value %1 + fix_lifetime %17 : $*FakeOptional + end_borrow %17 : $*FakeOptional + dealloc_stack %16 : $*FakeOptional + end_borrow %15 : $FakeOptional + end_borrow %14 : $FakeOptional + end_borrow %12 : $*FakeOptional + dealloc_stack %11 : $*FakeOptional + end_borrow %2 : $FakeOptional + destroy_value %1 : $FakeOptional %29 = tuple () - return %29 + return %29 : $() bb7: - end_borrow %17 - dealloc_stack %16 + end_borrow %17 : $*FakeOptional + dealloc_stack %16 : $*FakeOptional unreachable bb8: - end_borrow %6 - dealloc_stack %5 + end_borrow %6 : $*FakeOptional + dealloc_stack %5 : $*FakeOptional unreachable bb9: - end_borrow %6 - dealloc_stack %5 + end_borrow %6 : $*FakeOptional + dealloc_stack %5 : $*FakeOptional br bb3 } diff --git a/test/SILOptimizer/ossa_lifetime_completion.sil b/test/SILOptimizer/ossa_lifetime_completion.sil index 1b281f8608626..72bc08961c441 100644 --- a/test/SILOptimizer/ossa_lifetime_completion.sil +++ b/test/SILOptimizer/ossa_lifetime_completion.sil @@ -834,16 +834,16 @@ sil [ossa] @load_borrow_of_store_borrow_1 : $@convention(thin) (@guaranteed C) - entry(%instance : @guaranteed $C): specify_test "ossa_lifetime_completion %token availability" %addr = alloc_stack $C - %token = store_borrow %instance to %addr - %load = load_borrow %token - %borrow = begin_borrow %load + %token = store_borrow %instance to %addr : $*C + %load = load_borrow %token : $*C + %borrow = begin_borrow %load : $C cond_br undef, exit, die exit: - end_borrow %borrow - end_borrow %load - end_borrow %token - dealloc_stack %addr + end_borrow %borrow : $C + end_borrow %load : $C + end_borrow %token : $*C + dealloc_stack %addr : $*C %retval = tuple () return %retval : $() die: @@ -867,34 +867,34 @@ sil [ossa] @load_borrow_of_store_borrow_2 : $@convention(thin) (@guaranteed C) - entry(%instance : @guaranteed $C): specify_test "ossa_lifetime_completion %token availability" %addr = alloc_stack $C - %token = store_borrow %instance to %addr - %load = load_borrow %token - %borrow = begin_borrow %load + %token = store_borrow %instance to %addr : $*C + %load = load_borrow %token : $*C + %borrow = begin_borrow %load : $C cond_br undef, exit, die exit: - end_borrow %borrow - end_borrow %load - end_borrow %token - dealloc_stack %addr + end_borrow %borrow : $C + end_borrow %load : $C + end_borrow %token : $*C + dealloc_stack %addr : $*C %retval = tuple () return %retval : $() die: - end_borrow %borrow - end_borrow %load + end_borrow %borrow : $C + end_borrow %load : $C unreachable } sil [ossa] @load_borrow_1 : $@convention(thin) (@in_guaranteed C) -> () { entry(%addr : $*C): specify_test "ossa_lifetime_completion %load availability" - %load = load_borrow %addr - %borrow = begin_borrow %load + %load = load_borrow %addr : $*C + %borrow = begin_borrow %load : $C cond_br undef, exit, die exit: - end_borrow %borrow - end_borrow %load + end_borrow %borrow : $C + end_borrow %load : $C %retval = tuple () return %retval : $() die: @@ -943,13 +943,13 @@ sil [ossa] @load_borrow_of_begin_access : $@convention(thin) (@in_guaranteed C) entry(%addr : $*C): specify_test "ossa_lifetime_completion %access availability" %access = begin_access [static] [read] %addr : $*C - %load = load_borrow %access - %borrow = begin_borrow %load + %load = load_borrow %access : $*C + %borrow = begin_borrow %load : $C cond_br undef, exit, die exit: - end_borrow %borrow - end_borrow %load + end_borrow %borrow : $C + end_borrow %load : $C end_access %access : $*C %retval = tuple () return %retval : $()