Change FSO heuristic.

FSO functions that have high potential but does not have caller inside
current module.

The thunk can then be inlined into the module calling the function and
the function would get the benefit of FSO.

The heuristic for selecting such function is
1. Have no indirect caller. This would introduce a thunk.
2. Have potential to give better performance. i.e. function argument can
be O2G.

Regression
TEST                                                    | OLD_MIN | NEW_MIN | DELTA (%) | SPEEDUP
---                                                     | ---     | ---     | ---       | ---
BenchLangCallingCFunction                               | 184     | 211     | +14.7%    | **0.87x**
Calculator                                              | 55      | 59      | +7.3%     | **0.93x**
DeadArray                                               | 687     | 741     | +7.9%     | **0.93x**
MonteCarloPi                                            | 39275   | 41669   | +6.1%     | **0.94x**

Improvement
TEST                                                    | OLD_MIN | NEW_MIN | DELTA (%) | SPEEDUP
---                                                     | ---     | ---     | ---       | ---
LuhnAlgoLazy                                            | 2478    | 2327    | -6.1%     | **1.06x**
OpenClose                                               | 54      | 51      | -5.6%     | **1.06x**
SortLettersInPlace                                      | 1016    | 946     | -6.9%     | **1.07x**
ObjectiveCBridgeFromNSDictionaryAnyObjectToStringForced | 149993  | 139755  | -6.8%     | **1.07x**
Phonebook                                               | 9666    | 8992    | -7.0%     | **1.07x**
ObjectiveCBridgeFromNSDictionaryAnyObjectToString       | 222713  | 206538  | -7.3%     | **1.08x**
LuhnAlgoEager                                           | 2393    | 2226    | -7.0%     | **1.08x**
Dictionary                                              | 1307    | 1196    | -8.5%     | **1.09x**
JSONHelperDeserialize                                   | 3808    | 3492    | -8.3%     | **1.09x**
StdlibSort                                              | 7310    | 4084    | -44.1%    | **1.79x**

I see 0.15% increase in code size for Benchmark_O.

Thanks @gottesmm for suggesting this opportunity.

rdar://25345056

Author: trentxintong

include/swift/SILOptimizer/Utils/FunctionSignatureOptUtils.h

