Merge pull request #41310 from nate-chandler/lexical_lifetimes/destroy_hoisting/enable

[SILOpt] Enabled SSADestroyHoisting.

Author: nate-chandler

lib/SIL/IR/SILInstruction.cpp

@@ -1382,7 +1382,7 @@ bool SILInstruction::mayTrap() const {
 bool SILInstruction::maySynchronize() const {
   // TODO: We need side-effect analysis and library annotation for this to be
   //       a reasonable API.  For now, this is just a placeholder.
-  return isa<FullApplySite>(this);
+  return isa<FullApplySite>(this) || isa<EndApplyInst>(this);
 }
 
 bool SILInstruction::isMetaInstruction() const {

lib/SIL/Utils/MemAccessUtils.cpp

@@ -1959,42 +1959,34 @@ bool GatherUniqueStorageUses::visitUse(Operand *use, AccessUseType useTy) {
     case SILArgumentConvention::Indirect_Inout:
     case SILArgumentConvention::Indirect_InoutAliasable:
     case SILArgumentConvention::Indirect_Out:
-      visitor.visitStore(use);
-      break;
+      return visitor.visitStore(use);
     case SILArgumentConvention::Indirect_In_Guaranteed:
     case SILArgumentConvention::Indirect_In:
     case SILArgumentConvention::Indirect_In_Constant:
-      visitor.visitLoad(use);
-      break;
+      return visitor.visitLoad(use);
     case SILArgumentConvention::Direct_Unowned:
     case SILArgumentConvention::Direct_Owned:
     case SILArgumentConvention::Direct_Guaranteed:
       // most likely an escape of a box
-      visitor.visitUnknownUse(use);
-      break;
+      return visitor.visitUnknownUse(use);
     }
-    return true;
   }
   switch (user->getKind()) {
   case SILInstructionKind::DestroyAddrInst:
   case SILInstructionKind::DestroyValueInst:
     if (useTy == AccessUseType::Exact) {
-      visitor.visitDestroy(use);
-      return true;
+      return visitor.visitDestroy(use);
     }
-    visitor.visitUnknownUse(use);
-    return true;
+    return visitor.visitUnknownUse(use);
 
   case SILInstructionKind::DebugValueInst:
-    visitor.visitDebugUse(use);
-    return true;
+    return visitor.visitDebugUse(use);
 
   case SILInstructionKind::LoadInst:
   case SILInstructionKind::LoadWeakInst:
   case SILInstructionKind::LoadUnownedInst:
   case SILInstructionKind::ExistentialMetatypeInst:
-    visitor.visitLoad(use);
-    return true;
+    return visitor.visitLoad(use);
 
   case SILInstructionKind::StoreInst:
   case SILInstructionKind::StoreWeakInst:
@@ -2006,27 +1998,22 @@ bool GatherUniqueStorageUses::visitUse(Operand *use, AccessUseType useTy) {
     break;
 
   case SILInstructionKind::InjectEnumAddrInst:
-    visitor.visitStore(use);
-    return true;
+    return visitor.visitStore(use);
 
   case SILInstructionKind::CopyAddrInst:
     if (operIdx == CopyLikeInstruction::Dest) {
-      visitor.visitStore(use);
-      return true;
+      return visitor.visitStore(use);
     }
     assert(operIdx == CopyLikeInstruction::Src);
-    visitor.visitLoad(use);
-    return true;
+    return visitor.visitLoad(use);
 
   case SILInstructionKind::DeallocStackInst:
-    visitor.visitDealloc(use);
-    return true;
+    return visitor.visitDealloc(use);
 
   default:
     break;
   }
-  visitor.visitUnknownUse(use);
-  return true;
+  return visitor.visitUnknownUse(use);
 }
 
 //===----------------------------------------------------------------------===//

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -346,6 +346,8 @@ void addFunctionPasses(SILPassPipelinePlan &P,
   // Promote box allocations to stack allocations.
   P.addAllocBoxToStack();
 
+  P.addSSADestroyHoisting();
+
   // Propagate copies through stack locations.  Should run after
   // box-to-stack promotion since it is limited to propagating through
   // stack locations. Should run before aggregate lowering since that

lib/SILOptimizer/Transforms/SSADestroyHoisting.cpp

@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 ///
 /// This is a light-weight utility for hoisting destroy instructions for unique
-/// storage--typically alloc_stac or owned incoming arguments. Shrinking an
+/// storage--typically alloc_stack or owned incoming arguments. Shrinking an
 /// object's memory lifetime can allow removal of copy_addr and other
 /// optimization.
 ///
@@ -206,6 +206,9 @@ class DeinitBarriers {
   }
 
 private:
+  DeinitBarriers(DeinitBarriers const &) = delete;
+  DeinitBarriers &operator=(DeinitBarriers const &) = delete;
+
   // Conforms to BackwardReachability::BlockReachability
   class DestroyReachability {
     const KnownStorageUses &knownUses;
@@ -386,9 +389,13 @@ bool HoistDestroys::rewriteDestroys(const KnownStorageUses &knownUses,
 bool HoistDestroys::foldBarrier(SILInstruction *barrier) {
   if (auto *load = dyn_cast<LoadInst>(barrier)) {
     if (load->getOperand() == storageRoot) {
-      assert(load->getOwnershipQualifier() == LoadOwnershipQualifier::Copy);
-      load->setOwnershipQualifier(LoadOwnershipQualifier::Take);
-      return true;
+      if (load->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
+        load->setOwnershipQualifier(LoadOwnershipQualifier::Take);
+        return true;
+      } else {
+        assert(load->getOperand()->getType().isTrivial(*load->getFunction()));
+        return false;
+      }
     }
   }
   if (auto *copy = dyn_cast<CopyAddrInst>(barrier)) {

test/SILOptimizer/bridged_casts_folding.sil

@@ -49,9 +49,8 @@ bb3(%8 : @owned $NSObjectSubclass):
 
 // CHECK-LABEL: sil @anyhashable_cast_take_on_success
 // CHECK:         [[BRIDGED:%.*]] = apply {{.*}}(%0)
-// CHECK-NEXT:    checked_cast_br [[BRIDGED]] : $NSObject to NSObjectSubclass, [[YES:bb[0-9]+]], [[NO:bb[0-9]+]]
-// CHECK:       [[YES]]{{.*}}:
 // CHECK-NEXT:    destroy_addr %0
+// CHECK-NEXT:    checked_cast_br [[BRIDGED]] : $NSObject to NSObjectSubclass, {{bb[0-9]+}}, {{bb[0-9]+}}
 sil [ossa] @anyhashable_cast_take_on_success : $@convention(thin) (@in AnyHashable, @owned NSObjectSubclass) -> @owned NSObjectSubclass {
 entry(%0 : $*AnyHashable, %1 : @owned $NSObjectSubclass):
   %2 = alloc_stack $NSObjectSubclass

test/SILOptimizer/bridged_casts_folding_ownership.sil

@@ -51,9 +51,8 @@ bb3(%8 : @owned $NSObjectSubclass):
 
 // CHECK-LABEL: sil @anyhashable_cast_take_on_success
 // CHECK:         [[BRIDGED:%.*]] = apply {{.*}}(%0)
-// CHECK-NEXT:    checked_cast_br [[BRIDGED]] : $NSObject to NSObjectSubclass, [[YES:bb[0-9]+]], [[NO:bb[0-9]+]]
-// CHECK:       [[YES]]{{.*}}:
 // CHECK-NEXT:    destroy_addr %0
+// CHECK-NEXT:    checked_cast_br [[BRIDGED]] : $NSObject to NSObjectSubclass, [[YES:bb[0-9]+]], [[NO:bb[0-9]+]]
 sil [ossa] @anyhashable_cast_take_on_success : $@convention(thin) (@in AnyHashable, @owned NSObjectSubclass) -> @owned NSObjectSubclass {
 entry(%0 : $*AnyHashable, %1 : @owned $NSObjectSubclass):
   %2 = alloc_stack $NSObjectSubclass

test/SILOptimizer/capture_promotion_generic_context_ownership.sil

@@ -53,8 +53,8 @@ entry(%0 : $*T, %b : @owned $<τ_0_0> { var τ_0_0 } <Int>):
 
 // CHECK-LABEL: sil @call_generic_promotable_box_from_different_generic
 // CHECK:       bb0([[ARG0:%.*]] : $*T, [[ARG1:%.*]] : $*U, [[ARG2:%.*]] : $Builtin.Int32):
-// CHECK-NEXT:    destroy_addr [[ARG0]] : $*T
 // CHECK-NEXT:    destroy_addr [[ARG1]] : $*U
+// CHECK-NEXT:    destroy_addr [[ARG0]] : $*T
 // CHECK:         [[F:%.*]] = function_ref @$s22generic_promotable_boxTf2ni_n : $@convention(thin) <τ_0_0> (@in τ_0_0, Builtin.Int32) -> Builtin.Int32
 // CHECK-NEXT:    [[CLOSURE:%.*]] = partial_apply [callee_guaranteed] [[F]]<U>([[ARG2]])
 // CHECK-NEXT:    return [[CLOSURE]]

test/SILOptimizer/hoist_destroy_addr.sil

@@ -39,11 +39,16 @@ public enum E {
   case B
 }
 
+struct TrivialStruct {
+  var e: E
+}
+
 sil @unknown : $@convention(thin) () -> ()
 sil @use_S : $@convention(thin) (@in_guaranteed S) -> ()
 
 sil @f_out : $@convention(thin) <T> () -> @out T
 sil @f_bool : $@convention(thin) () -> Builtin.Int1
+sil [ossa] @take_trivial_struct : $@convention(thin) (TrivialStruct) -> ()
 
 // CHECK-LABEL: sil [ossa] @test_simple
 // CHECK:      bb0(%0 : $*S):
@@ -229,3 +234,23 @@ bb3:
   return %16 : $()
 }
 
+// Hoist a destroy_addr of a trivial value over a function_ref.  DO NOT fold
+// with the load [trivial].
+//
+// CHECK-LABEL: sil [ossa] @test_hoist_trivial : {{.*}} {
+// CHECK:         load [trivial]
+// CHECK:         function_ref
+// CHECK-LABEL: } // end sil function 'test_hoist_trivial'
+sil [ossa] @test_hoist_trivial : $@convention(thin) (TrivialStruct) -> () {
+entry(%instance : @none $TrivialStruct):
+    %addr = alloc_stack $TrivialStruct
+    store %instance to [trivial] %addr : $*TrivialStruct
+    %copy = load [trivial] %addr : $*TrivialStruct
+    %take_trivial_struct = function_ref @take_trivial_struct : $@convention(thin) (TrivialStruct) -> ()
+    destroy_addr %addr : $*TrivialStruct
+    apply %take_trivial_struct(%copy) : $@convention(thin) (TrivialStruct) -> ()
+    dealloc_stack %addr : $*TrivialStruct
+
+    %retval = tuple ()
+    return %retval : $()
+}

test/SILOptimizer/shrink_borrow_scope.sil

@@ -11,6 +11,7 @@ class C {
     weak var d: D?
 }
 class D {}
+struct S {}
 class DBox {
   var d: D
 }
@@ -32,6 +33,7 @@ sil [ossa] @callee_guaranteed: $@convention(thin) (@guaranteed C) -> ()
 sil [ossa] @callee_owned : $@convention(thin) (@owned C) -> ()
 sil [ossa] @callee_optional_d_guaranteed: $@convention(thin) (@guaranteed Optional<D>) -> ()
 sil [ossa] @synchronization_point : $@convention(thin) () -> ()
+sil [ossa] @modify_s : $@yield_once @convention(thin) () -> @yields @inout S
 
 // =============================================================================
 // = DECLARATIONS                                                             }}
@@ -806,6 +808,27 @@ exit:
   return %retval : $()
 }
 
+// Don't hoist over end_apply.  These are lowered to calls to continuations
+// which can have the same sorts of side-effects as function calls.
+
+// CHECK-LABEL: sil [ossa] @dont_hoist_over_end_apply : {{.*}} {
+// CHECK:       end_apply
+// CHECK:       end_borrow
+// CHECK-LABEL: } // end sil function 'dont_hoist_over_end_apply'
+sil [ossa] @dont_hoist_over_end_apply : $@convention(thin) (@owned C, S) -> () {
+entry(%instance : @owned $C, %input : $S):
+    %lifetime = begin_borrow [lexical] %instance : $C
+    %modify_s = function_ref @modify_s : $@yield_once @convention(thin) () -> @yields @inout S
+    (%addr, %continuation) = begin_apply %modify_s() : $@yield_once @convention(thin) () -> @yields @inout S
+    store %input to [trivial] %addr : $*S
+    end_apply %continuation
+    end_borrow %lifetime : $C
+    destroy_value %instance : $C
+    %retval = tuple ()
+    return %retval : $()
+}
+
+
 // =============================================================================
 // instruction tests                                                          }}
 // =============================================================================