swiftlang · aschwaighofer · Mar 9, 2017 · Mar 9, 2017
diff --git a/include/swift/SILOptimizer/Analysis/ARCAnalysis.h b/include/swift/SILOptimizer/Analysis/ARCAnalysis.h
@@ -375,19 +375,18 @@ getSingleUnsafeGuaranteedValueResult(BuiltinInst *UnsafeGuaranteedInst);
 /// "unsafeGuaranteed"'s token.
 BuiltinInst *getUnsafeGuaranteedEndUser(SILInstruction *UnsafeGuaranteedToken);
 
-/// Walk backwards from an unsafeGuaranteedEnd builtin instruction looking for a
+/// Walk forwards from an unsafeGuaranteedEnd builtin instruction looking for a
 /// release on the reference returned by the matching unsafeGuaranteed builtin
 /// ignoring releases on the way.
 /// Return nullptr if no release is found.
 ///
 ///    %4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
 ///    %5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
 ///    %6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
-///    strong_release %5 : $Foo // <-- Matching release.
-///    strong_release %6 : $Foo // Ignore.
 ///    %12 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
+///    strong_release %5 : $Foo // <-- Matching release.
 ///
-/// Alternatively, look for the release after the unsafeGuaranteedEnd.
+/// Alternatively, look for the release before the unsafeGuaranteedEnd.
 SILInstruction *findReleaseToMatchUnsafeGuaranteedValue(
     SILInstruction *UnsafeGuaranteedEndI, SILInstruction *UnsafeGuaranteedI,
     SILValue UnsafeGuaranteedValue, SILBasicBlock &BB,

diff --git a/lib/SILOptimizer/Analysis/ARCAnalysis.cpp b/lib/SILOptimizer/Analysis/ARCAnalysis.cpp
@@ -1133,11 +1133,11 @@ SILInstruction *swift::findReleaseToMatchUnsafeGuaranteedValue(
   auto UnsafeGuaranteedOpdRoot =
       RCFI.getRCIdentityRoot(UnsafeGuaranteedI->getOperand(0));
 
-  // Look before the "unsafeGuaranteedEnd".
-  for (auto ReverseIt = ++UnsafeGuaranteedEndI->getIterator().getReverse(),
-            End = BB.rend();
-       ReverseIt != End; ++ReverseIt) {
-    SILInstruction &CurInst = *ReverseIt;
+  // Try finding it after the "unsafeGuaranteedEnd".
+  for (auto ForwardIt = std::next(UnsafeGuaranteedEndI->getIterator()),
+            End = BB.end();
+       ForwardIt != End; ++ForwardIt) {
+    SILInstruction &CurInst = *ForwardIt;
 
     // Is this a release?
     if (isa<ReleaseValueInst>(CurInst) || isa<StrongReleaseInst>(CurInst)) {
@@ -1152,11 +1152,11 @@ SILInstruction *swift::findReleaseToMatchUnsafeGuaranteedValue(
       break;
   }
 
-  // Otherwise, try finding it after the "unsafeGuaranteedEnd".
-  for (auto ForwardIt = std::next(UnsafeGuaranteedEndI->getIterator()),
-            End = BB.end();
-       ForwardIt != End; ++ForwardIt) {
-    SILInstruction &CurInst = *ForwardIt;
+  // Otherwise, Look before the "unsafeGuaranteedEnd".
+  for (auto ReverseIt = ++UnsafeGuaranteedEndI->getIterator().getReverse(),
+            End = BB.rend();
+       ReverseIt != End; ++ReverseIt) {
+    SILInstruction &CurInst = *ReverseIt;
 
     // Is this a release?
     if (isa<ReleaseValueInst>(CurInst) || isa<StrongReleaseInst>(CurInst)) {
@@ -1170,6 +1170,7 @@ SILInstruction *swift::findReleaseToMatchUnsafeGuaranteedValue(
         !isa<DebugValueAddrInst>(CurInst))
       break;
   }
+
   return nullptr;
 }
 
diff --git a/lib/SILOptimizer/Transforms/UnsafeGuaranteedPeephole.cpp b/lib/SILOptimizer/Transforms/UnsafeGuaranteedPeephole.cpp
@@ -44,6 +44,56 @@
 
 using namespace swift;
 
+/// Pattern match and remove "retain(self), apply(self), release(self)" calls
+/// inbetween unsafeGuaranteed pairs and remove the retain/release pairs.
+static void tryRemoveRetainReleasePairsBetween(
+    RCIdentityFunctionInfo &RCFI, SILInstruction *UnsafeGuaranteedI,
+    SILInstruction *Retain, SILInstruction *Release,
+    SILInstruction *UnsafeGuaranteedEndI) {
+  auto *BB = UnsafeGuaranteedI->getParent();
+  if (BB != UnsafeGuaranteedEndI->getParent() || BB != Retain->getParent() ||
+      BB != Release->getParent())
+    return;
+
+  SILInstruction *CandidateRetain = nullptr;
+  SmallVector<SILInstruction *, 8> InstsToDelete;
+
+  SILBasicBlock::iterator It(UnsafeGuaranteedI);
+  while (It != BB->end() && It != SILBasicBlock::iterator(Release) &&
+         It != SILBasicBlock::iterator(UnsafeGuaranteedEndI)) {
+    auto *CurInst = &*It++;
+    if (CurInst != Retain &&
+        (isa<StrongRetainInst>(CurInst) || isa<RetainValueInst>(CurInst)) &&
+        RCFI.getRCIdentityRoot(CurInst->getOperand(0)) ==
+            SILValue(UnsafeGuaranteedI)) {
+      CandidateRetain = CurInst;
+      continue;
+    }
+    if (!CurInst->mayHaveSideEffects())
+      continue;
+
+    if (isa<DebugValueInst>(CurInst) || isa<DebugValueAddrInst>(CurInst))
+      continue;
+
+    if (isa<ApplyInst>(CurInst) || isa<PartialApplyInst>(CurInst))
+      continue;
+
+    if (CandidateRetain != nullptr && CurInst != Release &&
+        (isa<StrongReleaseInst>(CurInst) || isa<ReleaseValueInst>(CurInst)) &&
+        RCFI.getRCIdentityRoot(CurInst->getOperand(0)) ==
+            SILValue(UnsafeGuaranteedI)) {
+      // Delete the retain/release pair.
+      InstsToDelete.push_back(CandidateRetain);
+      InstsToDelete.push_back(CurInst);
+    }
+
+    // Otherwise, reset our scan.
+    CandidateRetain = nullptr;
+  }
+  for (auto *Inst: InstsToDelete)
+    Inst->eraseFromParent();
+}
+
 /// Remove retain/release pairs around builtin "unsafeGuaranteed" instruction
 /// sequences.
 static bool removeGuaranteedRetainReleasePairs(SILFunction &F,
@@ -154,6 +204,12 @@ static bool removeGuaranteedRetainReleasePairs(SILFunction &F,
       // Okay we found a post dominating release. Let's remove the
       // retain/unsafeGuaranteed/release combo.
       //
+      // Before we do this check whether there are any pairs of retain releases
+      // we can safely remove.
+      tryRemoveRetainReleasePairsBetween(RCIA, UnsafeGuaranteedI,
+                                         LastRetainInst, LastRelease,
+                                         UnsafeGuaranteedEndI);
+
       LastRetainInst->eraseFromParent();
       LastRelease->eraseFromParent();
       UnsafeGuaranteedEndI->eraseFromParent();

diff --git a/test/SILOptimizer/unsafe_guaranteed_peephole.sil b/test/SILOptimizer/unsafe_guaranteed_peephole.sil
@@ -542,3 +542,85 @@ bb2(%9: $Error):
   strong_release %5 : $Foo
   throw  %9: $Error
 }
+
+// CHECK-LABEL: sil @testUnsafeGuaranteed_simple_retain_release_pair
+// CHECK: bb0([[P:%.*]] : $Foo):
+// CHECK-NOT: retain
+// CHECK-NOT: unsafeGuaranteed
+// CHECK:   [[M:%.*]] = class_method [[P]] : $Foo, #Foo.beep
+// CHECK:   apply [[M]]([[P]])
+// CHECK-NOT: release
+// CHECK-NOT: unsafeGuaranteedEnd
+// CHECK:   [[T:%.*]] = tuple ()
+// CHECK:   return [[T]]
+// CHECK: }
+sil @testUnsafeGuaranteed_simple_retain_release_pair : $@convention(method) (@guaranteed Foo) -> () {
+bb0(%0 : $Foo):
+  strong_retain %0 : $Foo
+  %4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
+  %5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
+  %6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
+  %19 = class_method %5 : $Foo, #Foo.beep!1 : (Foo) -> () -> (), $@convention(method) (@guaranteed Foo) -> ()
+  retain_value %4 : $(Foo, Builtin.Int8)
+  %20 = apply %19(%5) : $@convention(method) (@guaranteed Foo) -> ()
+  strong_release %5 : $Foo
+  strong_release %5 : $Foo
+  %16 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
+  %17 = tuple ()
+  return %17 : $()
+}
+
+// CHECK-LABEL: sil @testUnsafeGuaranteed_simple_retain_release_pair2
+// CHECK: bb0([[P:%.*]] : $Foo):
+// CHECK-NOT: retain
+// CHECK-NOT: unsafeGuaranteed
+// CHECK:   [[M:%.*]] = class_method [[P]] : $Foo, #Foo.beep
+// CHECK:   apply [[M]]([[P]])
+// CHECK-NOT: release
+// CHECK-NOT: unsafeGuaranteedEnd
+// CHECK:   [[T:%.*]] = tuple ()
+// CHECK:   return [[T]]
+// CHECK: }
+sil @testUnsafeGuaranteed_simple_retain_release_pair2 : $@convention(method) (@guaranteed Foo) -> () {
+bb0(%0 : $Foo):
+  strong_retain %0 : $Foo
+  %4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
+  %5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
+  %6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
+  %19 = class_method %5 : $Foo, #Foo.beep!1 : (Foo) -> () -> (), $@convention(method) (@guaranteed Foo) -> ()
+  retain_value %4 : $(Foo, Builtin.Int8)
+  %20 = apply %19(%5) : $@convention(method) (@guaranteed Foo) -> ()
+  strong_release %5 : $Foo
+  %16 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
+  strong_release %5 : $Foo
+  %17 = tuple ()
+  return %17 : $()
+}
+
+
+// CHECK-LABEL: sil @testUnsafeGuaranteed_simple_retain_release_pair_not_safe
+// CHECK: bb0([[P:%.*]] : $Foo):
+// CHECK:   [[M:%.*]] = class_method [[P]] : $Foo, #Foo.beep
+// CHECK: retain
+// CHECK: release
+// CHECK:   apply [[M]]([[P]])
+// CHECK: release
+// CHECK:   [[T:%.*]] = tuple ()
+// CHECK:   return [[T]]
+// CHECK: }
+sil @testUnsafeGuaranteed_simple_retain_release_pair_not_safe : $@convention(method) (@guaranteed Foo) -> () {
+bb0(%0 : $Foo):
+  strong_retain %0 : $Foo
+  %4 = builtin "unsafeGuaranteed"<Foo>(%0 : $Foo) : $(Foo, Builtin.Int8)
+  %5 = tuple_extract %4 : $(Foo, Builtin.Int8), 0
+  %6 = tuple_extract %4 : $(Foo, Builtin.Int8), 1
+  %19 = class_method %5 : $Foo, #Foo.beep!1 : (Foo) -> () -> (), $@convention(method) (@guaranteed Foo) -> ()
+  retain_value %4 : $(Foo, Builtin.Int8)
+  strong_release %0 : $Foo
+  %20 = apply %19(%5) : $@convention(method) (@guaranteed Foo) -> ()
+  strong_release %5 : $Foo
+  strong_release %5 : $Foo
+  %16 = builtin "unsafeGuaranteedEnd"(%6 : $Builtin.Int8) : $()
+  %17 = tuple ()
+  return %17 : $()
+}