Skip to content

Commit 6c67762

Browse files
nate-chandlernate-chandler
committed
[CanOSSALife] Lexical lifetimes go to unreachables
When canonicalizing the lifetime of a lexical value, deinit barriers are respected. This is done by walking backwards from lifetime ends and adding encountered deinit barriers to liveness. Only destroy lifetime ends were walked back from under the assumption that lifetimes would be complete. Without complete OSSA lifetimes, however, it's necessary to also necessary to consider lifetimes that end with unreachables. Unfortunately, we can't simply walk back from those unreachables because there may be instructions which are secretly users of the value being canonicalized (e.g. destroys of `partial_apply`s to which a `begin_borrow` of the value was passed). Such uses don't appear in the use list because lifetime canonicalization expects complete lifetimes and only visits lifetime ends of `begin_borrow`s. Here, instead, the instructions before the relevant unreachables are added to liveness. In order to determine which unreachables are relevant, it's necessary to have a liveness that includes the original destroys. So a copy of liveness is created and those destroys are added to it. rdar://115468707
1 parent 43d5743 commit 6c67762

File tree

4 files changed

+154
-7
lines changed

4 files changed

+154
-7
lines changed

‎include/swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ class CanonicalizeOSSALifetime final {
319319

320320
currentDef = def;
321321

322-
if (maximizeLifetime) {
322+
if (maximizeLifetime || respectsDeinitBarriers()) {
323323
liveness->initializeDiscoveredBlocks(&discoveredBlocks);
324324
}
325325
liveness->initializeDef(getCurrentDef());
@@ -405,8 +405,9 @@ class CanonicalizeOSSALifetime final {
405405

406406
void recordDebugValue(DebugValueInst *dvi) { debugValues.insert(dvi); }
407407

408-
void recordConsumingUse(Operand *use) {
409-
consumingBlocks.insert(use->getUser()->getParent());
408+
void recordConsumingUse(Operand *use) { recordConsumingUser(use->getUser()); }
409+
void recordConsumingUser(SILInstruction *user) {
410+
consumingBlocks.insert(user->getParent());
410411
}
411412
bool computeCanonicalLiveness();
412413

‎lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
#include "swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h"
6969
#include "swift/SIL/InstructionUtils.h"
7070
#include "swift/SIL/NodeDatastructures.h"
71+
#include "swift/SIL/OSSALifetimeCompletion.h"
7172
#include "swift/SIL/OwnershipUtils.h"
7273
#include "swift/SIL/PrunedLiveness.h"
7374
#include "swift/SIL/Test.h"
@@ -249,6 +250,35 @@ void CanonicalizeOSSALifetime::extendLivenessToDeinitBarriers() {
249250
SmallVector<SILInstruction *, 4> outsideDestroys;
250251
findDestroysOutsideBoundary(outsideDestroys);
251252

253+
// OSSALifetimeCompletion: With complete lifetimes, creating completeLiveness
254+
// and using it to visiti unreachable lifetime ends should be deleted.
255+
SmallVector<SILBasicBlock *, 32> discoveredBlocks(this->discoveredBlocks);
256+
SSAPrunedLiveness completeLiveness(*liveness, &discoveredBlocks);
257+
258+
for (auto *end : outsideDestroys) {
259+
completeLiveness.updateForUse(end, /*lifetimeEnding*/ true);
260+
}
261+
262+
OSSALifetimeCompletion::visitUnreachableLifetimeEnds(
263+
getCurrentDef(), completeLiveness, [&](auto *unreachable) {
264+
recordConsumingUser(unreachable);
265+
if (auto *previous = unreachable->getPreviousInstruction()) {
266+
if (liveness->isInterestingUser(previous) ==
267+
PrunedLiveness::IsInterestingUser::NonUser) {
268+
liveness->updateForUse(previous, /*lifetimeEnding=*/false);
269+
}
270+
return;
271+
}
272+
for (auto *predecessor :
273+
unreachable->getParent()->getPredecessorBlocks()) {
274+
auto *previous = &predecessor->back();
275+
if (liveness->isInterestingUser(previous) ==
276+
PrunedLiveness::IsInterestingUser::NonUser) {
277+
liveness->updateForUse(previous, /*lifetimeEnding=*/false);
278+
}
279+
}
280+
});
281+
252282
auto *def = getCurrentDef()->getDefiningInstruction();
253283
using InitialBlocks = ArrayRef<SILBasicBlock *>;
254284
auto *defBlock = getCurrentDef()->getParentBlock();
@@ -890,6 +920,27 @@ static void insertDestroyBeforeInstruction(SILInstruction *nextInstruction,
890920
SILValue currentDef,
891921
CanonicalOSSAConsumeInfo &consumes,
892922
InstModCallbacks &callbacks) {
923+
// OSSALifetimeCompletion: This conditional clause can be deleted with
924+
// complete lifetimes.
925+
if (isa<UnreachableInst>(nextInstruction)) {
926+
// Don't create a destroy_value if the next instruction is an unreachable.
927+
// If there was a destroy here already, it would be reused. Avoids
928+
// creating an explicit destroy of a value which might have an unclosed
929+
// borrow scope. Doing so would result in
930+
//
931+
// somewhere:
932+
// %def
933+
// %borrow = begin_borrow ...
934+
//
935+
// die:
936+
// destroy_value %def
937+
// unreachable
938+
//
939+
// which is invalid (although the verifier doesn't catch
940+
// it--rdar://115850528) because there must be an `end_borrow %borrow`
941+
// before the destroy_value.
942+
return;
943+
}
893944
SILBuilderWithScope builder(nextInstruction);
894945
auto loc =
895946
RegularLocation::getAutoGeneratedLocation(nextInstruction->getLoc());

‎test/SILOptimizer/canonicalize_ossa_lifetime_unit.sil

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,7 @@ entry(%instance : @owned $MoE):
181181
return %retval : $()
182182
}
183183

184-
// CHECK-LABEL: begin running test 1 of 1 on respect_boundary_edges_when_extending_to_deinit_barriers: canonicalize-ossa-lifetime with: true, false, true, @argument
185-
// respect_boundary_edges_when_extending_to_deinit_barriers
184+
// CHECK-LABEL: begin running test 1 of 1 on respect_boundary_edges_when_extending_to_deinit_barriers
186185
// CHECK-LABEL: sil [ossa] @respect_boundary_edges_when_extending_to_deinit_barriers : {{.*}} {
187186
// CHECK: bb0([[INSTANCE:%[^,]+]] :
188187
// CHECK: apply {{%[^,]+}}()
@@ -191,8 +190,9 @@ entry(%instance : @owned $MoE):
191190
// CHECK: unreachable
192191
// CHECK: [[DESTROY]]:
193192
// CHECK: destroy_value [[INSTANCE]] : $C
193+
// CHECK: return
194194
// CHECK-LABEL: } // end sil function 'respect_boundary_edges_when_extending_to_deinit_barriers'
195-
// CHECK-LABEL: end running test 1 of 1 on respect_boundary_edges_when_extending_to_deinit_barriers: canonicalize-ossa-lifetime with: true, false, true, @argument
195+
// CHECK-LABEL: end running test 1 of 1 on respect_boundary_edges_when_extending_to_deinit_barriers
196196
sil [ossa] @respect_boundary_edges_when_extending_to_deinit_barriers : $@convention(thin) (@owned C) -> () {
197197
entry(%instance : @owned $C):
198198
test_specification "canonicalize-ossa-lifetime true false true @argument"
@@ -208,3 +208,97 @@ destroy:
208208
%retval = tuple ()
209209
return %retval : $()
210210
}
211+
212+
// CHECK-LABEL: begin running test {{.*}} on respect_boundary_edges_when_extending_to_deinit_barriers_2
213+
// CHECK-LABEL: sil [ossa] @respect_boundary_edges_when_extending_to_deinit_barriers_2 : {{.*}} {
214+
// CHECK: bb0([[INSTANCE:%[^,]+]] :
215+
// CHECK: apply
216+
// CHECK: cond_br undef, [[DIE:bb[0-9]+]], [[DESTROY:bb[0-9]+]]
217+
// CHECK: [[DIE]]:
218+
// CHECK: apply
219+
// CHECK: unreachable
220+
// CHECK: [[DESTROY]]:
221+
// CHECK: destroy_value [[INSTANCE]]
222+
// CHECK-LABEL: } // end sil function 'respect_boundary_edges_when_extending_to_deinit_barriers_2'
223+
// CHECK-LABEL: end running test {{.*}} on respect_boundary_edges_when_extending_to_deinit_barriers_2
224+
sil [ossa] @respect_boundary_edges_when_extending_to_deinit_barriers_2 : $@convention(thin) (@owned C) -> () {
225+
entry(%instance : @owned $C):
226+
test_specification "canonicalize-ossa-lifetime true false true @argument"
227+
%barrier = function_ref @barrier : $@convention(thin) () -> ()
228+
apply %barrier() : $@convention(thin) () -> ()
229+
cond_br undef, die, destroy
230+
231+
die:
232+
apply %barrier() : $@convention(thin) () -> ()
233+
br rlydie
234+
235+
rlydie:
236+
unreachable
237+
238+
destroy:
239+
destroy_value %instance : $C
240+
%retval = tuple ()
241+
return %retval : $()
242+
}
243+
244+
// CHECK-LABEL: begin running test {{.*}} on respect_boundary_edges_when_extending_to_deinit_barriers_3
245+
// CHECK-LABEL: sil [ossa] @respect_boundary_edges_when_extending_to_deinit_barriers_3 : {{.*}} {
246+
// CHECK: bb0([[INSTANCE:%[^,]+]] :
247+
// CHECK: apply
248+
// CHECK: cond_br undef, [[DIE:bb[0-9]+]], [[DESTROY:bb[0-9]+]]
249+
// CHECK: [[DIE]]:
250+
// CHECK: apply
251+
// CHECK-NOT: destroy_value [[INSTANCE]]
252+
// CHECK: unreachable
253+
// CHECK: [[DESTROY]]:
254+
// CHECK: destroy_value [[INSTANCE]]
255+
// CHECK-LABEL: } // end sil function 'respect_boundary_edges_when_extending_to_deinit_barriers_3'
256+
// CHECK-LABEL: end running test {{.*}} on respect_boundary_edges_when_extending_to_deinit_barriers_3
257+
sil [ossa] @respect_boundary_edges_when_extending_to_deinit_barriers_3 : $@convention(thin) (@owned C) -> () {
258+
entry(%instance : @owned $C):
259+
test_specification "canonicalize-ossa-lifetime true false true @argument"
260+
%barrier = function_ref @barrier : $@convention(thin) () -> ()
261+
apply %barrier() : $@convention(thin) () -> ()
262+
cond_br undef, die, destroy
263+
264+
die:
265+
apply %barrier() : $@convention(thin) () -> ()
266+
unreachable
267+
268+
destroy:
269+
destroy_value %instance : $C
270+
%retval = tuple ()
271+
return %retval : $()
272+
}
273+
274+
// Verify that an explicit destroy_value which happens to be just before an
275+
// unreachable isn't deleted if liveness extends to the previous instruction.
276+
// CHECK-LABEL: begin running test 1 of 1 on respect_preexisting_destroy_values_when_extending_to_deinit_barriers
277+
// CHECK-LABEL: sil [ossa] @respect_preexisting_destroy_values_when_extending_to_deinit_barriers : {{.*}} {
278+
// CHECK: bb0([[INSTANCE:%[^,]+]] :
279+
// CHECK: cond_br undef, [[DIE:bb[0-9]+]], [[DESTROY:bb[0-9]+]]
280+
// CHECK: [[DIE]]:
281+
// CHECK: apply {{%[^,]+}}()
282+
// CHECK: destroy_value [[INSTANCE]] : $C
283+
// CHECK: unreachable
284+
// CHECK: [[DESTROY]]:
285+
// CHECK: destroy_value [[INSTANCE]] : $C
286+
// CHECK: return
287+
// CHECK-LABEL: } // end sil function 'respect_preexisting_destroy_values_when_extending_to_deinit_barriers'
288+
// CHECK-LABEL: end running test 1 of 1 on respect_preexisting_destroy_values_when_extending_to_deinit_barriers
289+
sil [ossa] @respect_preexisting_destroy_values_when_extending_to_deinit_barriers : $@convention(thin) (@owned C) -> () {
290+
entry(%instance : @owned $C):
291+
test_specification "canonicalize-ossa-lifetime true false true @argument"
292+
%barrier = function_ref @barrier : $@convention(thin) () -> ()
293+
cond_br undef, die, destroy
294+
295+
die:
296+
apply %barrier() : $@convention(thin) () -> ()
297+
destroy_value %instance : $C
298+
unreachable
299+
300+
destroy:
301+
destroy_value %instance : $C
302+
%retval = tuple ()
303+
return %retval : $()
304+
}

‎test/SILOptimizer/mem2reg_lifetime.sil

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1858,7 +1858,8 @@ right:
18581858
// CHECK: {{bb[^,]+}}([[INSTANCE:%[^,]+]] : @owned $C):
18591859
// CHECK: cond_br undef, [[BB1:bb[0-9]+]], [[BB4:bb[0-9]+]]
18601860
// CHECK: [[BB1]]:
1861-
// CHECK: [[LIFETIME_OWNED:%[^,]+]] = move_value [lexical] [[INSTANCE]]
1861+
// CHECK: [[COPY:%[^,]+]] = copy_value [[INSTANCE]]
1862+
// CHECK: [[LIFETIME_OWNED:%[^,]+]] = move_value [lexical] [[COPY]]
18621863
// CHECK: cond_br undef, [[BB2:bb[0-9]+]], [[BB3:bb[0-9]+]]
18631864
// CHECK: [[BB2]]:
18641865
// CHECK: destroy_value [[LIFETIME_OWNED]]

0 commit comments

Comments
 (0)