Merge pull request #79493 from meg-gupta/csesave

Avoid creating unoptimizable copies in CSE

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 : $()
+}