Merge pull request swiftlang#78581 from meg-gupta/outlinerfix

Outliner: Do not outline if the BridgedArg value is not available at the BridgedCall

Author: meg-gupta

include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

@@ -309,6 +309,11 @@ class OwnershipRAUWHelper {
 /// specified lexical value.
 bool areUsesWithinLexicalValueLifetime(SILValue, ArrayRef<Operand *>);
 
+/// Whether the provided uses lie within the current liveness of the
+/// specified value.
+bool areUsesWithinValueLifetime(SILValue value, ArrayRef<Operand *> uses,
+                                DeadEndBlocks *deBlocks);
+
 /// A utility composed ontop of OwnershipFixupContext that knows how to replace
 /// a single use of a value with another value with a different ownership. We
 /// allow for the values to have different types.

lib/SILOptimizer/Transforms/Outliner.cpp

@@ -757,7 +757,8 @@ class BridgedArgument {
       : BridgeFun(nullptr), BridgeCall(nullptr), OptionalResult(nullptr),
         ReleaseAfterBridge(nullptr), ReleaseArgAfterCall(nullptr), Idx(0) {}
 
-  static BridgedArgument match(unsigned ArgIdx, SILValue Arg, ApplyInst *AI);
+  static BridgedArgument match(unsigned ArgIdx, SILValue Arg, ApplyInst *AI,
+                               DeadEndBlocks *deBlocks);
 
   operator bool() const { return BridgeFun != nullptr; }
   SILValue bridgedValue() { return BridgedValue; }
@@ -826,7 +827,7 @@ static SILInstruction *findReleaseOf(SILValue releasedValue,
 }
 
 BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
-                                       ApplyInst *AI) {
+                                       ApplyInst *AI, DeadEndBlocks *deBlocks) {
   // Match
   // %15 = function_ref @$SSS10FoundationE19_bridgeToObjectiveCSo8NSStringCyF
   // %16 = apply %15(%14) : $@convention(method) (@guaranteed String) -> @owned NSString
@@ -879,6 +880,7 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
 
   if (SILBasicBlock::iterator(BridgeCall) == BridgeCall->getParent()->begin())
     return BridgedArgument();
+
   auto *FunRef =
       dyn_cast<FunctionRefInst>(std::prev(SILBasicBlock::iterator(BridgeCall)));
   if (!FunRef || !FunRef->hasOneUse() || BridgeCall->getCallee() != FunRef)
@@ -912,6 +914,12 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
       BridgeFun->getName() != bridgeWitness.mangle())
     return BridgedArgument();
 
+  if (hasOwnership && !BridgedValueRelease) {
+    SmallVector<Operand *> newUses{&AI->getOperandRef(ArgIdx)};
+    if (!areUsesWithinValueLifetime(BridgedValue, newUses, deBlocks)) {
+      return BridgedArgument();
+    }
+  }
   return BridgedArgument(ArgIdx, FunRef, BridgeCall, Enum, BridgedValueRelease,
                          ReleaseAfter);
 }
