Wire up improved epilogue release matcher with function signature

trentxintong · trentxintong · commit 670df6dc9d5e · 2016-02-15T16:10:40.000-08:00
optimization.

We get some improvements on the # of parameters converted to guanrateed
from owned on the stdlib.

before
======
103 sil-function-signature-opts      - Total owned args -&gt; guaranteed args

after
======
118 sil-function-signature-opts      - Total owned args -&gt; guaranteed args

I see the following improvements by running benchmarks with and without this
change. Only difference &gt;=1.05X

ErrorHandling         8154             7497      -657     -8.1%     1.09x
LinkedList            9973             9529      -444     -4.5%     1.05x
ObjectAllocation      239              222       -17     -7.1%     1.08x
RC4                   23167            21993     -1174     -5.1%     1.05x (!)

This is part of rdar://22380547
diff --git a/lib/SILOptimizer/IPO/FunctionSignatureOpts.cpp b/lib/SILOptimizer/IPO/FunctionSignatureOpts.cpp
@@ -61,17 +61,30 @@ static bool isRelease(SILInstruction *I) {
   }
 }
 
+/// Returns true if LHS and RHS contain identical set of releases.
+static bool hasIdenticalReleases(ReleaseList LHS, ReleaseList RHS) {
+  llvm::DenseSet<SILInstruction *> Releases;
+  if (LHS.size() != RHS.size())
+    return false;
+  for (auto &X : LHS) 
+    Releases.insert(X);
+  for (auto &X : RHS) 
+    if (Releases.find(X) == Releases.end())
+      return false;
+  return true;
+}
+
 /// Returns .Some(I) if I is a release that is the only non-debug instruction
 /// with side-effects in the use-def graph originating from Arg. Returns
 /// .Some(nullptr), if all uses from the arg were either debug insts or do not
 /// have side-effects. Returns .None if there were any non-release instructions
 /// with side-effects in the use-def graph from Arg or if there were multiple
 /// release instructions with side-effects in the use-def graph from Arg.
-static llvm::Optional<NullablePtr<SILInstruction>>
+static llvm::Optional<ReleaseList>
 getNonTrivialNonDebugReleaseUse(SILArgument *Arg) {
   llvm::SmallVector<SILInstruction *, 8> Worklist;
   llvm::SmallPtrSet<SILInstruction *, 8> SeenInsts;
-  llvm::Optional<SILInstruction *> Result;
+  ReleaseList Result;
 
   for (Operand *I : getNonDebugUses(SILValue(Arg)))
     Worklist.push_back(I->getUser());
@@ -91,12 +104,8 @@ getNonTrivialNonDebugReleaseUse(SILArgument *Arg) {
       if (!isRelease(U))
         return None;
 
-      // If we have already seen a release of some sort, bail.
-      if (Result.hasValue())
-        return None;
-
       // Otherwise, set result to that value.
-      Result = U;
+      Result.push_back(U);
       continue;
     }
 
@@ -105,9 +114,7 @@ getNonTrivialNonDebugReleaseUse(SILArgument *Arg) {
       Worklist.push_back(I->getUser());
   }
 
-  if (!Result.hasValue())
-    return NullablePtr<SILInstruction>();
-  return NullablePtr<SILInstruction>(Result.getValue());
+  return Result;
 }
 
 //===----------------------------------------------------------------------===//
@@ -138,11 +145,11 @@ struct ArgumentDescriptor {
   /// If non-null, this is the release in the return block of the callee, which
   /// is associated with this parameter if it is @owned. If the parameter is not
   /// @owned or we could not find such a release in the callee, this is null.
-  SILInstruction *CalleeRelease;
+  ReleaseList CalleeRelease;
 
   /// The same as CalleeRelease, but the release in the throw block, if it is a
   /// function which has a throw block.
-  SILInstruction *CalleeReleaseInThrowBlock;
+  ReleaseList CalleeReleaseInThrowBlock;
 
   /// The projection tree of this arguments.
   ProjectionTree ProjTree;
@@ -239,9 +246,9 @@ void ArgumentDescriptor::computeOptimizedInterfaceParams(
   // If we cannot explode this value, handle callee release and return.
   if (!shouldExplode()) {
     DEBUG(llvm::dbgs() << "            ProjTree cannot explode arg.\n");
-    // If we found a release in the callee in the last BB on an @owned
+    // If we found releases in the callee in the last BB on an @owned
     // parameter, change the parameter to @guaranteed and continue...
-    if (CalleeRelease) {
+    if (!CalleeRelease.empty()) {
       DEBUG(llvm::dbgs() << "            Has callee release.\n");
       assert(ParameterInfo.getConvention() ==
                  ParameterConvention::Direct_Owned &&
@@ -277,8 +284,8 @@ void ArgumentDescriptor::computeOptimizedInterfaceParams(
     // If Ty is guaranteed, just pass it through.
     ParameterConvention Conv = ParameterInfo.getConvention();
     if (Conv == ParameterConvention::Direct_Guaranteed) {
-      assert(!CalleeRelease && "Guaranteed parameter should not have a callee "
-                               "release.");
+      assert(CalleeRelease.empty() && "Guaranteed parameter should not have a "
+                                      "callee release.");
       SILParameterInfo NewInfo(Ty.getSwiftRValueType(),
                                ParameterConvention::Direct_Guaranteed);
       Out.push_back(NewInfo);
@@ -289,7 +296,7 @@ void ArgumentDescriptor::computeOptimizedInterfaceParams(
     // guaranteed.
     assert(ParameterInfo.getConvention() == ParameterConvention::Direct_Owned &&
            "Can only transform @owned => @guaranteed in this code path");
-    if (CalleeRelease) {
+    if (!CalleeRelease.empty()) {
       SILParameterInfo NewInfo(Ty.getSwiftRValueType(),
                                ParameterConvention::Direct_Guaranteed);
       Out.push_back(NewInfo);
@@ -344,16 +351,10 @@ unsigned ArgumentDescriptor::updateOptimizedBBArgs(SILBuilder &Builder,
     // do need the instruction to be non-null.
     //
     // TODO: This should not be necessary.
-    if (CalleeRelease) {
-      SILType CalleeReleaseTy = CalleeRelease->getOperand(0)->getType();
-      CalleeRelease->setOperand(
+    for (auto &X : CalleeRelease) {
+      SILType CalleeReleaseTy = X->getOperand(0)->getType();
+      X->setOperand(
           0, SILUndef::get(CalleeReleaseTy, Builder.getModule()));
-
-      // TODO: Currently we cannot mark arguments as dead if they are released
-      // in a throw block. But as soon as we can do this, we have to handle
-      // CalleeReleaseInThrowBlock as well.
-      assert(!CalleeReleaseInThrowBlock &&
-             "released arg in throw block cannot be dead");
     }
 
     // We should be able to recursively delete all of the remaining
@@ -533,7 +534,7 @@ bool ParameterAnalyzer::analyze() {
 
     // If this argument is not ABI required and has no uses except for debug
     // instructions, remove it.
-    if (!isABIRequired && OnlyRelease && OnlyRelease.getValue().isNull()) {
+    if (!isABIRequired && OnlyRelease && OnlyRelease.getValue().empty()) {
       A.IsDead = true;
       HaveOptimizedArg = true;
       ++NumDeadArgsEliminated;
@@ -542,21 +543,22 @@ bool ParameterAnalyzer::analyze() {
     // See if we can find a ref count equivalent strong_release or release_value
     // at the end of this function if our argument is an @owned parameter.
     if (A.hasConvention(ParameterConvention::Direct_Owned)) {
-      if (auto *Release = ArgToReturnReleaseMap.getSingleReleaseForArgument(A.Arg)) {
-        SILInstruction *ReleaseInThrow = nullptr;
+      auto Releases = ArgToReturnReleaseMap.getReleasesForArgument(A.Arg);
+      if (!Releases.empty()) {
 
         // If the function has a throw block we must also find a matching
         // release in the throw block.
-        if (!ArgToThrowReleaseMap.hasBlock() ||
-            (ReleaseInThrow = ArgToThrowReleaseMap.getSingleReleaseForArgument(A.Arg))) {
+        auto ReleasesInThrow = ArgToThrowReleaseMap.getReleasesForArgument(A.Arg);
+        if (!ArgToThrowReleaseMap.hasBlock() || !ReleasesInThrow.empty()) {
 
           // TODO: accept a second release in the throw block to let the
           // argument be dead.
-          if (OnlyRelease && OnlyRelease.getValue().getPtrOrNull() == Release) {
+          if (OnlyRelease && hasIdenticalReleases(OnlyRelease.getValue(), Releases)) {
             A.IsDead = true;
           }
-          A.CalleeRelease = Release;
-          A.CalleeReleaseInThrowBlock = ReleaseInThrow;
+
+          A.CalleeRelease = Releases;
+          A.CalleeReleaseInThrowBlock = ReleasesInThrow;
           HaveOptimizedArg = true;
           ++NumOwnedConvertedToGuaranteed;
         }
@@ -600,7 +602,7 @@ std::string ParameterAnalyzer::getOptimizedName() const {
 
     // If we have an @owned argument and found a callee release for it,
     // convert the argument to guaranteed.
-    if (Arg.CalleeRelease) {
+    if (!Arg.CalleeRelease.empty()) {
       FSSM.setArgumentOwnedToGuaranteed(i);
     }
 
@@ -721,7 +723,7 @@ static void rewriteApplyInstToCallNewFunction(SignatureOptimizer &Optimizer,
       // If we have any arguments that were consumed but are now guaranteed,
       // insert a release_value in the error block.
       for (auto &ArgDesc : ArgDescs) {
-        if (!ArgDesc.CalleeRelease)
+        if (ArgDesc.CalleeRelease.empty())
           continue;
         Builder.createReleaseValue(Loc, FAS.getArgument(ArgDesc.Index));
       }
@@ -733,7 +735,7 @@ static void rewriteApplyInstToCallNewFunction(SignatureOptimizer &Optimizer,
     // If we have any arguments that were consumed but are now guaranteed,
     // insert a release_value.
     for (auto &ArgDesc : ArgDescs) {
-      if (!ArgDesc.CalleeRelease)
+      if (ArgDesc.CalleeRelease.empty())
         continue;
       Builder.createReleaseValue(Loc, FAS.getArgument(ArgDesc.Index));
     }
@@ -784,7 +786,7 @@ static void createThunkBody(SILBasicBlock *BB, SILFunction *NewF,
     // insert a release_value in the error block.
     Builder.setInsertionPoint(ErrorBlock);
     for (auto &ArgDesc : ArgDescs) {
-      if (!ArgDesc.CalleeRelease)
+      if (ArgDesc.CalleeRelease.empty())
         continue;
       Builder.createReleaseValue(Loc, BB->getBBArg(ArgDesc.Index));
     }
@@ -801,7 +803,7 @@ static void createThunkBody(SILBasicBlock *BB, SILFunction *NewF,
   // If we have any arguments that were consumed but are now guaranteed,
   // insert a release_value.
   for (auto &ArgDesc : ArgDescs) {
-    if (!ArgDesc.CalleeRelease)
+    if (ArgDesc.CalleeRelease.empty())
       continue;
     Builder.createReleaseValue(Loc, BB->getBBArg(ArgDesc.Index));
   }
@@ -911,12 +913,10 @@ static bool optimizeFunctionSignature(llvm::BumpPtrAllocator &BPA,
   //
   // TODO: If more stuff needs to be placed here, refactor into its own method.
   for (auto &A : Optimizer.getArgDescList()) {
-    if (A.CalleeRelease) {
-      A.CalleeRelease->eraseFromParent();
-      if (A.CalleeReleaseInThrowBlock) {
-        A.CalleeReleaseInThrowBlock->eraseFromParent();
-      }
-    }
+    for (auto &X : A.CalleeRelease) 
+      X->eraseFromParent();
+    for (auto &X : A.CalleeReleaseInThrowBlock) 
+      X->eraseFromParent();
   }
 
   // Rewrite all apply insts calling F to call NewF. Update each call site as
diff --git a/test/SILOptimizer/functionsigopts.sil b/test/SILOptimizer/functionsigopts.sil
@@ -4,6 +4,38 @@
 import Builtin
 import Swift
 
+/////////////////////
+// Data Structures //
+/////////////////////
+
+class foo {
+  var a: Int
+   deinit 
+  init()
+}
+
+class bar {
+  var start: foo
+  var end: foo
+   deinit 
+  init()
+}
+
+struct baz {
+  var start: foo
+  var end: foo
+  init()
+}
+
+struct boo {
+  var tbaz = baz()
+  var a = 0
+  init()
+}
+
+
+sil @use_Int : $@convention(thin) (Int) -> ()
+
 // We do this check separately to ensure that this does not occur anywhere in
 // the output for the file. (Maybe this is an interesting enough feature to add to FileCheck?).
 
@@ -32,6 +64,44 @@ bb3:
   return %4 : $()
 }
 
+// Make sure %0 is a dead argument.
+//
+// CHECK-LABEL: sil [thunk] @exploded_release_to_dead_argument
+// CHECK: bb0([[INPUT_ARG0:%[0-9]+]] : $boo):
+// CHECK: [[IN1:%.*]] = function_ref @_TTSf4dg__exploded_release_to_dead_argument 
+// CHECK: apply [[IN1]]()
+// CHECK: release_value [[INPUT_ARG0]] 
+sil @exploded_release_to_dead_argument : $@convention(thin) (@owned boo) -> () {
+bb0(%0 : $boo):
+  %1 = struct_extract %0 : $boo, #boo.tbaz
+  %2 = struct_extract %1 : $baz, #baz.start
+  %3 = struct_extract %1 : $baz, #baz.end
+  release_value  %2 : $foo
+  release_value  %3 : $foo
+  %4 = tuple ()
+  return %4 : $()
+}
+
+// Make sure %0 is not a dead argument, but gets converted to a guaranteed arg.
+//
+// CHECK-LABEL: sil [thunk] @exploded_release_to_guaranteed_param
+// CHECK: bb0([[INPUT_ARG0:%[0-9]+]] : $boo):
+// CHECK: [[IN1:%.*]] = function_ref @_TTSf4gs__exploded_release_to_guaranteed_param 
+// CHECK: release_value [[INPUT_ARG0]] 
+sil @exploded_release_to_guaranteed_param : $@convention(thin) (@owned boo) -> () {
+bb0(%0 : $boo):
+  %1 = struct_extract %0 : $boo, #boo.tbaz
+  %2 = struct_extract %1 : $baz, #baz.start
+  %3 = struct_extract %1 : $baz, #baz.end
+  %4 = struct_extract %0 : $boo, #boo.a
+  %5 = function_ref @use_Int : $@convention(thin) (Int) -> ()
+  apply %5(%4) : $@convention(thin) (Int) -> ()
+  release_value  %2 : $foo
+  release_value  %3 : $foo
+  %6 = tuple ()
+  return %6 : $()
+}
+
 // CHECK-LABEL: sil [fragile] [thunk] @dead_arg_with_callsites : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
 // CHECK: bb0
 // CHECK-NEXT: function_ref
@@ -122,6 +192,25 @@ bb0(%0 : $Builtin.NativeObject, %1 : $Builtin.NativeObject):
   return %5 : $()
 }
 
+sil [fragile] @exploded_release_to_guaranteed_param_callsite : $@convention(thin) (@owned boo) -> () {
+bb0(%0 : $boo):
+  %2 = function_ref @exploded_release_to_guaranteed_param : $@convention(thin) (@owned boo) -> ()
+  retain_value %0 : $boo
+  %4 = apply %2(%0) : $@convention(thin) (@owned boo) -> ()
+  %5 = tuple()
+  return %5 : $()
+}
+
+
+sil [fragile] @exploded_release_to_dead_param_callsite : $@convention(thin) (@owned boo) -> () {
+bb0(%0 : $boo):
+  %2 = function_ref @exploded_release_to_dead_argument : $@convention(thin) (@owned boo) -> ()
+  retain_value %0 : $boo
+  %4 = apply %2(%0) : $@convention(thin) (@owned boo) -> ()
+  %5 = tuple()
+  return %5 : $()
+}
+
 //========================================
 // @owned => @guaranteed with error result
 
@@ -446,6 +535,13 @@ bb0(%0 : $Builtin.Int32):
   return %9999 : $()
 }
 
+// CHECK-LABEL: sil @_TTSf4dg__exploded_release_to_dead_argument
+
+// CHECK-LABEL: sil @_TTSf4gs__exploded_release_to_guaranteed_param
+// CHECK: bb0([[INPUT_ARG0:%[0-9]+]] : $foo, [[INPUT_ARG1:%[0-9]+]] : $foo, [[INPUT_ARG02:%[0-9]+]] : $Int):
+// CHECK-NOT: strong_release
+// CHECK: return
+
 // Check that we specialized this function by removing the dead argument and
 // copied everything appropriately.
 
