Fix GenericSpecializer for addressable parameters.

Addressable parameters must remain indirect.

Incidentally also fixes an obvious latent bug in which all specialization was
disabled if any metatypes could not be specialized.

Fixes rdar://145687827 (Crash of inline-stored Span properties with optimizations)

Author: atrick

include/swift/SILOptimizer/Utils/Generics.h

@@ -84,7 +84,8 @@ class ReabstractionInfo {
   /// specializer.
   bool ConvertIndirectToDirect = true;
 
-  /// If true, drop unused arguments.
+  /// If true, drop unused arguments. Dropping unused arguments is a
+  /// prerequisite before promoting an indirect argument to a direct argument.
   /// See `droppedArguments`.
   bool dropUnusedArguments = false;
 

lib/SILOptimizer/Utils/Generics.cpp

@@ -455,6 +455,82 @@ static bool shouldNotSpecialize(SILFunction *Callee, SILFunction *Caller,
   return false;
 }
 
+// Addressable parameters cannot be dropped because the address may
+// escape. They also can't be promoted to direct convention, so there
+// is no danger in preserving them.
+static bool canConvertArg(CanSILFunctionType substType, unsigned paramIdx,
+                          SILFunction *caller) {
+  return !substType->isAddressable(paramIdx, caller);
+}
+
+// If there is no read from an indirect argument, this argument has to be
+// dropped. At the call site the store to the argument's memory location could
+// have been removed (based on the callee's memory effects).  Therefore,
+// converting such an unused indirect argument to a direct argument, would load
+// an uninitialized value at the call site.  This would lead to verifier errors
+// and in worst case to a miscompile because IRGen can implicitly use dead
+// arguments, e.g. for getting the type of a class reference.
+static bool canDropUnusedArg(ApplySite apply, SILFunction *callee,
+                             CanSILFunctionType substType,
+                             unsigned paramIdx) {
+  FullApplySite fas = apply.asFullApplySite();
+  if (!fas) {
+    return false;
+  }
+  Operand &op = fas.getOperandsWithoutIndirectResults()[paramIdx];
+  return !callee->argumentMayRead(&op, op.get());
+}
+
+static bool isUsedAsDynamicSelf(SILArgument *arg) {
+  for (Operand *use : arg->getUses()) {
+    if (use->isTypeDependent())
+      return true;
+  }
+  return false;
+}
+
+static bool canDropMetatypeArg(ApplySite apply, SILFunction *callee,
+                               unsigned paramIdx) {
+  if (!callee->isDefinition())
+    return false;
+
+  unsigned calleeArgIdx =
+    apply.getSubstCalleeConv().getSILArgIndexOfFirstParam() + paramIdx;
+  SILArgument *calleeArg = callee->getArguments()[calleeArgIdx];
+
+  if (isUsedAsDynamicSelf(calleeArg))
+    return false;
+
+  if (calleeArg->getType().getASTType()->hasDynamicSelfType())
+    return false;
+
+  // We don't drop metatype arguments of not applied arguments (in case of
+  // `partial_apply`).
+  unsigned firstAppliedArgIdx = apply.getCalleeArgIndexOfFirstAppliedArg();
+  if (firstAppliedArgIdx > calleeArgIdx)
+      return false;
+
+  auto mt = calleeArg->getType().castTo<MetatypeType>();
+  if (mt->hasRepresentation()
+      && mt->getRepresentation() == MetatypeRepresentation::Thin) {
+    return true;
+  }
+  // If the passed thick metatype value is not a `metatype` instruction
+  // we don't know the real metatype at runtime. It's not necessarily the
+  // same as the declared metatype. It could e.g. be an upcast of a class
+  // metatype.
+  SILValue callerArg = apply.getArguments()[calleeArgIdx - firstAppliedArgIdx];
+  if (isa<MetatypeInst>(callerArg))
+    return true;
+
+  // But: if the metatype is not used in the callee we don't have to care
+  // what metatype value is passed. We can just remove it.
+  if (onlyHaveDebugUses(calleeArg))
+    return true;
+
+  return false;
+}
+
 /// Prepares the ReabstractionInfo object for further processing and checks
 /// if the current function can be specialized at all.
 /// Returns false, if the current function cannot be specialized.
@@ -771,7 +847,7 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
   for (SILParameterInfo PI : SubstitutedType->getParameters()) {
     auto IdxToInsert = IdxForParam;
     ++IdxForParam;
-    unsigned argIdx = i++;
+    unsigned paramIdx = i++;
 
     SILFunctionConventions substConv(SubstitutedType, getModule());
     TypeCategory tc = getParamTypeCategory(PI, substConv, getResilienceExpansion());
@@ -782,22 +858,14 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
     case ParameterConvention::Indirect_In_CXX:
     case ParameterConvention::Indirect_In:
     case ParameterConvention::Indirect_In_Guaranteed: {
-      if (Callee && Apply && dropUnusedArguments) {
-        // If there is no read from an indirect argument, this argument has to
-        // be dropped. At the call site the store to the argument's memory location
-        // could have been removed (based on the callee's memory effects).
-        // Therefore, converting such an unused indirect argument to a direct
-        // argument, would load an uninitialized value at the call site.
-        // This would lead to verifier errors and in worst case to a miscompile
-        // because IRGen can implicitly use dead arguments, e.g. for getting the
-        // type of a class reference.
-        if (FullApplySite fas = Apply.asFullApplySite()) {
-          Operand &op = fas.getOperandsWithoutIndirectResults()[argIdx];
-          if (!Callee->argumentMayRead(&op, op.get())) {
-            droppedArguments.set(IdxToInsert);
-            break;
-          }
-        }
+      if (Apply && !canConvertArg(SubstitutedType, paramIdx,
+                                  Apply.getFunction())) {
+        continue;
+      }
+      if (Callee && Apply && dropUnusedArguments
+          && canDropUnusedArg(Apply, Callee, SubstitutedType, paramIdx)) {
+        droppedArguments.set(IdxToInsert);
+        break;
       }
       Conversions.set(IdxToInsert);
       if (tc == LoadableAndTrivial)
@@ -822,8 +890,10 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
     case ParameterConvention::Direct_Unowned:
     case ParameterConvention::Direct_Guaranteed: {
       CanType ty = PI.getInterfaceType();
-      if (dropUnusedArguments && isa<MetatypeType>(ty) && !ty->hasArchetype())
+      if (dropUnusedArguments && isa<MetatypeType>(ty) && !ty->hasArchetype()
+          && Apply && Callee && canDropMetatypeArg(Apply, Callee, paramIdx)) {
         droppedArguments.set(IdxToInsert);
+      }
       break;
     }
     }
@@ -3211,57 +3281,6 @@ static bool usePrespecialized(
   return false;
 }
 
-static bool isUsedAsDynamicSelf(SILArgument *arg) {
-  for (Operand *use : arg->getUses()) {
-    if (use->isTypeDependent())
-      return true;
-  }
-  return false;
-}
-
-static bool canDropMetatypeArgs(ApplySite apply, SILFunction *callee) {
-  if (!callee->isDefinition())
-    return false;
-
-  auto calleeArgs = callee->getArguments();
-  unsigned firstAppliedArgIdx = apply.getCalleeArgIndexOfFirstAppliedArg();
-  for (unsigned calleeArgIdx = 0; calleeArgIdx < calleeArgs.size(); ++calleeArgIdx) {
-    SILArgument *calleeArg = calleeArgs[calleeArgIdx];
-    auto mt = calleeArg->getType().getAs<MetatypeType>();
-    if (!mt)
-      continue;
-
-    if (isUsedAsDynamicSelf(calleeArg))
-      return false;
-
-    if (calleeArg->getType().getASTType()->hasDynamicSelfType())
-      return false;
-
-    // We don't drop metatype arguments of not applied arguments (in case of `partial_apply`).
-    if (firstAppliedArgIdx > calleeArgIdx)
-      return false;
-
-    if (mt->hasRepresentation() && mt->getRepresentation() == MetatypeRepresentation::Thin)
-      continue;
-
-    // If the passed thick metatype value is not a `metatype` instruction
-    // we don't know the real metatype at runtime. It's not necessarily the
-    // same as the declared metatype. It could e.g. be an upcast of a class
-    // metatype.
-    SILValue callerArg = apply.getArguments()[calleeArgIdx - firstAppliedArgIdx];
-    if (isa<MetatypeInst>(callerArg))
-      continue;
-
-    // But: if the metatype is not used in the callee we don't have to care
-    // what metatype value is passed. We can just remove it.
-    if (callee->isDefinition() && onlyHaveDebugUses(calleeArg))
-      continue;
-
-    return false;
-  }
-  return true;
-}
-
 void swift::trySpecializeApplyOfGeneric(
     SILOptFunctionBuilder &FuncBuilder,
     ApplySite Apply, DeadInstructionSet &DeadApplies,
@@ -3314,8 +3333,7 @@ void swift::trySpecializeApplyOfGeneric(
                            FuncBuilder.getModule().isWholeModule(), Apply, RefF,
                            Apply.getSubstitutionMap(), serializedKind,
                            /*ConvertIndirectToDirect=*/ true,
-                           /*dropUnusedArguments=*/
-                           canDropMetatypeArgs(Apply, RefF),
+                           /*dropUnusedArguments=*/ true,
                            &ORE);
   if (!ReInfo.canBeSpecialized())
     return;