Merge pull request #62523 from eeckstein/improve-devirtualizer

Devirtualizer: don't de-virtualize witness calls to non-generic thunks which call a generic function.

Author: eeckstein

lib/SILOptimizer/Utils/Devirtualize.cpp

@@ -1122,6 +1122,24 @@ devirtualizeWitnessMethod(ApplySite applySite, SILFunction *f,
   return {newApplySite, changedCFG};
 }
 
+static bool isNonGenericThunkOfGenericExternalFunction(SILFunction *thunk) {
+  if (!thunk->isThunk())
+    return false;
+  if (thunk->getGenericSignature())
+    return false;
+  for (SILBasicBlock &block : *thunk) {
+    for (SILInstruction &inst : block) {
+      if (FullApplySite fas = FullApplySite::isa(&inst)) {
+        if (SILFunction *calledFunc = fas.getReferencedFunctionOrNull()) {
+          if (fas.hasSubstitutions() && !calledFunc->isDefinition())
+            return true;
+        }
+      }
+    }
+  }
+  return false;
+}
+
 static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
   SILFunction *f;
   SILWitnessTable *wt;
@@ -1150,6 +1168,25 @@ static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
     return false;
   }
 
+  // The following check is for performance reasons: if `f` is a non-generic thunk
+  // which calls a (not inlinable) generic function in the defining module, it's
+  // more efficient to not devirtualize, but call the non-generic thunk - even though
+  // it's done through the witness table.
+  // Example:
+  // ```
+  //   protocol P {
+  //     func f(x: [Int])   // not generic
+  //   }
+  //   struct S: P {
+  //     func f(x: some RandomAccessCollection<Int>) { ... } // generic
+  //   }
+  // ```
+  // In the defining module, the generic conformance can be specialized (which is not
+  // possible in the client module, because it's not inlinable).
+  if (isNonGenericThunkOfGenericExternalFunction(f)) {
+    return false;
+  }
+
   // FIXME: devirtualizeWitnessMethod does not support cases with covariant
   // 'Self'-rooted type parameters nested inside a collection type, like
   // '[Self]' or '[* : Self.A]', because it doesn't know how to deal with

test/ModuleInterface/Conformances.swiftinterface

