Merge pull request #64637 from nate-chandler/copy-propagation/canonicalize-lexical-copies

[CopyPropagation] Canonicalize copies of owned lexical values.

Author: nate-chandler

Diff for: 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<CopyValueInst>()) {
+        ownedValues.insert(cvi);
+      }
+    }
+  }
 };
 
 } // namespace

Diff for: 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 for: 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: