Merge pull request #68180 from meg-gupta/lowerb4inline

Lower OwnershipModelEliminator to just before inlining

Author: meg-gupta

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ObjectOutliner.swift

@@ -298,10 +298,7 @@ private func isValidUseOfObject(_ use: Operand) -> Bool {
        is UpcastInst,
        is BeginDeallocRefInst,
        is RefTailAddrInst,
-       is RefElementAddrInst,
-       is StructInst,
-       is PointerToAddressInst,
-       is IndexAddrInst:
+       is RefElementAddrInst:
     for instUse in (inst as! SingleValueInstruction).uses {
       if !isValidUseOfObject(instUse) {
         return false

SwiftCompilerSources/Sources/Optimizer/ModulePasses/MandatoryPerformanceOptimizations.swift

@@ -167,11 +167,6 @@ private func shouldInline(apply: FullApplySite, callee: Function, alreadyInlined
     return true
   }
 
-  if apply.parentFunction.hasOwnership && !callee.hasOwnership {
-    // Cannot inline a non-ossa function into an ossa function
-    return false
-  }
-
   if apply is BeginApplyInst {
     // Avoid co-routines because they might allocate (their context).
     return true

SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift

@@ -510,6 +510,14 @@ extension FullApplySite {
        !calleeFunction.isSerialized {
       return false
     }
+
+    // Cannot inline a non-ossa function into an ossa function
+    if parentFunction.hasOwnership,
+      let calleeFunction = referencedFunction,
+      !calleeFunction.hasOwnership {
+      return false
+    }
+
     return true
   }
 

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -61,12 +61,6 @@ static llvm::cl::opt<bool> SILViewSILGenCFG(
     "sil-view-silgen-cfg", llvm::cl::init(false),
     llvm::cl::desc("Enable the sil cfg viewer pass before diagnostics"));
 
-
-llvm::cl::opt<bool> SILDisableLateOMEByDefault(
-    "sil-disable-late-ome-by-default", llvm::cl::init(false),
-    llvm::cl::desc(
-        "Disable late OME for non-transparent functions by default"));
-
 llvm::cl::opt<bool>
     EnableDestroyHoisting("enable-destroy-hoisting", llvm::cl::init(false),
                           llvm::cl::desc("Enable the DestroyHoisting pass."));
@@ -452,16 +446,6 @@ void addFunctionPasses(SILPassPipelinePlan &P,
   // Cleanup, which is important if the inliner has restarted the pass pipeline.
   P.addPerformanceConstantPropagation();
 
-  if (!P.getOptions().EnableOSSAModules && !SILDisableLateOMEByDefault) {
-    if (P.getOptions().StopOptimizationBeforeLoweringOwnership)
-      return;
-
-    if (SILPrintFinalOSSAModule) {
-      addModulePrinterPipeline(P, "SIL Print Final OSSA Module");
-    }
-    P.addNonTransparentFunctionOwnershipModelEliminator();
-  }
-
   addSimplifyCFGSILCombinePasses(P);
 
   P.addArrayElementPropagation();
@@ -506,13 +490,21 @@ void addFunctionPasses(SILPassPipelinePlan &P,
   }
   P.addARCSequenceOpts();
 
-  if (P.getOptions().EnableOSSAModules) {
-    // We earlier eliminated ownership if we are not compiling the stdlib. Now
-    // handle the stdlib functions, re-simplifying, eliminating ARC as we do.
-    if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
-      P.addCopyPropagation();
+  // We earlier eliminated ownership if we are not compiling the stdlib. Now
+  // handle the stdlib functions, re-simplifying, eliminating ARC as we do.
+  if (P.getOptions().CopyPropagation != CopyPropagationOption::Off) {
+    P.addCopyPropagation();
+  }
+  P.addSemanticARCOpts();
+
+  if (!P.getOptions().EnableOSSAModules) {
+    if (P.getOptions().StopOptimizationBeforeLoweringOwnership)
+      return;
+
+    if (SILPrintFinalOSSAModule) {
+      addModulePrinterPipeline(P, "SIL Print Final OSSA Module");
     }
-    P.addSemanticARCOpts();
+    P.addNonTransparentFunctionOwnershipModelEliminator();
   }
 
   switch (OpLevel) {
@@ -641,16 +633,6 @@ static void addPerfEarlyModulePassPipeline(SILPassPipelinePlan &P) {
   // not blocked by any other passes' optimizations, so do it early.
   P.addDifferentiabilityWitnessDevirtualizer();
 
-  if (!P.getOptions().EnableOSSAModules && SILDisableLateOMEByDefault) {
-    if (P.getOptions().StopOptimizationBeforeLoweringOwnership)
-      return;
-
-    if (SILPrintFinalOSSAModule) {
-      addModulePrinterPipeline(P, "SIL Print Final OSSA Module");
-    }
-    P.addNonTransparentFunctionOwnershipModelEliminator();
-  }
-
   // Start by linking in referenced functions from other modules.
   P.addPerformanceSILLinker();
 

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -2319,6 +2319,8 @@ bool SimplifyCFG::simplifyUnreachableBlock(UnreachableInst *UI) {
     case SILInstructionKind::StrongReleaseInst:
     case SILInstructionKind::RetainValueInst:
     case SILInstructionKind::ReleaseValueInst:
+    case SILInstructionKind::DestroyValueInst:
+    case SILInstructionKind::EndBorrowInst:
       break;
     // We can only ignore a dealloc_stack instruction if we can ignore all
     // instructions in the block.

lib/SILOptimizer/Utils/InstructionDeleter.cpp

@@ -62,18 +62,28 @@ static bool isScopeAffectingInstructionDead(SILInstruction *inst,
     return false;
   }
 
-  // If inst has any owned move-only value as a result, deleting it may shorten
-  // that value's lifetime which is illegal according to language rules.
-  //
-  // In particular, this check is needed before returning true when
-  // getSingleValueCopyOrCast returns true.  That function returns true for
-  // move_value instructions.  And `move_value %moveOnlyValue` must not be
-  // deleted.
   for (auto result : inst->getResults()) {
+    // If inst has any owned move-only value as a result, deleting it may
+    // shorten that value's lifetime which is illegal according to language
+    // rules.
+    //
+    // In particular, this check is needed before returning true when
+    // getSingleValueCopyOrCast returns true.  That function returns true for
+    // move_value instructions.  And `move_value %moveOnlyValue` must not be
+    // deleted.
     if (result->getType().getASTType()->isNoncopyable() &&
         result->getOwnershipKind() == OwnershipKind::Owned) {
       return false;
     }
+
+    // If result was lexical, lifetime shortening maybe observed, return.
+    if (result->isLexical()) {
+      auto resultTy = result->getType().getAs<SILFunctionType>();
+      // Allow deleted dead lexical values when they are trivial no escape types.
+      if (!resultTy || !resultTy->isTrivialNoEscape()) {
+        return false;
+      }
+    }
   }
 
   // If inst is a copy or beginning of scope, inst is dead, since we know that

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -386,6 +386,11 @@ bool OwnershipRAUWHelper::hasValidRAUWOwnership(SILValue oldValue,
   if (m->getStage() == SILStage::Raw)
     return false;
 
+  // OSSA rauw can create copies. Bail out if we have move only values.
+  if (newValue->getType().isMoveOnly()) {
+    return false;
+  }
+
   return true;
 }
 

test/Interpreter/moveonly_linkedlist.swift

@@ -1,5 +1,6 @@
 // RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all)
 // RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all)
+// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all -Xfrontend -enable-ossa-modules)
 
 // REQUIRES: executable_test
 

test/SILOptimizer/consuming_parameter.swift

@@ -18,12 +18,11 @@ public func async_dead_arg_call(o: consuming AnyObject) async {
 
 // CHECK-LABEL: sil [ossa] @async_dead_arg_call_lexical : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @noImplicitCopy @_lexical @owned
-// CHECK:         [[MOVE:%[^,]+]] = move_value [lexical] [[INSTANCE]]
 // CHECK:         [[EXECUTOR:%[^,]+]] = enum $Optional<Builtin.Executor>, #Optional.none!enumelt 
 // CHECK:         [[CALLEE:%[^,]+]] = function_ref @async_callee
 // CHECK:         apply [[CALLEE]]()
 // CHECK:         hop_to_executor [[EXECUTOR]]
-// CHECK:         destroy_value [[MOVE]]
+// CHECK:         destroy_value [[INSTANCE]]
 // CHECK-LABEL: } // end sil function 'async_dead_arg_call_lexical'
 @_silgen_name("async_dead_arg_call_lexical")
 public func async_dead_arg_call_lexical(@_noEagerMove o: consuming AnyObject) async {

test/SILOptimizer/pre_specialize_layouts.swift

@@ -213,7 +213,7 @@ public func usePrespecializedThrowsEntryPoints() throws {
 // OPT-macosx:   [[R9:%.*]] = unchecked_addr_cast [[R7]] : $*Array<Int> to $*Builtin.BridgeObject
 // OPT-macosx:   [[A3:%.*]] = unchecked_bitwise_cast [[P4]] : $Array<Int> to $Builtin.BridgeObject
 // OPT-macosx:   [[A4:%.*]] = unchecked_bitwise_cast [[P5]] : $Array<Float> to $Builtin.BridgeObject
-// OPT-macosx:   [[F2]]([[R8]], [[R9]], [[A3]], [[A4]], [[P3]]) : $@convention(thin) (@guaranteed Builtin.BridgeObject, @guaranteed Builtin.BridgeObject, Int64) -> (@out Builtin.BridgeObject, Int64, @out Builtin.BridgeObject) // user: %38
+// OPT-macosx:   [[F2]]([[R8]], [[R9]], [[A3]], [[A4]], [[P3]]) : $@convention(thin) (@guaranteed Builtin.BridgeObject, @guaranteed Builtin.BridgeObject, Int64) -> (@out Builtin.BridgeObject, Int64, @out Builtin.BridgeObject)
 // OPT: } // end sil function '$s22pre_specialize_layouts40usePresepcializedMultipleIndirectResults___2xs2ysy0a20_specialized_module_C09SomeClassC_AA0m5OtherN0Cs5Int64VSaySiGSaySfGtF'
 public final class SomeOtherClass {}
 public func usePresepcializedMultipleIndirectResults(_ c: SomeClass, _ d: SomeOtherClass, _ x: Int64, xs: [Int], ys: [Float]) {

test/SILOptimizer/sil_combine_protocol_conf.swift

@@ -124,7 +124,6 @@ public class Other {
 // CHECK: [[W1:%.*]] = witness_method $@opened("{{.*}}", any DerivedProtocol) Self, #DerivedProtocol.foo : <Self where Self : DerivedProtocol> (Self) -> () -> Int, [[O1]] : $*@opened("{{.*}}", any DerivedProtocol) Self : $@convention(witness_method: DerivedProtocol) <τ_0_0 where τ_0_0 : DerivedProtocol> (@in_guaranteed τ_0_0) -> Int
 // CHECK: apply [[W1]]<@opened("{{.*}}", any DerivedProtocol) Self>([[O1]]) : $@convention(witness_method: DerivedProtocol) <τ_0_0 where τ_0_0 : DerivedProtocol> (@in_guaranteed τ_0_0) -> Int
 // CHECK: struct_extract
-// CHECK: integer_literal
 // CHECK: builtin
 // CHECK: tuple_extract 
 // CHECK: tuple_extract 

test/stdlib/move_function.swift

@@ -91,7 +91,6 @@ tests.test("simpleVarTest") {
     expectTrue(_isUnique_native(&x))
 
     var y = x
-    expectFalse(_isUnique_native(&x))
     let _ = consume y
     expectTrue(_isUnique_native(&x))
     y = Klass()
@@ -101,7 +100,6 @@ tests.test("simpleVarTest") {
 tests.test("simpleInoutVarTest") {
     func inOutTest(_ x: inout Klass) {
         var y = x
-        expectFalse(_isUnique_native(&x))
         let _ = consume y
         expectTrue(_isUnique_native(&x))
         y = Klass()

validation-test/SILOptimizer/lexical-lifetimes.swift

@@ -29,7 +29,7 @@ class DataWrapper {
     var pointer: UnsafeMutableRawBufferPointer
 
     init(count: Int) {
-        pointer = UnsafeMutableRawBufferPointer.allocate(byteCount: count, alignment: MemoryLayout<Int>.alignment)
+        pointer = UnsafeMutableRawBufferPointer.allocate(byteCount: count * MemoryLayout<Int>.stride, alignment: MemoryLayout<Int>.alignment)
     }
 
     var bytes: UnsafeMutableRawBufferPointer { return pointer }
@@ -43,7 +43,7 @@ var fileHandleMap: [Int32 : String] = [:]
 
 func openTheFile(_ path: String) -> Int32 {
   let fd: Int32 = 42
-  assert(fileHandleMap[fd] == nil)
+  precondition(fileHandleMap[fd] == nil)
   fileHandleMap[fd] = path
   return fd
 }
@@ -53,7 +53,7 @@ func closeTheFile(_ fd: Int32) {
 }
 
 func writeToTheFile(_ fd: Int32) {
-  assert(fileHandleMap[fd] != nil)
+  precondition(fileHandleMap[fd] != nil)
 }
 
 class FileHandleWrapper {