Fix ExistentialSpecializer to detect repeated specialization.

Add handling for repeated specialization and remove an incorrect
assertion in
ExistentialTransform::createExistentialSpecializedFunctionName():
assert(!F->getModule().hasFunction(MangledName));

The ExistentialSpecializer replaces the original function with a thunk
to a newly specialized function. Repeated attempts to specialize the
same function bail out because the pass avoids reoptimizing thunks.

Ultimately, the intention is that the thunks will all be inlined into
their callers. Dead function elimination will then remove the
thunks. If the original function was itself a specialization, then the
GenericSpecialize may regenerate the original again in non-thunk form.

Consider the pipeline:
- GenericSpecializer
- ExistentialSpecializer
- Inliner
- DeadFunctionElimination
- GenericSpecializer
- ExistentialSpecializer

This is not a problem with the ExistentialSpecializer itself. In fact,
it may respecialize the same function in different ways, for example
specializing more of the arguments each time. Each different
specialization transforms the original function into a thunk, that
thunk is inlined, and the newly specialized code is called directly.

Of course, the ExistentialSpecializer may also decide to respecialize
a function the same way as before. When doing this, it still needs to
produce the thunk, which was dead function eliminated since last
specialization of the same function. However, it can simply reuse the
previous specialization by performing a name lookup first.

The design problem is that the SILModule makes assumptions about
duplicate symbols when managing symbol memory but does not provide a
robust way to protect against such duplicate symbols. That will be
improved in a separate commit.

Minimal fix for: rdar://72135512 The ExistentialSpecializer crashes

Author: atrick

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialTransform.cpp

@@ -292,9 +292,7 @@ std::string ExistentialTransform::createExistentialSpecializedFunctionName() {
     int Idx = IdxIt.first;
     Mangler.setArgumentExistentialToGeneric(Idx);
   }
-  auto MangledName = Mangler.mangle();
-  assert(!F->getModule().hasFunction(MangledName));
-  return MangledName;
+  return Mangler.mangle();
 }
 
 /// Convert all existential argument types to generic argument type.
