Merge pull request #1971 from trentxintong/FSO

Change FSO argument explosion heuristic and add some TODOs

Author: trentxintong

include/swift/SILOptimizer/Analysis/ARCAnalysis.h

@@ -186,6 +186,10 @@ class ConsumedArgToEpilogueReleaseMatcher {
   RCIdentityFunctionInfo *RCFI;
   ExitKind Kind;
   llvm::SmallMapVector<SILArgument *, ReleaseList, 8> ArgInstMap;
+
+  /// Set to true if we found some releases but not all for the argument.
+  llvm::DenseSet<SILArgument *> FoundSomeReleases;
+
   bool HasBlock = false;
 
   /// Return true if we have seen releases to part or all of \p Derived in
@@ -221,6 +225,12 @@ class ConsumedArgToEpilogueReleaseMatcher {
 
   bool hasBlock() const { return HasBlock; }
 
+  /// Return true if we've found some epilgoue releases for the argument
+  /// but not all.
+  bool hasSomeReleasesForArgument(SILArgument *Arg) {
+    return FoundSomeReleases.find(Arg) != FoundSomeReleases.end();
+  }
+
   bool isSingleRelease(SILArgument *Arg) const {
     auto Iter = ArgInstMap.find(Arg);
     assert(Iter != ArgInstMap.end() && "Failed to get release list for argument");

include/swift/SILOptimizer/Utils/FunctionSignatureOptUtils.h

@@ -123,11 +123,22 @@ struct ArgumentDescriptor {
   }
 
   /// Return true if it's both legal and a good idea to explode this argument.
-  bool shouldExplode() const {
+  bool shouldExplode(ConsumedArgToEpilogueReleaseMatcher &ERM) const {
     // We cannot optimize the argument.
     if (!canOptimizeLiveArg())
       return false;
 
+    // If this argument is @owned and we can not find all the releases for it
+    // try to explode it, maybe we can find some of the releases and O2G some
+    // of its components.
+    //
+    // This is a potentially a very profitable optimization. Ignore other
+    // heuristics.
+    if (hasConvention(SILArgumentConvention::Direct_Owned)) {
+      if (ERM.hasSomeReleasesForArgument(Arg))
+        return true;
+    }
+
     // See if the projection tree consists of potentially multiple levels of
     // structs containing one field. In such a case, there is no point in
     // exploding the argument.

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

@@ -739,6 +739,10 @@ processMatchingReleases() {
     if (releaseArgument(Arg.second, Arg.first))
       continue;
 
+    // OK. we did find some epilogue releases, just not all.
+    if (!Arg.second.empty())
+      FoundSomeReleases.insert(Arg.first);
+
     ArgToRemove.insert(Arg.first);
   }
 

lib/SILOptimizer/Transforms/FunctionSignatureOpts.cpp

@@ -9,6 +9,16 @@
 // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+//
+// I looked through many functions in the Stdlib. This is what i feel how FSO
+// can be further improved.
+//
+// TODO: Most of the O2G for function arguments are done. but we are still
+// missing a lot of things on O2G for return result. We maybe able to improve
+// the ConsumedResultToEpilogueRetainMatcher. Right now it is kept conservative
+// in present of complex control flows and release instructions.
+//
+//===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "sil-function-signature-opt"
 #include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
@@ -539,6 +549,7 @@ class FunctionSignatureOpts : public SILFunctionTransform {
     auto *AA = PM->getAnalysis<AliasAnalysis>();
     auto *CA = PM->getAnalysis<CallerAnalysis>();
 
+
     SILFunction *F = getFunction();
     llvm::BumpPtrAllocator Allocator;
     FunctionSignatureInfo FSI(F, Allocator, AA, RCIA->get(F));

lib/SILOptimizer/Utils/FunctionSignatureOptUtils.cpp

@@ -260,7 +260,7 @@ bool FunctionSignatureInfo::analyzeParameters() {
     bool HaveOptimizedArg = false;
 
     // Whether we will explode the argument or not.
-    A.Explode = A.shouldExplode();
+    A.Explode = A.shouldExplode(ArgToReturnReleaseMap);
 
     bool isABIRequired = isArgumentABIRequired(Args[i]);
     auto OnlyRelease = getNonTrivialNonDebugReleaseUse(Args[i]);

test/SILOptimizer/functionsigopts.sil

@@ -42,8 +42,64 @@ struct lotsoffield {
   init()
 }
 
+struct goo {
+  var left : foo
+  var right : foo
+  var top : foo
+  var bottom : foo
+}
+
 sil @use_Int : $@convention(thin) (Int) -> ()
 
+
+// CHECK-LABEL: sil [thunk] [always_inline] @argument_with_incomplete_epilogue_release
+// CHECK: [[IN2:%.*]] = struct_extract [[IN1:%.*]] : $goo, #goo.top
+// CHECK: function_ref @_TTSf4g_n___TTSf4s__argument_with_incomplete_epilogue_release : $@convention(thin) (@guaranteed foo, @owned foo) -> ()
+// CHECK: release_value [[IN2]]
+sil @argument_with_incomplete_epilogue_release : $@convention(thin) (@owned goo) -> () {
+bb0(%0 : $goo):
+  // make inline costs = 2
+  %c1 = builtin "assert_configuration"() : $Builtin.Int32
+  %c2 = builtin "assert_configuration"() : $Builtin.Int32
+  %c3 = builtin "assert_configuration"() : $Builtin.Int32
+  %c4 = builtin "assert_configuration"() : $Builtin.Int32
+  %c5 = builtin "assert_configuration"() : $Builtin.Int32
+  %c6 = builtin "assert_configuration"() : $Builtin.Int32
+  %c7 = builtin "assert_configuration"() : $Builtin.Int32
+  %c8 = builtin "assert_configuration"() : $Builtin.Int32
+  %c9 = builtin "assert_configuration"() : $Builtin.Int32
+  %c10 = builtin "assert_configuration"() : $Builtin.Int32
+  %c11 = builtin "assert_configuration"() : $Builtin.Int32
+  %c12 = builtin "assert_configuration"() : $Builtin.Int32
+  %c13 = builtin "assert_configuration"() : $Builtin.Int32
+  %c14 = builtin "assert_configuration"() : $Builtin.Int32
+  %c15 = builtin "assert_configuration"() : $Builtin.Int32
+  %c16 = builtin "assert_configuration"() : $Builtin.Int32
+  %c17 = builtin "assert_configuration"() : $Builtin.Int32
+  %c18 = builtin "assert_configuration"() : $Builtin.Int32
+  %c19 = builtin "assert_configuration"() : $Builtin.Int32
+  %c20 = builtin "assert_configuration"() : $Builtin.Int32
+  %c21 = builtin "assert_configuration"() : $Builtin.Int32
+  %c22 = builtin "assert_configuration"() : $Builtin.Int32
+
+
+
+  %1 = struct_extract %0 : $goo, #goo.top
+  %2 = ref_element_addr %1 : $foo, #foo.a
+  %3 = load %2 : $*Int
+  %4 = function_ref @use_Int : $@convention(thin) (Int) -> ()
+  apply %4(%3) : $@convention(thin) (Int) -> ()
+
+  %5 = struct_extract %0 : $goo, #goo.bottom
+  %6 = ref_element_addr %5 : $foo, #foo.a
+  %7 = load %6 : $*Int
+  apply %4(%7) : $@convention(thin) (Int) -> ()
+
+  release_value %1 : $foo
+  %8 = tuple ()
+  return %8 : $()
+}
+
 // 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?).
 
@@ -174,6 +230,7 @@ bb0(%0 : $boo):
   return %4 : $(Int, Int)
 }
 
+
 // CHECK-LABEL: sil [fragile] @dead_arg_no_callsites : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
 // CHECK-NOT: function_ref @_TTSf4d_n__dead_arg_no_callsites
 sil [fragile] @dead_arg_no_callsites : $@convention(thin) (Builtin.NativeObject, Builtin.NativeObject) -> () {
@@ -720,7 +777,13 @@ bb0(%0 : $boo):
   return %5 : $()
 }
 
-
+sil [fragile] @argument_with_incomplete_epilogue_release_callsite : $@convention(thin) (@owned goo) -> () {
+bb0(%0 : $goo):
+  %2 = function_ref @argument_with_incomplete_epilogue_release : $@convention(thin) (@owned goo) -> ()
+  %4 = apply %2(%0) : $@convention(thin) (@owned goo) -> ()
+  %5 = tuple()
+  return %5 : $()
+}
 
 
 //========================================