Merge pull request #25038 from atrick/fix-specializer-leak

Fix ExistentialSpecializer to correctly cleanup storage.

Author: rajbarik

include/swift/AST/SILOptions.h

@@ -50,9 +50,6 @@ class SILOptions {
   /// Remove all runtime assertions during optimizations.
   bool RemoveRuntimeAsserts = false;
 
-  /// Enable existential specializer optimization.
-  bool ExistentialSpecializer = false;
-
   /// Controls whether the SIL ARC optimizations are run.
   bool EnableARCOptimizations = true;
 

include/swift/Option/FrontendOptions.td

@@ -472,9 +472,6 @@ def sil_unroll_threshold : Separate<["-"], "sil-unroll-threshold">,
   MetaVarName<"<250>">,
   HelpText<"Controls the aggressiveness of loop unrolling">;
 
-def sil_existential_specializer : Flag<["-"], "sil-existential-specializer">,
-  HelpText<"Enable SIL existential specializer optimization">;
-
 def sil_merge_partial_modules : Flag<["-"], "sil-merge-partial-modules">,
   HelpText<"Merge SIL from all partial swiftmodules into the final module">;
 

lib/Frontend/CompilerInvocation.cpp

@@ -702,9 +702,6 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
       return true;
     }
   }
-  if (Args.hasArg(OPT_sil_existential_specializer)) {
-    Opts.ExistentialSpecializer = true;
-  }
   if (const Arg *A = Args.getLastArg(OPT_num_threads)) {
     if (StringRef(A->getValue()).getAsInteger(10, Opts.NumThreads)) {
       Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

@@ -28,6 +28,11 @@
 
 using namespace swift;
 
+static llvm::cl::opt<bool>
+EnableExistentialSpecializer("enable-existential-specializer",
+                             llvm::cl::Hidden,
+                             llvm::cl::init(true));
+
 STATISTIC(NumFunctionsWithExistentialArgsSpecialized,
           "Number of functions with existential args specialized");
 
@@ -66,7 +71,7 @@ class ExistentialSpecializer : public SILFunctionTransform {
     auto *F = getFunction();
 
     /// Don't optimize functions that should not be optimized.
-    if (!F->shouldOptimize() || !F->getModule().getOptions().ExistentialSpecializer) {
+    if (!F->shouldOptimize() || !EnableExistentialSpecializer) {
       return;
     }
 
@@ -110,19 +115,6 @@ bool ExistentialSpecializer::findConcreteTypeFromSoleConformingType(
   return true;
 }
 
-/// Check if the argument Arg is used in a destroy_use instruction.
-static void
-findIfCalleeUsesArgInDestroyUse(SILValue Arg,
-                                ExistentialTransformArgumentDescriptor &ETAD) {
-  for (Operand *ArgUse : Arg->getUses()) {
-    auto *ArgUser = ArgUse->getUser();
-    if (isa<DestroyAddrInst>(ArgUser)) {
-      ETAD.DestroyAddrUse = true;
-      break;
-    }
-  }
-}
-
 /// Helper function to ensure that the argument is not InOut or InOut_Aliasable
 static bool isNonInoutIndirectArgument(SILValue Arg,
                                        SILArgumentConvention ArgConvention) {
@@ -188,24 +180,27 @@ bool ExistentialSpecializer::canSpecializeExistentialArgsInFunction(
       continue;
     }
 
-    /// Determine attributes of the existential addr arguments such as
-    /// destroy_use, immutable_access. 
+    /// Determine attributes of the existential addr argument.
     ExistentialTransformArgumentDescriptor ETAD;
     auto paramInfo = origCalleeConv.getParamInfoForSILArg(Idx);
-    ETAD.AccessType = (paramInfo.isIndirectMutating() || paramInfo.isConsumed())
+    // The ExistentialSpecializerCloner copies the incoming generic argument
+    // into an existential. This won't work if the original argument is
+    // mutated. Furthermore, SILCombine would not be able to replace a mutated
+    // existential with a concrete value, so the specialization thunk could not
+    // be optimized away.
+    if (paramInfo.isIndirectMutating())
+      continue;
+
+    ETAD.AccessType = paramInfo.isConsumed()
                           ? OpenedExistentialAccess::Mutable
                           : OpenedExistentialAccess::Immutable;
-    ETAD.DestroyAddrUse = false;
-    if ((CalleeArgs[Idx]->getType().getPreferredExistentialRepresentation(
-            F->getModule()))
-        != ExistentialRepresentation::Class)
-      findIfCalleeUsesArgInDestroyUse(CalleeArg, ETAD);
+    ETAD.isConsumed = paramInfo.isConsumed();
 
     /// Save the attributes
     ExistentialArgDescriptor[Idx] = ETAD;
     LLVM_DEBUG(llvm::dbgs()
                << "ExistentialSpecializer Pass:Function: " << F->getName()
-               << " Arg:" << Idx << "has a concrete type.\n");
+               << " Arg:" << Idx << " has a concrete type.\n");
     returnFlag |= true;
   }
   return returnFlag;