From 4f0d1dab5e76dbcbc1e69ef4401c2b652b6a0f6e Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Sat, 25 Mar 2023 12:21:30 -0700 Subject: [PATCH] [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 --- .../Transforms/CopyPropagation.cpp | 22 ++++++++- test/SILOptimizer/copy_propagation.sil | 45 +++++++++++++++---- test/SILOptimizer/copy_propagation_borrow.sil | 9 +--- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/lib/SILOptimizer/Transforms/CopyPropagation.cpp b/lib/SILOptimizer/Transforms/CopyPropagation.cpp index 0be6c33217e62..6e8cb7977c617 100644 --- a/lib/SILOptimizer/Transforms/CopyPropagation.cpp +++ b/lib/SILOptimizer/Transforms/CopyPropagation.cpp @@ -115,7 +115,7 @@ struct CanonicalDefWorklist { } } if (!canonicalizeBorrows) { - ownedValues.insert(def); + recordOwnedValue(def); return; } // Look through hoistable owned forwarding instructions on the @@ -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; } } @@ -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()) { + ownedValues.insert(cvi); + } + } + } }; } // namespace diff --git a/test/SILOptimizer/copy_propagation.sil b/test/SILOptimizer/copy_propagation.sil index 9473c891005ee..59a2a2667eecd 100644 --- a/test/SILOptimizer/copy_propagation.sil +++ b/test/SILOptimizer/copy_propagation.sil @@ -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 @@ -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 @@ -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 : $() +} diff --git a/test/SILOptimizer/copy_propagation_borrow.sil b/test/SILOptimizer/copy_propagation_borrow.sil index 8ecd7ba031e5a..be3a1a7b47b0c 100644 --- a/test/SILOptimizer/copy_propagation_borrow.sil +++ b/test/SILOptimizer/copy_propagation_borrow.sil @@ -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) -> () { @@ -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: