Skip to content

[CopyPropagation] Canonicalize copies of owned lexical values. #64637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 27, 2023
Merged

[CopyPropagation] Canonicalize copies of owned lexical values. #64637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions lib/SILOptimizer/Transforms/CopyPropagation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ struct CanonicalDefWorklist {
}
}
if (!canonicalizeBorrows) {
ownedValues.insert(def);
recordOwnedValue(def);
return;
}
// Look through hoistable owned forwarding instructions on the
Expand All @@ -134,7 +134,7 @@ struct CanonicalDefWorklist {
// Add any forwarding uses of this owned def. This may include uses that
// we looked through above, but may also include other uses.
addForwardingUses(def);
ownedValues.insert(def);
recordOwnedValue(def);
return;
}
}
Expand Down Expand Up @@ -168,6 +168,24 @@ struct CanonicalDefWorklist {
}
ownedForwards.remove(i);
}

private:
void recordOwnedValue(SILValue def) {
ownedValues.insert(def);
// Direct copies of owned lexical values are not themselves lexical and
// consequently need to be canonicalized separately because the
// canonicalization of the canonical def will respect deinit barriers
// but canonicalization of the copies should not.
//
// Add these copies to the worklist _after_ the canonical def because the
// worklist is drained backwards and canonicalizing the copies first
// enables the canonical lexical defs to be further canonicalized.
if (def->isLexical()) {
for (auto *cvi : def->getUsersOfType<CopyValueInst>()) {
ownedValues.insert(cvi);
}
}
}
};

} // namespace
Expand Down
45 changes: 36 additions & 9 deletions test/SILOptimizer/copy_propagation.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,15 @@ bb0(%0 : @owned $B):

