Cleanup canonicalize OSSA reborrow handling and add tests

Author: atrick

lib/SILOptimizer/Utils/CanonicalizeOSSALifetime.cpp

@@ -121,17 +121,18 @@ static DestroyValueInst *dynCastToDestroyOf(SILInstruction *instruction,
 
 bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
   defUseWorklist.initialize(getCurrentDef());
+  // Only the first level of reborrows need to be consider. All nested inner
+  // adjacent reborrows and phis are encapsulated within their lifetimes.
+  SILPhiArgument *arg;
+  if ((arg = dyn_cast<SILPhiArgument>(getCurrentDef())) && arg->isPhi()) {
+    visitAdjacentReborrowsOfPhi(arg, [&](SILPhiArgument *reborrow) {
+      defUseWorklist.insert(reborrow);
+      return true;
+    });
+  }
   while (SILValue value = defUseWorklist.pop()) {
-    SILPhiArgument *arg;
-    if ((arg = dyn_cast<SILPhiArgument>(value)) && arg->isPhi()) {
-      visitAdjacentReborrowsOfPhi(arg, [&](SILPhiArgument *reborrow) {
-        defUseWorklist.insert(reborrow);
-        return true;
-      });
-    }
     for (Operand *use : value->getUses()) {
       auto *user = use->getUser();
-
       // Recurse through copies.
       if (auto *copy = dyn_cast<CopyValueInst>(user)) {
         defUseWorklist.insert(copy);
@@ -187,31 +188,31 @@ bool CanonicalizeOSSALifetime::computeCanonicalLiveness() {
       case OperandOwnership::InteriorPointer:
       case OperandOwnership::GuaranteedForwarding:
       case OperandOwnership::EndBorrow:
-        // Guaranteed values are considered uses of the value when the value
-        // owneed. If user is a guaranteed phi, then the owned lifetime either
-        // dominates it or its lifetime ends at an outer adjacent reborrow.
+        // Guaranteed values are exposed by inner adjacent reborrows. If user is
+        // a guaranteed phi (GuaranteedForwarding), then the owned lifetime
+        // either dominates it or its lifetime ends at an outer adjacent
+        // reborrow. Only instructions that end the reborrow lifetime should
+        // actually affect liveness of the outer owned value.
         liveness.updateForUse(user, /*lifetimeEnding*/ false);
         break;
       case OperandOwnership::Reborrow:
-        BranchInst *branch;
-        if (!(branch = dyn_cast<BranchInst>(user))) {
-          // Non-phi reborrows (tuples, etc) never end the lifetime of the owned
-          // value.
-          liveness.updateForUse(user, /*lifetimeEnding*/ false);
-          defUseWorklist.insert(cast<SingleValueInstruction>(user));
-          break;
-        }
-        if (is_contained(user->getOperandValues(), getCurrentDef())) {
-          // An adjacent phi consumes the value being reborrowed.  Although this
-          // use doesn't end the lifetime, this user does.
-          liveness.updateForUse(user, /*lifetimeEnding*/ true);
+        BranchInst *branch = cast<BranchInst>(user);
+        // This is a cheap variation on visitEnclosingDef. We already know that
+        // getCurrentDef() is the enclosing def for this use. If the reborrow's
+        // has a enclosing def is an outer adjacent phi then this branch must
+        // consume getCurrentDef() as the outer phi operand.
+        if (is_contained(branch->getOperandValues(), getCurrentDef())) {
+          // An adjacent phi consumes the value being reborrowed. Although this
+          // use doesn't end the lifetime, this branch does end the lifetime by
+          // consuming the owned value.
+          liveness.updateForUse(branch, /*lifetimeEnding*/ true);
           break;
         }
         // No adjacent phi consumes the value.  This use is not lifetime ending.
-        liveness.updateForUse(user, /*lifetimeEnding*/ false);
+        liveness.updateForUse(branch, /*lifetimeEnding*/ false);
         // This branch reborrows a guaranteed phi whose lifetime is dependent on
         // currentDef.  Uses of the reborrowing phi extend liveness.
-        auto *reborrow = branch->getArgForOperand(use);
+        auto *reborrow = PhiOperand(use).getValue();
         defUseWorklist.insert(reborrow);
         break;
       }

test/SILOptimizer/copy_propagation_canonicalize_with_reborrows.sil

@@ -271,3 +271,82 @@ exit:
   return %retval : $()
 }
 
+// The owned phi %5 has an inner adjacent phi %4, so its end_borrow is
+// considered a use of the owned lifetime. The end_borrow/destroy
+// maintain their position.
+//
+// CHECK-LABEL: sil [ossa] @reborrow_adjacent_to_ownedphi : $@convention(thin) (@owned X) -> () {
+// CHECK:   end_borrow
+// CHECK:   destroy_value
+sil [ossa] @reborrow_adjacent_to_ownedphi : $@convention(thin) (@owned X) -> () {
+bb0(%0 : @owned $X):
+  %1 = begin_borrow %0 : $X
+  br bb1(%1 : $X, %0 : $X)
+
+bb1(%4 : @guaranteed $X, %5 : @owned $X):
+  end_borrow %4 : $X
+  destroy_value %5 : $X
+  %23 = tuple ()
+  return %23 : $()
+}
+
+// The owned phi %5 has an inner adjacent phi %4, so its reborrow use,
+// %6 is considered a use of the owned value. Since %5 is also
+// consumed by an owned phi in bb2, the owned lifetime extends no further.
+//
+// Lifetime analysis is successful; the copy/destroy in bb1 are removed. The
+// end_borrow/destroy maintain their position.
+//
+// CHECK-LABEL: sil [ossa] @reborrow_adjacent_to_consume : $@convention(thin) (@owned X) -> () {
+// CHECK: bb1(%{{.*}} : @guaranteed $X, %{{.*}} : @owned $X):
+// CHECK-NEXT:   br bb2
+// CHECK: bb2(%{{.*}} : @guaranteed $X, %{{.*}} : @owned $X):
+// CHECK-NEXT:   end_borrow
+// CHECK-NEXT:   destroy_value
+sil [ossa] @reborrow_adjacent_to_consume : $@convention(thin) (@owned X) -> () {
+bb0(%0 : @owned $X):
+  %1 = begin_borrow %0 : $X
+  br bb1(%1 : $X, %0 : $X)
+
+bb1(%4 : @guaranteed $X, %5 : @owned $X):
+  %copy = copy_value %5 : $X
+  destroy_value %copy : $X
+  br bb2(%4 : $X, %5 : $X)
+
+bb2(%6 : @guaranteed $X, %7 : @owned $X):
+  end_borrow %6 : $X
+  destroy_value %7 : $X
+  %23 = tuple ()
+  return %23 : $()
+}
+
+// The owned phi %5 has an inner adjacent phi %4, so its reborrow use,
+// %6 is considered a use of the owned value. Since %5 is not consumed
+// by an owned phi in bb2, the owned lifetime extends to the uses of
+// the second reborrow, %6. The copy/destroy in bb2 are removed. The
+// end_borrow/destroy maintain their position.
+//
+// Lifetime analysis is successful; the copy/destroy in bb2 are removed.
+//
+// CHECK-LABEL: sil [ossa] @reborrow_no_adjacent_consume : $@convention(thin) (@owned X) -> () {
+// CHECK: bb1(%{{.*}} : @guaranteed $X, %{{.*}} : @owned $X):
+// CHECK-NEXT:   br bb2
+// CHECK: bb2(%{{.*}} : @guaranteed $X):
+// CHECK-NEXT:   end_borrow
+// CHECK-NEXT:   destroy_value
+sil [ossa] @reborrow_no_adjacent_consume : $@convention(thin) (@owned X) -> () {
+bb0(%0 : @owned $X):
+  %1 = begin_borrow %0 : $X
+  br bb1(%1 : $X, %0 : $X)
+
+bb1(%4 : @guaranteed $X, %5 : @owned $X):
+  br bb2(%4 : $X)
+
+bb2(%6 : @guaranteed $X):
+  %copy = copy_value %5 : $X
+  destroy_value %copy : $X
+  end_borrow %6 : $X
+  destroy_value %5 : $X
+  %23 = tuple ()
+  return %23 : $()
+}