@@ -642,41 +640,47 @@ void ExistentialTransform::populateThunkBody() {
 ///     its inline strategy as always inline.
 void ExistentialTransform::createExistentialSpecializedFunction() {
   std::string Name = createExistentialSpecializedFunctionName();
-  SILLinkage linkage = getSpecializedLinkage(F, F->getLinkage());
 
-  /// Create devirtualized function type.
+  /// Create devirtualized function type and populate ArgToGenericTypeMap.
   auto NewFTy = createExistentialSpecializedFunctionType();
 
-  auto NewFGenericSig = NewFTy->getInvocationGenericSignature();
-  auto NewFGenericEnv = NewFGenericSig->getGenericEnvironment();
-
   /// Step 1: Create the new protocol constrained generic function.
-  NewF = FunctionBuilder.createFunction(
+  if (auto *CachedFn = F->getModule().lookUpFunction(Name)) {
+    // The specialized body still exists (because it is now called directly),
+    // but the thunk has been dead-code eliminated.
+    assert(CachedFn->getLoweredFunctionType() == NewFTy);
+    NewF = CachedFn;
+  } else {
+    auto NewFGenericSig = NewFTy->getInvocationGenericSignature();
+    auto NewFGenericEnv = NewFGenericSig->getGenericEnvironment();
+    SILLinkage linkage = getSpecializedLinkage(F, F->getLinkage());
+
+    NewF = FunctionBuilder.createFunction(
       linkage, Name, NewFTy, NewFGenericEnv, F->getLocation(), F->isBare(),
       F->isTransparent(), F->isSerialized(), IsNotDynamic, F->getEntryCount(),
       F->isThunk(), F->getClassSubclassScope(), F->getInlineStrategy(),
       F->getEffectsKind(), nullptr, F->getDebugScope());
-  /// Set the semantics attributes for the new function.
-  for (auto &Attr : F->getSemanticsAttrs())
-    NewF->addSemanticsAttr(Attr);
 
-  /// Set Unqualified ownership, if any.
-  if (!F->hasOwnership()) {
-    NewF->setOwnershipEliminated();
-  }
+    /// Set the semantics attributes for the new function.
+    for (auto &Attr : F->getSemanticsAttrs())
+      NewF->addSemanticsAttr(Attr);
 
-  /// Step 1a: Populate the body of NewF.
-  SubstitutionMap Subs = SubstitutionMap::get(
+    /// Set Unqualified ownership, if any.
+    if (!F->hasOwnership()) {
+      NewF->setOwnershipEliminated();
+    }
+    /// Step 1a: Populate the body of NewF.
+    SubstitutionMap Subs = SubstitutionMap::get(
       NewFGenericSig,
       [&](SubstitutableType *type) -> Type {
         return NewFGenericEnv->mapTypeIntoContext(type);
       },
       LookUpConformanceInModule(F->getModule().getSwiftModule()));
-  ExistentialSpecializerCloner cloner(F, NewF, Subs, ArgumentDescList,
-                                      ArgToGenericTypeMap,
-                                      ExistentialArgDescriptor);
-  cloner.cloneAndPopulateFunction();
-
+    ExistentialSpecializerCloner cloner(F, NewF, Subs, ArgumentDescList,
+                                        ArgToGenericTypeMap,
+                                        ExistentialArgDescriptor);
+    cloner.cloneAndPopulateFunction();
+  }
   /// Step 2: Create the thunk with always_inline and populate its body.
   populateThunkBody();
 

test/SILOptimizer/existential_transform_respecialize.sil

@@ -0,0 +1,99 @@
+// RUN: %target-sil-opt -enable-sil-verify-all %s -existential-specializer | %FileCheck %s
+
+import Builtin
+import Swift
+import SwiftShims
+
+// -----------------------------------------------------------------------------
+// Test respecialization after the original thunk has been dead-code-eliminated.
+//
+// Consider the pipeline:
+//  - GenericSpecializer
+//  - ExistentialSpecializer
+//  - Inliner
+//  - DeadFunctionElimination
+//  - GenericSpecializer
+//  - ExistentialSpecializer
+//
+// This test models the pipeline just before the final invocation of
+// ExistentialSpecializer. The SIL now contains a function "wrapFoo"
+// that has already been specialized (with the suffix
+// "Tf4ee_n"). However, the thunk for that specialization does not
+// exist (it would have been inlined and dead-code eliminated by the
+// pipeline above). This allows the ExistentialSpecializer to kick in
+// again on a call to the same function.
+//
+// Here we reinvoke ExistentialSpecializer to ensure that it reuses
+// the existing specialization of wrapFoo (with the suffix "Tf4ee_n").
+// "callPrespecializedWrapFoo" is what drives the optimization this
+// time around by calling into the original, unspecialized wrapFoo.
+
+internal protocol SomeProtocol : AnyObject {
+  func foo() -> Int
+}
+
+@_hasMissingDesignatedInitializers public class SomeClass : SomeProtocol {
+  @_hasStorage @_hasInitialValue var x: Int { get set }
+  init()
+  @inline(never) func foo() -> Int
+}
+
+// SomeClass.foo()
+sil [noinline] @$s1t9SomeClassC3fooSiyF : $@convention(method) (@guaranteed SomeClass) -> Int
+
+// The original "wrapFoo" function prior to existential specialization.
+//
+// This will be replaced with a thunk by existential specialization.
+// CHECK-LABEL: sil hidden [signature_optimized_thunk] [always_inline] @$s1t7wrapFoo1a1bSi_SitAA12SomeProtocol_p_AaE_ptF : $@convention(thin) (@guaranteed SomeProtocol, @guaranteed SomeProtocol) -> (Int, Int) {
+sil hidden [noinline] @$s1t7wrapFoo1a1bSi_SitAA12SomeProtocol_p_AaE_ptF : $@convention(thin) (@guaranteed SomeProtocol, @guaranteed SomeProtocol) -> (Int, Int) {
+bb0(%0 : $SomeProtocol, %1 : $SomeProtocol):
+  debug_value %0 : $SomeProtocol, let, name "a", argno 1
+  debug_value %1 : $SomeProtocol, let, name "b", argno 2
+  %4 = open_existential_ref %0 : $SomeProtocol to $@opened("15146E00-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol
+  %5 = unchecked_ref_cast %4 : $@opened("15146E00-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol to $SomeClass
+  // function_ref SomeClass.foo()
+  %6 = function_ref @$s1t9SomeClassC3fooSiyF : $@convention(method) (@guaranteed SomeClass) -> Int
+  %7 = apply %6(%5) : $@convention(method) (@guaranteed SomeClass) -> Int
+  %8 = open_existential_ref %1 : $SomeProtocol to $@opened("15147238-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol
+  %9 = unchecked_ref_cast %8 : $@opened("15147238-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol to $SomeClass
+  %10 = apply %6(%9) : $@convention(method) (@guaranteed SomeClass) -> Int
+  %11 = tuple (%7 : $Int, %10 : $Int)
+  return %11 : $(Int, Int)
+}
+
+// CHECK-LABEL: sil @callPrespecializedWrapFoo : $@convention(thin) (@guaranteed SomeClass) -> (Int, Int) {
+sil @callPrespecializedWrapFoo : $@convention(thin) (@guaranteed SomeClass) -> (Int, Int) {
+bb0(%0 : $SomeClass):
+  debug_value %0 : $SomeClass, let, name "a", argno 1
+  %2 = init_existential_ref %0 : $SomeClass : $SomeClass, $SomeProtocol
+  %3 = init_existential_ref %0 : $SomeClass : $SomeClass, $SomeProtocol
+  // function_ref wrapFoo(a:b:)
+  %4 = function_ref @$s1t7wrapFoo1a1bSi_SitAA12SomeProtocol_p_AaE_ptF : $@convention(thin) (@guaranteed SomeProtocol, @guaranteed SomeProtocol) -> (Int, Int)
+  %5 = apply %4(%2, %3) : $@convention(thin) (@guaranteed SomeProtocol, @guaranteed SomeProtocol) -> (Int, Int)
+  %6 = tuple_extract %5 : $(Int, Int), 0
+  %7 = tuple_extract %5 : $(Int, Int), 1
+  %8 = tuple (%6 : $Int, %7 : $Int)
+  return %8 : $(Int, Int)
+}
+
+// wrapFoo after existential specialization AND thunk inlining.
+// After thunk inlining, the original thunk may be dead, but the specialized function may still be called.
+//
+// CHECK-LABEL: sil shared [noinline] @$s1t7wrapFoo1a1bSi_SitAA12SomeProtocol_p_AaE_ptFTf4ee_n : $@convention(thin) <τ_0_0, τ_0_1 where τ_0_0 : SomeProtocol, τ_0_1 : SomeProtocol> (@guaranteed τ_0_0, @guaranteed τ_0_1) -> (Int, Int) {
+sil shared [noinline] @$s1t7wrapFoo1a1bSi_SitAA12SomeProtocol_p_AaE_ptFTf4ee_n : $@convention(thin) <τ_0_0, τ_0_1 where τ_0_0 : SomeProtocol, τ_0_1 : SomeProtocol> (@guaranteed τ_0_0, @guaranteed τ_0_1) -> (Int, Int) {
+bb0(%0 : $τ_0_0, %1 : $τ_0_1):
+  %2 = init_existential_ref %0 : $τ_0_0 : $τ_0_0, $SomeProtocol
+  %3 = init_existential_ref %1 : $τ_0_1 : $τ_0_1, $SomeProtocol
+  debug_value %2 : $SomeProtocol, let, name "a", argno 1
+  debug_value %3 : $SomeProtocol, let, name "b", argno 2
+  %6 = open_existential_ref %2 : $SomeProtocol to $@opened("47F4DDE6-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol
+  %7 = unchecked_ref_cast %6 : $@opened("47F4DDE6-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol to $SomeClass
+  // function_ref SomeClass.foo()
+  %8 = function_ref @$s1t9SomeClassC3fooSiyF : $@convention(method) (@guaranteed SomeClass) -> Int
+  %9 = apply %8(%7) : $@convention(method) (@guaranteed SomeClass) -> Int
+  %10 = open_existential_ref %3 : $SomeProtocol to $@opened("47F4E1A6-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol
+  %11 = unchecked_ref_cast %10 : $@opened("47F4E1A6-3B09-11EB-9A47-D0817AD9F6DD") SomeProtocol to $SomeClass
+  %12 = apply %8(%11) : $@convention(method) (@guaranteed SomeClass) -> Int
+  %13 = tuple (%9 : $Int, %12 : $Int)
+  return %13 : $(Int, Int)
+}