From b0786e7e06230a3d1f59271389b56222ea74aca3 Mon Sep 17 00:00:00 2001
From: Andrew Trick <atrick@apple.com>
Date: Tue, 28 Jan 2020 21:13:22 -0800
Subject: [PATCH 1/2] Fix propagation of guaranteed phi args during
 DiagnoseUnreachable.

Fixes <rdar://58929109> assertion failure - UNREACHABLE executed at
swift/include/swift/SIL/OwnershipUtils.h:127!
---
 .../Mandatory/DiagnoseUnreachable.cpp         |  7 ++---
 test/SILOptimizer/diagnose_unreachable.sil    | 30 +++++++++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp b/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp
index 6923725dc55b6..107829132d2a8 100644
--- a/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp
+++ b/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp
@@ -185,12 +185,9 @@ static void propagateBasicBlockArgs(SILBasicBlock &BB) {
     // this to CCP and trigger another round of copy propagation.
     SILArgument *Arg = *AI;
 
-    // If this argument is guaranteed and Args[Idx] is a SILFunctionArgument,
-    // delete the end_borrow.
-    if (Arg->getOwnershipKind() == ValueOwnershipKind::Guaranteed &&
-        isa<SILFunctionArgument>(Args[Idx])) {
+    // If this argument is guaranteed and Args[Idx], delete the end_borrow.
+    if (Arg->getOwnershipKind() == ValueOwnershipKind::Guaranteed)
       deleteEndBorrows(Arg);
-    }
 
     // We were able to fold, so all users should use the new folded value.
     Arg->replaceAllUsesWith(Args[Idx]);
diff --git a/test/SILOptimizer/diagnose_unreachable.sil b/test/SILOptimizer/diagnose_unreachable.sil
index bbf443dc223ec..7045ca9e368ab 100644
--- a/test/SILOptimizer/diagnose_unreachable.sil
+++ b/test/SILOptimizer/diagnose_unreachable.sil
@@ -782,3 +782,33 @@ bb1(%2 : @owned $Builtin.NativeObject):
 bb2:
   unreachable
 }
+
+// Test propagation of guaranteed phi arguments. The nested end_borrow
+// must be removed, even with the outer borrow is *not* a function
+// argument.
+
+enum EnumWithB {
+  case A(B)
+  func testit() -> Int
+}
+
+// CHECK-LABEL: sil hidden [ossa] @testPropagateGuaranteedPhi : $@convention(method) (@guaranteed EnumWithB) -> () {
+// CHECK: bb1([[PHI:%.*]] : @guaranteed $B):
+// CHECK:   br bb2
+// CHECK: bb2:
+// CHECK:   end_borrow [[PHI]] : $B
+// CHECK-NOT: end_borrow
+// CHECK-LABEL: } // end sil function 'testPropagateGuaranteedPhi'
+sil hidden [ossa] @testPropagateGuaranteedPhi : $@convention(method) (@guaranteed EnumWithB) -> () {
+bb0(%0 : @guaranteed $EnumWithB):
+  switch_enum %0 : $EnumWithB, case #EnumWithB.A!enumelt.1: bb1
+
+bb1(%2 : @guaranteed $B):
+  br bb3(%2 : $B)
+
+bb3(%4 : @guaranteed $B):
+  end_borrow %4 : $B
+  end_borrow %2 : $B
+  %99 = tuple ()
+  return %99 : $()
+}

From 1af49ecb996e66aa0b7de7395ee073973a13ed7c Mon Sep 17 00:00:00 2001
From: Andrew Trick <atrick@apple.com>
Date: Tue, 28 Jan 2020 22:54:08 -0800
Subject: [PATCH 2/2] Fix an EscapeAnalysis assert to handle recent changes.

setPointsToEdge should assert that its target isn't already merged,
but now that we batch up multiple merge requests, it's fine to allow
the target to be scheduled-for-merge.

Many assertions have been recently added and tightened in order to
"discover" unexpected cases. There's nothing incorrect about how these
cases were handled, but they lack unit tests. In this case I still
haven't been able to reduce a test case. I'm continuing to work on
it, but don't want to further delay the fix.
---
 include/swift/SILOptimizer/Analysis/EscapeAnalysis.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h b/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h
index c1d405c4d7f42..2d56b97d85695 100644
--- a/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h
+++ b/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h
@@ -449,7 +449,7 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
 
     /// Sets the outgoing points-to edge. The \p To node must be a Content node.
     void setPointsToEdge(CGNode *To) {
-      assert(!To->mergeTo);
+      assert(!To->isMerged);
       assert(To->Type == NodeType::Content &&
              "Wrong node type for points-to edge");
       pointsToIsEdge = true;