Avoid creating unoptimizable copies in CSE

CSE uses OSSA rauw which creates copies and copies that are created to optimize
across borrow scopes are unoptimizable. This PR avoids this situation for now.

Author: meg-gupta

include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

@@ -279,6 +279,8 @@ class OwnershipRAUWHelper {
     return ctx->extraAddressFixupInfo.base;
   }
 
+  bool mayIntroduceUnoptimizableCopies();
+
   /// Perform OSSA fixup on newValue and return a fixed-up value based that can
   /// be used to replace all uses of oldValue.
   ///

lib/SILOptimizer/Transforms/CSE.cpp

@@ -1101,13 +1101,14 @@ bool CSE::processNode(DominanceInfoNode *Node) {
         if (!isa<SingleValueInstruction>(Inst))
           continue;
 
-        OwnershipRAUWHelper helper(RAUWFixupContext,
-                                   cast<SingleValueInstruction>(Inst),
-                                   cast<SingleValueInstruction>(AvailInst));
+        auto oldValue = cast<SingleValueInstruction>(Inst);
+        auto newValue = cast<SingleValueInstruction>(AvailInst);
+        OwnershipRAUWHelper helper(RAUWFixupContext, oldValue, newValue);
         // If RAUW requires cloning the original, then there's no point. If it
         // also requires introducing a copy and new borrow scope, then it's a
         // very bad idea.
-        if (!helper.isValid() || helper.requiresCopyBorrowAndClone())
+        if (!helper.isValid() || helper.requiresCopyBorrowAndClone() ||
+            helper.mayIntroduceUnoptimizableCopies())
           continue;
         // Replace SingleValueInstruction using OSSA RAUW here
         nextI = helper.perform();

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -439,6 +439,18 @@ static bool canFixUpOwnershipForRAUW(SILValue oldValue, SILValue newValue,
   }
 }
 
+bool OwnershipRAUWHelper::mayIntroduceUnoptimizableCopies() {
+  if (oldValue->getOwnershipKind() != OwnershipKind::Guaranteed) {
+    return false;
+  }
+
+  if (areUsesWithinValueLifetime(newValue, ctx->guaranteedUsePoints,
+                                 &ctx->deBlocks)) {
+    return false;
+  }
+  return true;
+}
+
 bool swift::areUsesWithinLexicalValueLifetime(SILValue value,
                                               ArrayRef<Operand *> uses) {
   assert(value->isLexical());

test/SILOptimizer/cse_ossa_nontrivial.sil

@@ -142,9 +142,11 @@ bb0(%0 : @owned $Klass, %1 : @owned $Klass):
   return %5 : $(Klass)
 }
 
+// mayIntroduceUnoptimizableCopies returns true because swift::areUsesWithinValueLifetime returns false
+// since findOwnershipReferenceAggregate only looks through single operand forwarding instructions
 // CHECK-LABEL: sil [ossa] @tuple_test2 :
 // CHECK: tuple ({{%[0-9]+}} : $Klass, {{%[0-9]+}} : $Klass)
