Skip to content

Commit 4f0d1da

Browse files
committed
[CopyPropagation] Canonicalize copies of lexical.
Currently, CopyPropagation only canonicalizes defs that are "canonical", that is, the root of the copy_value tree. When that canonical def is lexical, however, the canonicalization respects deinit barriers. But copies of lexical values are not themselves lexical, so their lifetimes can be shortened without respect to deinit barriers. Here, immediate copies of lexical values are canonicalized before the lexical values themselves are. rdar://107197935
1 parent c0d7a82 commit 4f0d1da

File tree

3 files changed

+58
-18
lines changed

3 files changed

+58
-18
lines changed

lib/SILOptimizer/Transforms/CopyPropagation.cpp

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ struct CanonicalDefWorklist {
115115
}
116116
}
117117
if (!canonicalizeBorrows) {
118-
ownedValues.insert(def);
118+
recordOwnedValue(def);
119119
return;
120120
}
121121
// Look through hoistable owned forwarding instructions on the
@@ -134,7 +134,7 @@ struct CanonicalDefWorklist {
134134
// Add any forwarding uses of this owned def. This may include uses that
135135
// we looked through above, but may also include other uses.
136136
addForwardingUses(def);
137-
ownedValues.insert(def);
137+
recordOwnedValue(def);
138138
return;
139139
}
140140
}
@@ -168,6 +168,24 @@ struct CanonicalDefWorklist {
168168
}
169169
ownedForwards.remove(i);
170170
}
171+
172+
private:
173+
void recordOwnedValue(SILValue def) {
174+
ownedValues.insert(def);
175+
// Direct copies of owned lexical values are not themselves lexical and
176+
// consequently need to be canonicalized separately because the
177+
// canonicalization of the canonical def will respect deinit barriers
178+
// but canonicalization of the copies should not.
179+
//
180+
// Add these copies to the worklist _after_ the canonical def because the
181+
// worklist is drained backwards and canonicalizing the copies first
182+
// enables the canonical lexical defs to be further canonicalized.
183+
if (def->isLexical()) {
184+
for (auto *cvi : def->getUsersOfType<CopyValueInst>()) {
185+
ownedValues.insert(cvi);
186+
}
187+
}
188+
}
171189
};
172190

173191
} // namespace

test/SILOptimizer/copy_propagation.sil

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -305,13 +305,15 @@ bb0(%0 : @owned $B):
305305

306306
// FIXME: mark_dependence is currently a PointerEscape, so dependent live ranges are not canonicalized.
307307
//
308-
// CHECK-LABEL: sil [ossa] @testMarkDependence : $@convention(thin) (@inout Builtin.Int64, @owned B) -> Builtin.Int64 {
308+
// CHECK-LABEL: sil [ossa] @testMarkDependence : {{.*}} {
309309
// CHECK: copy_value
310310
// CHECK: destroy_value
311311
// CHECK: destroy_value
312312
// CHECK-LABEL: } // end sil function 'testMarkDependence'
313-
sil [ossa] @testMarkDependence : $@convention(thin) (@inout Builtin.Int64, @owned B) -> Builtin.Int64 {
314-
bb0(%0 : $*Builtin.Int64, %1 : @owned $B):
313+
sil [ossa] @testMarkDependence : $@convention(thin) (@inout Builtin.Int64) -> Builtin.Int64 {
314+
bb0(%0 : $*Builtin.Int64):
315+
%getOwnedB = function_ref @getOwnedB : $@convention(thin) () -> (@owned B)
316+
%1 = apply %getOwnedB() : $@convention(thin) () -> (@owned B)
315317
%ptr = mark_dependence %0 : $*Builtin.Int64 on %1 : $B
316318
%val = load [trivial] %ptr : $*Builtin.Int64
317319
%copy = copy_value %1 : $B
@@ -831,16 +833,18 @@ bb4:
831833
// Test a dead begin_borrow (with no scope ending uses). Make sure
832834
// copy-propagation doesn't end the lifetime before the dead borrow.
833835
//
834-
// CHECK-LABEL: sil hidden [ossa] @testDeadBorrow : $@convention(thin) (@owned C) -> () {
835-
// CHECK: bb0(%0 : @owned $C):
836-
// CHECK: copy_value %0 : $C
836+
// CHECK-LABEL: sil hidden [ossa] @testDeadBorrow : {{.*}} {
837+
// CHECK: bb0:
838+
// CHECK: copy_value %1 : $C
837839
// CHECK: destroy_value
838-
// CHECK: copy_value %0 : $C
840+
// CHECK: copy_value %1 : $C
839841
// CHECK: begin_borrow
840842
// CHECK: unreachable
841843
// CHECK-LABEL: } // end sil function 'testDeadBorrow'
842-
sil hidden [ossa] @testDeadBorrow : $@convention(thin) (@owned C) -> () {
843-
bb0(%0 : @owned $C):
844+
sil hidden [ossa] @testDeadBorrow : $@convention(thin) () -> () {
845+
bb0:
846+
%getOwnedC = function_ref @getOwnedC : $@convention(thin) () -> (@owned C)
847+
%0 = apply %getOwnedC() : $@convention(thin) () -> (@owned C)
844848
%1 = copy_value %0 : $C
845849
destroy_value %1 : $C
846850
%6 = copy_value %0 : $C
@@ -989,3 +993,26 @@ bb0:
989993
%99 = tuple ()
990994
return %99 : $()
991995
}
996+
997+
// CHECK-LABEL: sil [ossa] @hoist_destroy_of_copy_of_lexical_over_deinit_barrier : $@convention(thin) (@owned C) -> () {
998+
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned
999+
// CHECK: [[BARRIER:%[^,]+]] = function_ref @barrier
1000+
// CHECK: [[BORROW:%[^,]+]] = function_ref @takeGuaranteedC
1001+
// CHECK: apply [[BORROW]]([[INSTANCE]])
1002+
// The destroy of the copy should be hoisted over the deinit barrier, and then
1003+
// canonicalization of the lexical value should remove the copy.
1004+
// CHECK: destroy_value [[INSTANCE]]
1005+
// CHECK: apply [[BARRIER]]()
1006+
// CHECK-LABEL: } // end sil function 'hoist_destroy_of_copy_of_lexical_over_deinit_barrier'
1007+
sil [ossa] @hoist_destroy_of_copy_of_lexical_over_deinit_barrier : $(@owned C) -> () {
1008+
entry(%instance : @owned $C):
1009+
%barrier = function_ref @barrier : $@convention(thin) () -> ()
1010+
%borrow = function_ref @takeGuaranteedC : $@convention(thin) (@guaranteed C) -> ()
1011+
%copy = copy_value %instance : $C
1012+
apply %borrow(%instance) : $@convention(thin) (@guaranteed C) -> ()
1013+
destroy_value %instance : $C
1014+
apply %barrier() : $@convention(thin) () -> ()
1015+
destroy_value %copy : $C
1016+
%retval = tuple ()
1017+
return %retval : $()
1018+
}

test/SILOptimizer/copy_propagation_borrow.sil

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,13 +241,12 @@ bb3:
241241
// CHECK-NOT: copy_value
242242
// CHECK: cond_br undef, bb1, bb2
243243
// CHECK: bb1:
244-
// CHECK-NEXT: [[COPY:%.*]] = copy_value [[OUTERCOPY]] : $C
245-
// CHECK-NEXT: apply %{{.*}}([[COPY]]) : $@convention(thin) (@owned C) -> ()
244+
// CHECK-NEXT: apply %{{.*}}([[OUTERCOPY]]) : $@convention(thin) (@owned C) -> ()
246245
// CHECK-NEXT: br bb3
247246
// CHECK: bb2:
247+
// CHECK-NEXT: destroy_value [[OUTERCOPY]] : $C
248248
// CHECK-NEXT: br bb3
249249
// CHECK: bb3:
250-
// CHECK-NEXT: destroy_value [[OUTERCOPY]] : $C
251250
// CHECK-NEXT: destroy_value %0 : $C
252251
// CHECK-LABEL: } // end sil function 'testLocalBorrowPostDomDestroy'
253252
sil [ossa] @testLocalBorrowPostDomDestroy : $@convention(thin) (@owned C) -> () {
@@ -328,10 +327,6 @@ bb3:
328327
// CHECK-NEXT: br bb3
329328
// CHECK: bb2:
330329
// CHECK-NEXT: apply %{{.*}}([[OUTERCOPY]]) : $@convention(thin) (@guaranteed C) -> ()
331-
//
332-
// This copy would be eliminated if the outer lifetime were also canonicalized (no unchecked_ownership_conversion)
333-
// CHECK-NEXT: [[COPY2:%.*]] = copy_value [[OUTERCOPY]] : $C
334-
// CHECK-NEXT: destroy_value [[COPY2]] : $C
335330
// CHECK-NEXT: destroy_value %4 : $C
336331
// CHECK-NEXT: br bb3
337332
// CHECK: bb3:

0 commit comments

Comments
 (0)