// FIXME: mark_dependence is currently a PointerEscape, so dependent live ranges are not canonicalized.
//
// CHECK-LABEL: sil [ossa] @testMarkDependence : $@convention(thin) (@inout Builtin.Int64, @owned B) -> Builtin.Int64 {
// CHECK-LABEL: sil [ossa] @testMarkDependence : {{.*}} {
// CHECK: copy_value
// CHECK: destroy_value
// CHECK: destroy_value
// CHECK-LABEL: } // end sil function 'testMarkDependence'
sil [ossa] @testMarkDependence : $@convention(thin) (@inout Builtin.Int64, @owned B) -> Builtin.Int64 {
bb0(%0 : $*Builtin.Int64, %1 : @owned $B):
sil [ossa] @testMarkDependence : $@convention(thin) (@inout Builtin.Int64) -> Builtin.Int64 {
bb0(%0 : $*Builtin.Int64):
%getOwnedB = function_ref @getOwnedB : $@convention(thin) () -> (@owned B)
%1 = apply %getOwnedB() : $@convention(thin) () -> (@owned B)
%ptr = mark_dependence %0 : $*Builtin.Int64 on %1 : $B
%val = load [trivial] %ptr : $*Builtin.Int64
%copy = copy_value %1 : $B
Expand Down Expand Up @@ -831,16 +833,18 @@ bb4:
// Test a dead begin_borrow (with no scope ending uses). Make sure
// copy-propagation doesn't end the lifetime before the dead borrow.
//
// CHECK-LABEL: sil hidden [ossa] @testDeadBorrow : $@convention(thin) (@owned C) -> () {
// CHECK: bb0(%0 : @owned $C):
// CHECK: copy_value %0 : $C
// CHECK-LABEL: sil hidden [ossa] @testDeadBorrow : {{.*}} {
// CHECK: bb0:
// CHECK: copy_value %1 : $C
// CHECK: destroy_value
// CHECK: copy_value %0 : $C
// CHECK: copy_value %1 : $C
// CHECK: begin_borrow
// CHECK: unreachable
// CHECK-LABEL: } // end sil function 'testDeadBorrow'
sil hidden [ossa] @testDeadBorrow : $@convention(thin) (@owned C) -> () {
bb0(%0 : @owned $C):
sil hidden [ossa] @testDeadBorrow : $@convention(thin) () -> () {
bb0:
%getOwnedC = function_ref @getOwnedC : $@convention(thin) () -> (@owned C)
%0 = apply %getOwnedC() : $@convention(thin) () -> (@owned C)
%1 = copy_value %0 : $C
destroy_value %1 : $C
%6 = copy_value %0 : $C
Expand Down Expand Up @@ -989,3 +993,26 @@ bb0:
%99 = tuple ()
return %99 : $()
}

// CHECK-LABEL: sil [ossa] @hoist_destroy_of_copy_of_lexical_over_deinit_barrier : $@convention(thin) (@owned C) -> () {
// CHECK: {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned
// CHECK: [[BARRIER:%[^,]+]] = function_ref @barrier
// CHECK: [[BORROW:%[^,]+]] = function_ref @takeGuaranteedC
// CHECK: apply [[BORROW]]([[INSTANCE]])
// The destroy of the copy should be hoisted over the deinit barrier, and then
// canonicalization of the lexical value should remove the copy.
// CHECK: destroy_value [[INSTANCE]]
// CHECK: apply [[BARRIER]]()
// CHECK-LABEL: } // end sil function 'hoist_destroy_of_copy_of_lexical_over_deinit_barrier'
sil [ossa] @hoist_destroy_of_copy_of_lexical_over_deinit_barrier : $(@owned C) -> () {
entry(%instance : @owned $C):
%barrier = function_ref @barrier : $@convention(thin) () -> ()
%borrow = function_ref @takeGuaranteedC : $@convention(thin) (@guaranteed C) -> ()
%copy = copy_value %instance : $C
apply %borrow(%instance) : $@convention(thin) (@guaranteed C) -> ()
destroy_value %instance : $C
apply %barrier() : $@convention(thin) () -> ()
destroy_value %copy : $C
%retval = tuple ()
return %retval : $()
}
9 changes: 2 additions & 7 deletions test/SILOptimizer/copy_propagation_borrow.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,12 @@ bb3:
// CHECK-NOT: copy_value
// CHECK: cond_br undef, bb1, bb2
// CHECK: bb1:
// CHECK-NEXT: [[COPY:%.*]] = copy_value [[OUTERCOPY]] : $C
// CHECK-NEXT: apply %{{.*}}([[COPY]]) : $@convention(thin) (@owned C) -> ()
// CHECK-NEXT: apply %{{.*}}([[OUTERCOPY]]) : $@convention(thin) (@owned C) -> ()
// CHECK-NEXT: br bb3
// CHECK: bb2:
// CHECK-NEXT: destroy_value [[OUTERCOPY]] : $C
// CHECK-NEXT: br bb3
// CHECK: bb3:
// CHECK-NEXT: destroy_value [[OUTERCOPY]] : $C
// CHECK-NEXT: destroy_value %0 : $C
// CHECK-LABEL: } // end sil function 'testLocalBorrowPostDomDestroy'
sil [ossa] @testLocalBorrowPostDomDestroy : $@convention(thin) (@owned C) -> () {
Expand Down Expand Up @@ -328,10 +327,6 @@ bb3:
// CHECK-NEXT: br bb3
// CHECK: bb2:
// CHECK-NEXT: apply %{{.*}}([[OUTERCOPY]]) : $@convention(thin) (@guaranteed C) -> ()
//
// This copy would be eliminated if the outer lifetime were also canonicalized (no unchecked_ownership_conversion)
// CHECK-NEXT: [[COPY2:%.*]] = copy_value [[OUTERCOPY]] : $C
// CHECK-NEXT: destroy_value [[COPY2]] : $C
// CHECK-NEXT: destroy_value %4 : $C
// CHECK-NEXT: br bb3
// CHECK: bb3:
Expand Down