Merge pull request #79878 from eeckstein/fix-condition-forwarding

ConditionForwarding: don't violate ownership

Author: eeckstein

lib/SILOptimizer/Transforms/ConditionForwarding.cpp

@@ -126,8 +126,25 @@ class ConditionForwarding : public SILFunctionTransform {
 
 /// Returns true if all instructions of block \p BB are safe to be moved
 /// across other code.
-static bool hasNoRelevantSideEffects(SILBasicBlock *BB) {
+static bool hasNoRelevantSideEffects(SILBasicBlock *BB, EnumInst *enumInst) {
   for (SILInstruction &I : *BB) {
+    if (BB->getParent()->hasOwnership() && &I != enumInst) {
+      // The instruction must not use any (non-trivial) value because we don't
+      // do liveness analysis. When moving the block, there is no guarantee that
+      // the operand value is still alive at the new location.
+      for (Operand *op : I.getRealOperands()) {
+        SILValue opv = op->get();
+        // The `enum` is an exception, because it's a forwarded value and we already
+        // check that it's forwarded to the `switch_enum` at the new location.
+        if (opv == enumInst)
+          continue;
+        // If the value is defined in the block it's a block-local liferange.
+        if (opv->getParentBlock() == BB)
+          continue;
+        if (opv->getOwnershipKind() != OwnershipKind::None)
+          return false;
+      }
+    }
     if (I.getMemoryBehavior() == MemoryBehavior::None)
       continue;
     if (auto *CF = dyn_cast<CondFailInst>(&I)) {
@@ -144,9 +161,6 @@ static bool hasNoRelevantSideEffects(SILBasicBlock *BB) {
         return false;
       continue;
     }
-    if (isa<BeginBorrowInst>(&I) || isa<EndBorrowInst>(&I)) {
-      continue;
-    }
     LLVM_DEBUG(llvm::dbgs() << "Bailing out, found inst with side-effects ");
     LLVM_DEBUG(I.dump());
     return false;
@@ -212,7 +226,7 @@ bool ConditionForwarding::tryOptimize(SwitchEnumInst *SEI) {
     CommonBranchBlock = PredPred;
 
     // We cannot move the block across other code if it has side-effects.
-    if (!hasNoRelevantSideEffects(Pred))
+    if (!hasNoRelevantSideEffects(Pred, EI))
       return false;
     PredBlocks.push_back(Pred);
   }

test/SILOptimizer/conditionforwarding_ossa.sil

@@ -419,3 +419,38 @@ bb6:
   %15 = tuple ()
   return %15
 }
+
+// CHECK-LABEL: sil [ossa] @copy_from_limited_liferange : 
+// CHECK:         switch_enum
+// CHECK-LABEL: } // end sil function 'copy_from_limited_liferange'
+sil [ossa] @copy_from_limited_liferange : $@convention(thin) (Builtin.Int1, @guaranteed C) -> () {
+bb0(%0 : $Builtin.Int1, %1 : @guaranteed $C):
+  %2 = begin_borrow [lexical] %1
+  cond_br %0, bb1, bb2
+
+bb1:
+  %4 = copy_value %2
+  %5 = enum $Optional<C>, #Optional.some!enumelt, %4
+  br bb3(%5)
+
+bb2:
+  %7 = enum $Optional<C>, #Optional.none!enumelt
+  br bb3(%7)
+
+bb3(%9 : @owned $Optional<C>):
+  end_borrow %2
+  switch_enum %9, case #Optional.some!enumelt: bb5, case #Optional.none!enumelt: bb4
+
+bb4:
+  br bb6
+
+bb5(%13 : @owned $C):
+  destroy_value %13
+  br bb6
+
+bb6:
+  %16 = tuple ()
+  return %16
+}
+
+