@@ -1087,10 +1095,6 @@ ObjCMethodCall::outline(SILModule &M) {
       if (BridgedArgIdx < BridgedArguments.size() &&
           BridgedArguments[BridgedArgIdx].Idx == OrigSigIdx) {
         auto bridgedArgValue = BridgedArguments[BridgedArgIdx].bridgedValue();
-        if (bridgedArgValue->getOwnershipKind() == OwnershipKind::Guaranteed) {
-          bridgedArgValue = makeGuaranteedValueAvailable(
-              bridgedArgValue, BridgedCall, *deBlocks);
-        }
         Args.push_back(bridgedArgValue);
         ++BridgedArgIdx;
       } else {
@@ -1215,7 +1219,8 @@ bool ObjCMethodCall::matchInstSequence(SILBasicBlock::iterator I) {
     if (Ty.isAnyObject())
       continue;
 
-    auto BridgedArg = BridgedArgument::match(CurIdx, Param.get(), BridgedCall);
+    auto BridgedArg =
+        BridgedArgument::match(CurIdx, Param.get(), BridgedCall, deBlocks);
     if (!BridgedArg)
       continue;
 

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -456,6 +456,30 @@ bool swift::areUsesWithinLexicalValueLifetime(SILValue value,
   return false;
 }
 
+bool swift::areUsesWithinValueLifetime(SILValue value, ArrayRef<Operand *> uses,
+                                       DeadEndBlocks *deBlocks) {
+  assert(value->getFunction()->hasOwnership());
+
+  if (value->getOwnershipKind() == OwnershipKind::None) {
+    return true;
+  }
+  if (value->getOwnershipKind() != OwnershipKind::Guaranteed &&
+      value->getOwnershipKind() != OwnershipKind::Owned) {
+    return false;
+  }
+  if (value->getOwnershipKind() == OwnershipKind::Guaranteed) {
+    value = findOwnershipReferenceAggregate(value);
+    BorrowedValue borrowedValue(value);
+    if (!borrowedValue.isLocalScope()) {
+      return true;
+    }
+  }
+  SSAPrunedLiveness liveness(value->getFunction());
+  liveness.initializeDef(value);
+  liveness.computeSimple();
+  return liveness.areUsesWithinBoundary(uses, deBlocks);
+}
+
 //===----------------------------------------------------------------------===//
 //                          BorrowedLifetimeExtender
 //===----------------------------------------------------------------------===//

test/SILOptimizer/outliner.swift

@@ -1,6 +1,8 @@
 // RUN: %target-swift-frontend -Osize -import-objc-header %S/Inputs/Outliner.h %s -Xllvm -sil-print-types -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation | %FileCheck %s
 // RUN: %target-swift-frontend -Osize -g -import-objc-header %S/Inputs/Outliner.h %s -Xllvm -sil-print-types -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation | %FileCheck %s
 
+// RUN: %target-swift-frontend -Osize -import-objc-header %S/Inputs/Outliner.h %s -Xllvm -sil-print-types -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation -enable-ossa-modules | %FileCheck %s
+// RUN: %target-swift-frontend -Osize -g -import-objc-header %S/Inputs/Outliner.h %s -Xllvm -sil-print-types -emit-sil -enforce-exclusivity=unchecked -enable-copy-propagation -enable-ossa-modules | %FileCheck %s
 // REQUIRES: objc_interop
 // REQUIRES: optimized_stdlib
 // REQUIRES: swift_in_compiler

test/SILOptimizer/outliner_ossa.sil

@@ -74,9 +74,9 @@ bb7(%64 : @owned $Optional<Data>):
   return %102 : $()
 }
 
+// Not optimized
 // CHECK-LABEL: sil [Osize] [ossa] @test2 :
-// CHECK:  [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
-// CHECK:  apply [[FUNC]]
+// CHECK: objc_method
 // CHECK-LABEL: } // end sil function 'test2'
 sil [Osize] [ossa] @test2 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
 bb0(%0: @owned $MyObject):
@@ -96,9 +96,9 @@ bb0(%0: @owned $MyObject):
   return %51 : $Optional<NSData>
 }
 
+// Not optimized
 // CHECK-LABEL: sil [Osize] [ossa] @test3 :
-// CHECK:  [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
-// CHECK:  apply [[FUNC]]
+// CHECK: objc_method
 // CHECK-LABEL: } // end sil function 'test3'
 sil [Osize] [ossa] @test3 : $@convention(thin) (@owned MyObject, @in Data) -> @owned Optional<NSData> {
 bb0(%0: @owned $MyObject, %1 : $*Data):
@@ -116,9 +116,9 @@ bb0(%0: @owned $MyObject, %1 : $*Data):
   return %51 : $Optional<NSData>
 }
 
+// Not optimized
 // CHECK-LABEL: sil [Osize] [ossa] @test4 : 
-// CHECK:  [[FUNC:%.*]] = function_ref @$s4main8MyObjectC8take_two4arg14arg210Foundation4DataVSgAJ_AJtFZToTembgbgnn_ :
-// CHECK:  apply [[FUNC]]
+// CHECK: objc_method
 // CHECK-LABEL: } // end sil function 'test4'
 sil [Osize] [ossa] @test4 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
 bb0(%0: @owned $MyObject):
@@ -177,9 +177,9 @@ bb3:
   return %51 : $Optional<NSData>
 }
 
+// Not optimized
 // CHECK-LABEL: sil [Osize] [ossa] @test6 :
-// CHECK:  [[FUNC:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembgnn_ :
-// CHECK:  apply [[FUNC]]
+// CHECK:  objc_method
 // CHECK-LABEL: } // end sil function 'test6'
 sil [Osize] [ossa] @test6 : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
 bb0(%0: @owned $MyObject):
@@ -305,3 +305,23 @@ sil [Osize] [ossa] @destroy_after_borrow : $@convention(thin) () -> @owned Sub {
   apply undef(%162) : $@convention(thin) (Int) -> ()
   return %107 : $Sub
 }
+
+// CHECK-LABEL: sil [Osize] [ossa] @test_consume :
+// CHECK: objc_method 
+// CHECK-LABEL: } // end sil function 'test_consume'
+sil [Osize] [ossa] @test_consume : $@convention(thin) (@owned MyObject) -> @owned Optional<NSData> {
+bb0(%0: @owned $MyObject):
+  %35 = metatype $@objc_metatype MyObject.Type
+  %41 = function_ref @getData : $@convention(thin) () -> @owned Data
+  %43 = apply %41() : $@convention(thin) () -> @owned Data
+  %44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
+  %45 = apply %44(%43) : $@convention(method) (@guaranteed Data) -> @owned NSData
+  %46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
+  destroy_value %0 : $MyObject
+  %48 = apply undef(%43) : $@convention(method) (@owned Data) -> ()
+  %50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take!foreign : (MyObject.Type) -> (Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
+  %51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
+  destroy_value %46 : $Optional<NSData>
+  return %51 : $Optional<NSData>
+}
+