Merge pull request #1946 from trentxintong/FS

Change FSO heuristic.

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()
-}
-