-// CHECK-NOT: tuple ({{%[0-9]+}} : $Klass, {{%[0-9]+}} : $Klass)
+// TODO-CHECK-NOT: tuple ({{%[0-9]+}} : $Klass, {{%[0-9]+}} : $Klass)
 // CHECK-LABEL: } // end sil function 'tuple_test2'
 sil [ossa] @tuple_test2 : $@convention(thin) (@guaranteed Klass, @guaranteed Klass) -> @owned (Klass) {
 bb0(%0 : @guaranteed $Klass, %1 : @guaranteed $Klass):
@@ -439,7 +441,7 @@ bb0(%0 : @guaranteed $(Klass, Klass)):
 
 // CHECK-LABEL: sil [ossa] @struct_test2 :
 // CHECK: struct $NonTrivialStruct
-// CHECK-NOT: struct $NonTrivialStruct
+// TODO-CHECK-NOT: struct $NonTrivialStruct
 // CHECK-LABEL: } // end sil function 'struct_test2'
 sil [ossa] @struct_test2 : $@convention(thin) (@guaranteed Klass) -> @owned Klass {
 bb0(%0 : @guaranteed $Klass):
@@ -454,7 +456,7 @@ bb0(%0 : @guaranteed $Klass):
 
 // CHECK-LABEL: sil [ossa] @struct_test3 :
 // CHECK: struct $NonTrivialStruct
-// CHECK-NOT: struct $NonTrivialStruct
+// TODO-CHECK-NOT: struct $NonTrivialStruct
 // CHECK-LABEL: } // end sil function 'struct_test3'
 sil [ossa] @struct_test3 : $@convention(thin) (@guaranteed Klass) -> () {
 bb0(%0 : @guaranteed $Klass):
@@ -504,7 +506,7 @@ bb0(%0 : @guaranteed $Klass):
 
 // CHECK-LABEL: sil [ossa] @struct_extract_test1 :
 // CHECK:       struct_extract
-// CHECK-NOT:   struct_extract
+// TODO-CHECK-NOT:   struct_extract
 // CHECK-LABEL: } // end sil function 'struct_extract_test1'
 sil [ossa] @struct_extract_test1 : $@convention(thin) (@owned NonTrivialStruct) -> @owned Klass {
 bb0(%0 : @owned $NonTrivialStruct):
@@ -644,9 +646,9 @@ bb3(%borrow : @guaranteed $Klass):
 // CHECK-LABEL: sil [ossa] @test_rauwfailsandthensucceeds :
 // CHECK: bb0
 // CHECK: struct_extract
-// CHECK: struct_extract
+// TODO-CHECK: struct_extract
 // CHECK: struct $_SliceBuffer
-// CHECK-NOT: struct $_SliceBuffer
+// TODO-CHECK-NOT: struct $_SliceBuffer
 // CHECK-LABEL: } // end sil function 'test_rauwfailsandthensucceeds'
 sil [ossa] @test_rauwfailsandthensucceeds : $@convention(method) <Element> (UnsafeMutablePointer<Element>, @guaranteed _ContiguousArrayBuffer<Element>, Int, UInt) -> @owned _SliceBuffer<Element> {
 bb0(%0 : $UnsafeMutablePointer<Element>, %1 : @guaranteed $_ContiguousArrayBuffer<Element>, %2 : $Int, %3 : $UInt):
@@ -1076,3 +1078,51 @@ bb0(%0 : $*Wrapper):
   %18 = tuple ()
   return %18
 }
+
+sil @get_nontrivialstruct : $@convention(thin) () -> @owned NonTrivialStruct
+
+// Both CopyPropagation and CopyToBorrow cannot eliminate the copy generated by
+// OSSA RAUW if we look through ownership instructions while CSE'ing
+// CHECK-LABEL: sil [ossa] @struct_copy_test1 :
+// CHECK: struct_extract
+// CHECK: struct_extract
+// CHECK-LABEL: } // end sil function 'struct_copy_test1'
+sil [ossa] @struct_copy_test1 : $@convention(thin) (@owned NonTrivialStruct) -> () {
+bb0(%0 : @owned $NonTrivialStruct):
+  %f = function_ref @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
+  %b1 = begin_borrow %0
+  %ex1 = struct_extract %b1, #NonTrivialStruct.val
+  apply %f(%ex1) : $@convention(thin) (@guaranteed Klass) -> ()
+  end_borrow %b1
+  %b2 = begin_borrow %0
+  %ex2 = struct_extract %b2, #NonTrivialStruct.val
+  apply %f(%ex2) : $@convention(thin) (@guaranteed Klass) -> ()
+  end_borrow %b2
+  destroy_value %0 : $NonTrivialStruct
+  %res = tuple ()
+  return %res : $()
+}
+
+// Both CopyPropagation and CopyToBorrow cannot eliminate the copy generated by
+// OSSA RAUW if we look through ownership instructions while CSE'ing
+// CHECK-LABEL: sil [ossa] @struct_copy_test2 :
+// CHECK: struct_extract
+// CHECK: struct_extract
+// CHECK-LABEL: } // end sil function 'struct_copy_test2'
+sil [ossa] @struct_copy_test2 : $@convention(thin) () -> () {
+bb0:
+  %f1 = function_ref @get_nontrivialstruct : $@convention(thin) () -> @owned NonTrivialStruct
+  %0 = apply %f1() : $@convention(thin) () -> @owned NonTrivialStruct
+  %f2 = function_ref @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
+  %b1 = begin_borrow %0
+  %ex1 = struct_extract %b1, #NonTrivialStruct.val
+  apply %f2(%ex1) : $@convention(thin) (@guaranteed Klass) -> ()
+  end_borrow %b1
+  %b2 = begin_borrow %0
+  %ex2 = struct_extract %b2, #NonTrivialStruct.val
+  apply %f2(%ex2) : $@convention(thin) (@guaranteed Klass) -> ()
+  end_borrow %b2
+  destroy_value %0 : $NonTrivialStruct
+  %res = tuple ()
+  return %res : $()
+}