Merge pull request swiftlang#41050 from nate-chandler/sil/move_value_lexical_restricts_motion

[SILOpt] Made lexical flag obstruct some motion.

Author: nate-chandler

include/swift/SIL/SILValue.h

@@ -540,6 +540,8 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   /// NOTE: This is implemented in ValueOwnership.cpp not SILValue.cpp.
   ValueOwnershipKind getOwnershipKind() const;
 
+  bool isLexical() const;
+
   static bool classof(SILNodePointer node) {
     return node->getKind() >= SILNodeKind::First_ValueBase &&
            node->getKind() <= SILNodeKind::Last_ValueBase;

lib/SIL/IR/SILValue.cpp

@@ -98,6 +98,18 @@ ValueBase::getDefiningInstructionResult() {
   return None;
 }
 
+bool ValueBase::isLexical() const {
+  // TODO: Eventually, rather than SILGen'ing a borrow scope for owned 
+  //       arguments, we will just have this check here.
+  // if (auto *argument = dyn_cast<SILArgument>(this))
+  //   return argument->getOwnershipKind() == OwnershipKind::Owned;
+  if (auto *bbi = dyn_cast<BeginBorrowInst>(this))
+    return bbi->isLexical();
+  if (auto *mvi = dyn_cast<MoveValueInst>(this))
+    return mvi->isLexical();
+  return false;
+}
+
 SILBasicBlock *SILNode::getParentBlock() const {
   if (auto *Inst = dyn_cast<SILInstruction>(this))
     return Inst->getParent();

lib/SILOptimizer/Utils/CanonicalOSSALifetime.cpp

@@ -772,6 +772,9 @@ bool CanonicalizeOSSALifetime::canonicalizeValueLifetime(SILValue def) {
   if (def.getOwnershipKind() != OwnershipKind::Owned)
     return false;
 
+  if (def->isLexical())
+    return false;
+
   if (poisonRefsMode && !shouldCanonicalizeWithPoison(def))
     return false;
 

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

@@ -300,6 +300,12 @@ bool OwnershipRAUWHelper::hasValidRAUWOwnership(SILValue oldValue,
                                                 SILValue newValue) {
   auto newOwnershipKind = newValue.getOwnershipKind();
 
+  // If the either value is lexical, replacing its uses may result in
+  // shortening or lengthening its lifetime in ways that don't respect lexical
+  // scope and deinit barriers.
+  if (oldValue->isLexical() || newValue->isLexical())
+    return false;
+
   // If our new kind is ValueOwnershipKind::None, then we are fine. We
   // trivially support that. This check also ensures that we can always
   // replace any value with a ValueOwnershipKind::None value.

test/SILOptimizer/copy_propagation.sil

@@ -28,6 +28,7 @@ class C {
 }
 
 sil [ossa] @dummy : $@convention(thin) () -> ()
+sil [ossa] @barrier : $@convention(thin) () -> ()
 sil [ossa] @getOwnedC : $@convention(thin) () -> (@owned C)
 sil [ossa] @takeOwnedC : $@convention(thin) (@owned C) -> ()
 sil [ossa] @takeOwnedCTwice : $@convention(thin) (@owned C, @owned C) -> ()
@@ -836,3 +837,26 @@ bb0(%0 : @owned $C):
   %7 = begin_borrow %6 : $C
   unreachable
 }
+
+// Test that copy propagation doesn't hoist a destroy_value corresponding to
+// a move value [lexical] over a barrier.
+// CHECK-ONONE-LABEL: sil [ossa] @dont_hoist_move_value_lexical_destroy_over_barrier_apply : {{.*}} {
+// CHECK-ONONE:       {{bb[0-9]+}}([[INSTANCE:%[^,]+]] : @owned $C):
+// CHECK-ONONE:         [[LIFETIME:%[^,]+]] = move_value [lexical] [[INSTANCE]]
+// CHECK-ONONE:         [[BARRIER:%[^,]+]] = function_ref @barrier
+// CHECK-ONONE:         [[TAKE_GUARANTEED_C:%[^,]+]] = function_ref @takeGuaranteedC
+// CHECK-ONONE:         apply [[TAKE_GUARANTEED_C]]([[LIFETIME]])
+// CHECK-ONONE:         apply [[BARRIER]]()
+// CHECK-ONONE:         destroy_value [[LIFETIME]]
+// CHECK-ONONE-LABEL: } // end sil function 'dont_hoist_move_value_lexical_destroy_over_barrier_apply'
+sil [ossa] @dont_hoist_move_value_lexical_destroy_over_barrier_apply : $@convention(thin) (@owned C) -> () {
+entry(%instance : @owned $C):
+  %lifetime = move_value [lexical] %instance : $C
+  %barrier = function_ref @barrier : $@convention(thin) () -> ()
+  %takeGuaranteedC = function_ref @takeGuaranteedC : $@convention(thin) (@guaranteed C) -> ()
+  apply %takeGuaranteedC(%lifetime) : $@convention(thin) (@guaranteed C) -> ()
+  apply %barrier() : $@convention(thin) () -> ()
+  destroy_value %lifetime : $C
+  %retval = tuple ()
+  return %retval : $()
+}

test/SILOptimizer/sil_combine_ossa.sil

@@ -4218,6 +4218,27 @@ bb0(%0 : @guaranteed $B):
   return %2 : $B
 }
 
+// Check that a mark_dependence on an enum on a lexical value doesn't result in
+// RAUWing the enum with the lexical value and extending the latter's lifetime.
+//
+// CHECK-LABEL: sil [ossa] @mark_dependence_base1_lexical1 : {{.*}} {
+// CHECK:       {{bb[0-9]+}}([[ADDR:%[^,]+]] : $*Builtin.Int64, [[INSTANCE:%[^,]+]] : @owned $B):
+// CHECK:         [[LIFETIME:%[^,]+]] = move_value [lexical] [[INSTANCE]]
+// CHECK:         [[OPTIONAL:%[^,]+]] = enum $FakeOptional<B>, #FakeOptional.some!enumelt, [[LIFETIME]]
+// CHECK:         [[DEPENDENT_ADDR:%[^,]+]] = mark_dependence [[ADDR]]
+// CHECK:         [[VALUE:%[^,]+]] = load [trivial] [[DEPENDENT_ADDR]]
+// CHECK:         return [[VALUE]]
+// CHECK-LABEL: } // end sil function 'mark_dependence_base1_lexical1'
+sil [ossa] @mark_dependence_base1_lexical1 : $@convention(thin) (@inout Builtin.Int64, @owned B) -> Builtin.Int64 {
+bb0(%addr : $*Builtin.Int64, %instance : @owned $B):
+  %lifetime = move_value [lexical] %instance : $B
+  %optional = enum $FakeOptional<B>, #FakeOptional.some!enumelt, %lifetime : $B
+  %dependent_addr = mark_dependence %addr : $*Builtin.Int64 on %optional : $FakeOptional<B>
+  %value = load [trivial] %dependent_addr : $*Builtin.Int64
+  destroy_value %optional : $FakeOptional<B>
+  return %value : $Builtin.Int64
+}
+
 protocol _NSArrayCore {}
 
 // CHECK-LABEL: sil [ossa] @mark_dependence_base2 :