@@ -172,6 +172,12 @@ struct ResultDescriptor {
 };
 
 class FunctionSignatureInfo {
+  /// Should this function be optimized.
+  bool ShouldOptimize;
+
+  /// Optimizing this function may lead to good performance potential.
+  bool HighlyProfitable;
+
   /// Function currently analyzing.
   SILFunction *F;
 
@@ -204,10 +210,15 @@ class FunctionSignatureInfo {
 public:
   FunctionSignatureInfo(SILFunction *F, llvm::BumpPtrAllocator &BPA,
                         AliasAnalysis *AA, RCIdentityFunctionInfo *RCFI) :
-  F(F), Allocator(BPA), AA(AA), RCFI(RCFI),
-  MayBindDynamicSelf(computeMayBindDynamicSelf(F)) {}
+  ShouldOptimize(false), HighlyProfitable(false), F(F), Allocator(BPA),
+  AA(AA), RCFI(RCFI), MayBindDynamicSelf(computeMayBindDynamicSelf(F)) {
+    analyze();
+  }
+
+  bool shouldOptimize() const { return ShouldOptimize; }
+  bool profitableOptimize() const { return HighlyProfitable; }
 
-  bool analyze();
+  void analyze();
   bool analyzeParameters();
   bool analyzeResult();
 

lib/SILOptimizer/Transforms/FunctionSignatureOpts.cpp

@@ -486,10 +486,6 @@ static SILFunction*
 createOptimizedFunction(RCIdentityFunctionInfo *RCIA,
                         FunctionSignatureInfo *FSI,
                         AliasAnalysis *AA, SILFunction *F) {
-  // Analyze function arguments. If there is no work to be done, exit early.
-  if (!FSI->analyze()) 
-    return nullptr;
-
   // This is the new function name.
   auto NewFName = FSI->getOptimizedName();
 
@@ -551,10 +547,21 @@ class FunctionSignatureOpts : public SILFunctionTransform {
     if (!F->shouldOptimize())
       return;
 
-    // If this function does not have a caller in the current module. We do not
-    // function signature specialize it.
-    if (!CA->hasCaller(F))
-      return;
+    // If there is no opportunity on the signature, simply return.
+    if (!FSI.shouldOptimize())
+     return;
+
+    // If this function does not have a caller in the current module.
+    if (!CA->hasCaller(F)) {
+      // If this function maybe called indirectly, e.g. from virtual table
+      // do not function signature specialize it, as this will introduce a thunk.
+      if (canBeCalledIndirectly(F->getRepresentation()))
+        return;
+      // if its not highly profitable to optimize this function. We do not
+      // function signature specialize it.
+      if (!FSI.profitableOptimize())
+        return;
+    }
 
     // Check the signature of F to make sure that it is a function that we
     // can specialize. These are conditions independent of the call graph.

lib/SILOptimizer/Utils/FunctionSignatureOptUtils.cpp

@@ -345,11 +345,19 @@ bool FunctionSignatureInfo::analyzeResult() {
 /// This function goes through the arguments of F and sees if we have anything
 /// to optimize in which case it returns true. If we have nothing to optimize,
 /// it returns false.
-bool FunctionSignatureInfo::analyze() {
+void FunctionSignatureInfo::analyze() {
   // Compute the signature optimization.
   bool OptimizedParams = analyzeParameters();
   bool OptimizedResult = analyzeResult();
-  return OptimizedParams || OptimizedResult;
+  ShouldOptimize = OptimizedParams || OptimizedResult;
+  // We set this function to highly profitable if we have a O2G on one of its
+  // parameters or results.
+  for (auto &X : ArgDescList) {
+    HighlyProfitable |= !X.CalleeRelease.empty();
+  }
+  for (auto &X : ResultDescList) {
+    HighlyProfitable |= !X.CalleeRetain.empty();
+  }
 }
 
 //===----------------------------------------------------------------------===//

test/SILOptimizer/cast_folding_objc.swift

@@ -55,33 +55,6 @@ func test0() -> Bool {
   return cast0(NSNumber(integer:1))
 }
 
-// Check that compiler understands that this cast always succeeds.
-// Since it is can be statically proven that NSString is bridgeable to String,
-// _forceBridgeFromObjectiveC from String should be invoked instead of
-// a more general, but less effective swift_bridgeNonVerbatimFromObjectiveC, which
-// also performs conformance checks at runtime.
-// CHECK-LABEL: sil [noinline] @_TF17cast_folding_objc30testBridgedCastFromObjCtoSwiftFCSo8NSStringSS
-// CHECK-NOT: {{ cast}}
-// CHECK: metatype $@thick String.Type
-// CHECK: function_ref @_TTWSSs21_ObjectiveCBridgeable10FoundationZFS_26_forceBridgeFromObjectiveCfTwx15_ObjectiveCType6resultRGSqx__T_
-// CHECK: apply
-// CHECK: return
-@inline(never)
-public func testBridgedCastFromObjCtoSwift(ns: NSString) -> String {
-  return ns as String
-}
-
-// Check that compiler understands that this cast always succeeds
-// CHECK-LABEL: sil [noinline] @_TF17cast_folding_objc30testBridgedCastFromSwiftToObjCFSSCSo8NSString
-// CHECK-NOT: {{ cast}}
-// CHECK: function_ref @_TFE10FoundationSS19_bridgeToObjectiveC
-// CHECK: apply
-// CHECK: return
-@inline(never)
-public func testBridgedCastFromSwiftToObjC(s: String) -> NSString {
-  return s as NSString
-}
-
 // Check that this cast does not get eliminated, because
 // the compiler does not statically know if this object
 // is NSNumber can be converted into Int.
@@ -270,3 +243,32 @@ public func testCastEveryToNonClassType<T>(o: T) -> Int.Type {
 }
 
 print("test0=\(test0())")
+
+// Check that compiler understands that this cast always succeeds.
+// Since it is can be statically proven that NSString is bridgeable to String,
+// _forceBridgeFromObjectiveC from String should be invoked instead of
+// a more general, but less effective swift_bridgeNonVerbatimFromObjectiveC, which
+// also performs conformance checks at runtime.
+// CHECK-LABEL: sil [noinline] @_TTSf4g___TF17cast_folding_objc30testBridgedCastFromObjCtoSwiftFCSo8NSStringSS
+// CHECK-NOT: {{ cast}}
+// CHECK: metatype $@thick String.Type
+// CHECK: function_ref @_TTWSSs21_ObjectiveCBridgeable10FoundationZFS_26_forceBridgeFromObjectiveCfTwx15_ObjectiveCType6resultRGSqx__T_
+// CHECK: apply
+// CHECK: return
+@inline(never)
+public func testBridgedCastFromObjCtoSwift(ns: NSString) -> String {
+  return ns as String
+}
+
+// Check that compiler understands that this cast always succeeds
+// CHECK-LABEL: sil [noinline] @_TTSf4gs___TF17cast_folding_objc30testBridgedCastFromSwiftToObjCFSSCSo8NSString
+// CHECK-NOT: {{ cast}}
+// CHECK: function_ref @_TFE10FoundationSS19_bridgeToObjectiveC
+// CHECK: apply
+// CHECK: return
+@inline(never)
+public func testBridgedCastFromSwiftToObjC(s: String) -> NSString {
+  return s as NSString
+}
+
+

test/SILOptimizer/devirt_access_other_module.swift

@@ -18,7 +18,7 @@ public func getExternalClass() -> ExternalClass {
 // Note: This will eventually be illegal (to have a public @_transparent function
 // referring to a private method), but for now it lets us test what can and
 // can't be optimized.
-// CHECK-LABEL: sil [transparent] [fragile] @_TF26devirt_access_other_module9invokeFooFCS_13ExternalClassT_
+// CHECK-LABEL: sil [transparent] [fragile] @_TTSf4g___TF26devirt_access_other_module9invokeFooFCS_13ExternalClassT_
 // CHECK-NOT: function_ref
 // CHECK: class_method
 // CHECK-NOT: function_ref

test/SILOptimizer/devirt_covariant_return.swift

@@ -105,6 +105,37 @@ func driver() -> () {
 
 driver()
 
+public class Bear {
+  public init?(fail: Bool) {
+    if fail { return nil }
+  }
+
+  // Check that devirtualizer can handle convenience initializers, which have covariant optional
+  // return types.
+  // CHECK-LABEL: sil @_TFC23devirt_covariant_return4Bearc
+  // CHECK: checked_cast_br [exact] %{{.*}} : $Bear to $PolarBear
+  // CHECK: upcast %{{.*}} : $Optional<PolarBear> to $Optional<Bear>
+  // CHECK: }
+  public convenience init?(delegateFailure: Bool, failAfter: Bool) {
+    self.init(fail: delegateFailure)
+    if failAfter { return nil }
+  }
+}
+
+final class PolarBear: Bear {
+
+  override init?(fail: Bool) {
+    super.init(fail: fail)
+  }
+
+  init?(chainFailure: Bool, failAfter: Bool) {
+    super.init(fail: chainFailure)
+    if failAfter { return nil }
+  }
+}
+
+
+
 
 class Payload {
   let value: Int32
@@ -148,7 +179,7 @@ final class C2:C {
 // Check that the Optional return value from doSomething
 // gets properly unwrapped into a Payload object and then further
 // devirtualized.
-// CHECK-LABEL: sil hidden @_TF23devirt_covariant_return7driver1FCS_2C1Vs5Int32 :
+// CHECK-LABEL: sil hidden @_TTSf4dg___TF23devirt_covariant_return7driver1FCS_2C1Vs5Int32
 // CHECK: integer_literal $Builtin.Int32, 2
 // CHECK: struct $Int32 (%{{.*}} : $Builtin.Int32)
 // CHECK-NOT: class_method
@@ -161,7 +192,7 @@ func driver1(c: C1) -> Int32 {
 // Check that the Optional return value from doSomething
 // gets properly unwrapped into a Payload object and then further
 // devirtualized.
-// CHECK-LABEL: sil hidden @_TF23devirt_covariant_return7driver3FCS_1CVs5Int32 :
+// CHECK-LABEL: sil hidden @_TTSf4g___TF23devirt_covariant_return7driver3FCS_1CVs5Int32
 // CHECK: bb{{[0-9]+}}(%{{[0-9]+}} : $C2):
 // CHECK-NOT: bb{{.*}}:
 // check that for C2, we convert the non-optional result into an optional and then cast.
@@ -172,35 +203,6 @@ func driver3(c: C) -> Int32 {
   return c.doSomething()!.getValue()
 }
 
-public class Bear {
-  public init?(fail: Bool) {
-    if fail { return nil }
-  }
-
-  // Check that devirtualizer can handle convenience initializers, which have covariant optional
-  // return types.
-  // CHECK-LABEL: sil @_TFC23devirt_covariant_return4Bearc
-  // CHECK: checked_cast_br [exact] %{{.*}} : $Bear to $PolarBear
-  // CHECK: upcast %{{.*}} : $Optional<PolarBear> to $Optional<Bear>
-  // CHECK: }
-  public convenience init?(delegateFailure: Bool, failAfter: Bool) {
-    self.init(fail: delegateFailure)
-    if failAfter { return nil }
-  }
-}
-
-final class PolarBear: Bear {
-
-  override init?(fail: Bool) {
-    super.init(fail: fail)
-  }
-
-  init?(chainFailure: Bool, failAfter: Bool) {
-    super.init(fail: chainFailure)
-    if failAfter { return nil }
-  }
-}
-
 public class D {
   let v: Int32
   init(_ n: Int32) {
@@ -230,7 +232,7 @@ public class D2: D1 {
 
 // Check that the boo call gets properly devirtualized and that
 // that D2.foo() is inlined thanks to this.
-// CHECK-LABEL: sil hidden @_TF23devirt_covariant_return7driver2FCS_2D2Vs5Int32
+// CHECK-LABEL: sil hidden @_TTSf4g___TF23devirt_covariant_return7driver2FCS_2D2Vs5Int32
 // CHECK-NOT: class_method
 // CHECK: checked_cast_br [exact] %{{.*}} : $D1 to $D2
 // CHECK: bb2
@@ -274,7 +276,7 @@ class EEE : CCC {
 
 // Check that c.foo() is devirtualized, because the optimizer can handle the casting the return type
 // correctly, i.e. it can cast (BBB, BBB) into (AAA, AAA)
-// CHECK-LABEL: sil hidden @_TF23devirt_covariant_return37testDevirtOfMethodReturningTupleTypesFTCS_3CCC1bCS_2BB_TCS_2AAS2__
+// CHECK-LABEL: sil hidden @_TTSf4g_n___TF23devirt_covariant_return37testDevirtOfMethodReturningTupleTypesFTCS_3CCC1bCS_2BB_TCS_2AAS2__
 // CHECK: checked_cast_br [exact] %{{.*}} : $CCC to $CCC
 // CHECK: checked_cast_br [exact] %{{.*}} : $CCC to $DDD
 // CHECK: checked_cast_br [exact] %{{.*}} : $CCC to $EEE
@@ -311,17 +313,19 @@ class DDDD : CCCC {
   }
 }
 
-// Check that c.foo OSX 10.9 be devirtualized, because the optimizer can handle the casting the return type
-// correctly, i.e. it cannot cast (BBBB, BBBB) into (AAAA, AAAA)
-// CHECK-LABEL: sil hidden @_TF23devirt_covariant_return38testDevirtOfMethodReturningTupleTypes2FCS_4CCCCTCS_4AAAAS1__
-// CHECK: checked_cast_br [exact] %{{.*}} : $CCCC to $CCCC
-// CHECK: checked_cast_br [exact] %{{.*}} : $CCCC to $DDDD
-// CHECK: class_method
-// CHECK: }
-func testDevirtOfMethodReturningTupleTypes2(c: CCCC) -> (AAAA, AAAA) {
-  return c.foo
+// Check devirtualization of methods with optional results, where
+// optional results need to be casted.
+// CHECK-LABEL: sil @{{.*}}testOverridingMethodWithOptionalResult
+// CHECK: checked_cast_br [exact] %{{.*}} : $F to $F
+// CHECK: checked_cast_br [exact] %{{.*}} : $F to $G
+// CHECK: switch_enum
+// CHECK: checked_cast_br [exact] %{{.*}} : $F to $H
+// CHECK: switch_enum
+public func testOverridingMethodWithOptionalResult(f: F) -> (F?, Int)? {
+  return f.foo()
 }
 
+
 public class F {
   @inline(never)
   public func foo() -> (F?, Int)? {
@@ -342,16 +346,3 @@ public class H: F {
     return nil
   }
 }
-
-// Check devirtualization of methods with optional results, where
-// optional results need to be casted.
-// CHECK-LABEL: sil @{{.*}}testOverridingMethodWithOptionalResult
-// CHECK: checked_cast_br [exact] %{{.*}} : $F to $F
-// CHECK: checked_cast_br [exact] %{{.*}} : $F to $G
-// CHECK: switch_enum
-// CHECK: checked_cast_br [exact] %{{.*}} : $F to $H
-// CHECK: switch_enum
-public func testOverridingMethodWithOptionalResult(f: F) -> (F?, Int)? {
-  return f.foo()
-}
-