@@ -3,8 +3,6 @@
 
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module-path %t/Conformances.swiftmodule -enable-library-evolution -emit-sil -o %t/Conformances.sil -enable-objc-interop -disable-objc-attr-requires-foundation-module %s
-// RUN: %FileCheck -check-prefix CHECK-MODULE %s < %t/Conformances.sil
-// RUN: %FileCheck -check-prefix NEGATIVE-MODULE %s < %t/Conformances.sil
 // RUN: %target-swift-frontend -enable-objc-interop -emit-sil -I %t %S/Inputs/ConformancesUser.swift -O | %FileCheck %s
 
 public protocol MyProto {
@@ -41,11 +39,11 @@ public struct FullStructImpl: MyProto {
 public struct OpaqueStructImpl: MyProto {}
 
 // CHECK-LABEL: sil @$s16ConformancesUser10testOpaqueSiyF
-// CHECK: function_ref @$s12Conformances7MyProtoPxycfC
+// CHECK: witness_method $OpaqueStructImpl, #MyProto.init!allocator
 // Note the default implementation is filled in here.
-// CHECK: function_ref @$s12Conformances7MyProtoPAAE6methodyyF
-// CHECK: function_ref @$s12Conformances7MyProtoP4propSivs
-// CHECK: function_ref @$s12Conformances7MyProtoPyS2icig
+// CHECK: witness_method $OpaqueStructImpl, #MyProto.method
+// CHECK: witness_method $OpaqueStructImpl, #MyProto.prop!setter
+// CHECK: witness_method $OpaqueStructImpl, #MyProto.subscript!getter
 // CHECK: end sil function '$s16ConformancesUser10testOpaqueSiyF'
 
 

test/SILOptimizer/devirt_witness_cross_module.swift

@@ -0,0 +1,98 @@
+// RUN: %empty-directory(%t)
+
+// First test: without cross-module-optimization
+
+// RUN: %target-build-swift -O -wmo -disable-cmo -parse-as-library -DMODULE -emit-module -emit-module-path=%t/Module.swiftmodule -module-name=Module %s
+// RUN: %target-build-swift -O -wmo -module-name=Main -I%t %s -c -emit-sil | %FileCheck --check-prefix=CHECK --check-prefix=CHECK-NOCMO %s
+
+// Second test: with cross-module-optimization (which is the default)
+
+// RUN: %target-build-swift -O -wmo -Xfrontend -enable-default-cmo -parse-as-library -DMODULE -emit-module -emit-module-path=%t/Module.swiftmodule -module-name=Module %s
+// RUN: %target-build-swift -O -wmo -module-name=Main -I%t %s -c -emit-sil | %FileCheck --check-prefix=CHECK --check-prefix=CHECK-CMO %s
+
+#if MODULE
+
+public protocol P {
+  func foo(x: [Int])
+  func bar(x: [Int])
+  func baz(x: some RandomAccessCollection<Int>)
+}
+
+public struct S: P {
+  public init() {}
+
+  @inline(never)
+  public func foo(x: some RandomAccessCollection<Int>) { }
+  @inline(never)
+  public func bar(x: [Int]) { }
+  @inline(never)
+  public func baz(x: some RandomAccessCollection<Int>) { }
+}
+
+#else
+
+import Module
+
+public struct Local: P {
+  @inline(never)
+  public func foo(x: some RandomAccessCollection<Int>) { }
+  @inline(never)
+  public func bar(x: [Int]) { }
+  @inline(never)
+  public func baz(x: some RandomAccessCollection<Int>) { }
+}
+
+// Don't devirtualize in this case because it's better to call the non-generic function
+// (which can be fully specialized in the module) via the witness table than the generic
+// de-virtualized function.
+
+// CHECK-LABEL: sil @$s4Main24testGenericInOtherModuleyyF
+// CHECK-NOCMO:   [[F:%[0-9]+]] = witness_method $S, #P.foo
+// CHECK-CMO:     [[F:%[0-9]+]] = function_ref @$s6Module1SV3foo1xyx_tSkRzSi7ElementRtzlFSaySiG_Tgq5{{.*}}
+// CHECK:         apply [[F]]
+// CHECK:       } // end sil function '$s4Main24testGenericInOtherModuleyyF'
+public func testGenericInOtherModule() {
+  let s = S()
+  callFoo(s, x: [])
+}
+
+// CHECK-LABEL: sil @$s4Main27testNonGenericInOtherModuleyyF
+// CHECK:         [[F:%[0-9]+]] = function_ref @$s6Module1SV3bar1xySaySiG_tF
+// CHECK:         apply [[F]]
+// CHECK:       } // end sil function '$s4Main27testNonGenericInOtherModuleyyF'
+public func testNonGenericInOtherModule() {
+  let s = S()
+  callBar(s, x: [])
+}
+
+// CHECK-LABEL: sil @$s4Main35testGenericRequirementInOtherModuleyyF
+// CHECK:         [[F:%[0-9]+]] = function_ref @$s6Module1SV3baz1xyx_tSkRzSi7ElementRtzlF{{.*}}
+// CHECK:         apply [[F]]
+// CHECK:       } // end sil function '$s4Main35testGenericRequirementInOtherModuleyyF'
+public func testGenericRequirementInOtherModule() {
+  let s = S()
+  callBaz(s, x: [])
+}
+
+// CHECK-LABEL: sil @$s4Main23testGenericInSameModuleyyF
+// CHECK:         [[F:%[0-9]+]] = function_ref @$s4Main5LocalV3foo1xyx_tSkRzSi7ElementRtzlFSaySiG_Tg5
+// CHECK:         apply [[F]]
+// CHECK:       } // end sil function '$s4Main23testGenericInSameModuleyyF'
+public func testGenericInSameModule() {
+  let l = Local()
+  callFoo(l, x: [])
+}
+
+func callFoo<T: P>(_ f: T, x: [Int]) {
+  f.foo(x: x)
+}
+
+func callBar<T: P>(_ f: T, x: [Int]) {
+  f.bar(x: x)
+}
+
+func callBaz<T: P>(_ f: T, x: [Int]) {
+  f.baz(x: x)
+}
+
+#endif

test/Serialization/sil-imported-enums.swift

@@ -16,6 +16,6 @@ import UsesImportedEnums
 // CHECK-LABEL: sil hidden @$s4main4test1eSbSo13NSRuncingModeV_tF
 func test(e: NSRuncingMode) -> Bool {
   // CHECK-NOT: return
-  // CHECK: $ss2eeoiySbx_xtSYRzSQ8RawValueRpzlF
+  // CHECK: witness_method $NSRuncingMode, #Equatable."=="
   return compareImportedEnumToSelf(e)
 }