Merge pull request #59138 from AnthonyLatsis/5.7-se-309-sil-opt

🍒 [5.7] SE-0309: SILOptimizer fixes & reenable executable tests

Author: AnthonyLatsis

include/swift/SILOptimizer/Utils/Existential.h

@@ -33,7 +33,7 @@ namespace swift {
 /// When successfull, ConcreteExistentialInfo can be used to determine the
 /// concrete type of the opened existential.
 struct OpenedArchetypeInfo {
-  ArchetypeType *OpenedArchetype = nullptr;
+  OpenedArchetypeType *OpenedArchetype = nullptr;
   // The opened value.
   SingleValueInstruction *OpenedArchetypeValue;
   // The existential value.

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -920,45 +920,56 @@ static bool canReplaceCopiedArg(FullApplySite Apply, SILValue Arg,
   return true;
 }
 
+/// Determine if the result type or argument types of the given apply, except
+/// for the argument at \p SkipArgIdx, contain an opened archetype rooted
+/// on \p RootOA.
+static bool applyInvolvesOpenedArchetypeWithRoot(FullApplySite Apply,
+                                                 OpenedArchetypeType *RootOA,
+                                                 unsigned SkipArgIdx) {
+  if (Apply.getType().getASTType()->hasOpenedExistentialWithRoot(RootOA)) {
+    return true;
+  }
+
+  const auto NumApplyArgs = Apply.getNumArguments();
+  for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
+    if (Idx == SkipArgIdx)
+      continue;
+    if (Apply.getArgument(Idx)
+            ->getType()
+            .getASTType()
+            ->hasOpenedExistentialWithRoot(RootOA)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 // Check the legal conditions under which a Arg parameter (specified as ArgIdx)
 // can be replaced with a concrete type. Concrete type info is passed as CEI
 // argument.
 bool SILCombiner::canReplaceArg(FullApplySite Apply,
                                 const OpenedArchetypeInfo &OAI,
                                 const ConcreteExistentialInfo &CEI,
                                 unsigned ArgIdx) {
-
-  // Don't specialize apply instructions that return the callee's Arg type,
-  // because this optimization does not know how to substitute types in the
-  // users of this apply. In the function type substitution below, all
-  // references to OpenedArchetype will be substituted. So walk to type to
-  // find all possible references, such as returning Optional<Arg>.
-  if (Apply.getType().getASTType().findIf(
-          [&OAI](Type t) -> bool { return t->isEqual(OAI.OpenedArchetype); })) {
-    return false;
-  }
-  // Bail out if any other arguments or indirect result that refer to the
-  // OpenedArchetype. The following optimization substitutes all occurrences
-  // of OpenedArchetype in the function signature, but will only rewrite the
-  // Arg operand.
+  // Don't specialize apply instructions if the result type references
+  // OpenedArchetype, because this optimization does not know how to substitute
+  // types in the users of this apply. In the function type substitution below,
+  // all references to OpenedArchetype will be substituted. So walk the type to
+  // find all possible references, such as returning Optional<OpenedArchetype>.
+  // The same holds for other arguments or indirect result that refer to the
+  // OpenedArchetype, because the following optimization will rewrite only the
+  // argument at ArgIdx.
   //
   // Note that the language does not allow Self to occur in contravariant
   // position. However, SIL does allow this and it can happen as a result of
   // upstream transformations. Since this is bail-out logic, it must handle
   // all verifiable SIL.
-
-  // This bailout check is also needed for non-Self arguments [including Self].
-  unsigned NumApplyArgs = Apply.getNumArguments();
-  for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
-    if (Idx == ArgIdx)
-      continue;
-    if (Apply.getArgument(Idx)->getType().getASTType().findIf(
-            [&OAI](Type t) -> bool {
-              return t->isEqual(OAI.OpenedArchetype);
-            })) {
-      return false;
-    }
+  if (applyInvolvesOpenedArchetypeWithRoot(Apply, OAI.OpenedArchetype,
+                                           ArgIdx)) {
+    return false;
   }
+
   // If the convention is mutating, then the existential must have been
   // initialized by copying the concrete value (regardless of whether
   // CEI.isConcreteValueCopied is true). Replacing the existential address with
@@ -1052,37 +1063,24 @@ SILValue SILCombiner::canCastArg(FullApplySite Apply,
       !CEI.ConcreteValue->getType().isAddress())
     return SILValue();
 
-  // Don't specialize apply instructions that return the callee's Arg type,
-  // because this optimization does not know how to substitute types in the
-  // users of this apply. In the function type substitution below, all
-  // references to OpenedArchetype will be substituted. So walk to type to
-  // find all possible references, such as returning Optional<Arg>.
-  if (Apply.getType().getASTType().findIf(
-          [&OAI](Type t) -> bool { return t->isEqual(OAI.OpenedArchetype); })) {
-    return SILValue();
-  }
-  // Bail out if any other arguments or indirect result that refer to the
-  // OpenedArchetype. The following optimization substitutes all occurrences
-  // of OpenedArchetype in the function signature, but will only rewrite the
-  // Arg operand.
+  // Don't specialize apply instructions if the result type references
+  // OpenedArchetype, because this optimization does not know how to substitute
+  // types in the users of this apply. In the function type substitution below,
+  // all references to OpenedArchetype will be substituted. So walk the type to
+  // find all possible references, such as returning Optional<OpenedArchetype>.
+  // The same holds for other arguments or indirect result that refer to the
+  // OpenedArchetype, because the following optimization will rewrite only the
+  // argument at ArgIdx.
   //
   // Note that the language does not allow Self to occur in contravariant
   // position. However, SIL does allow this and it can happen as a result of
   // upstream transformations. Since this is bail-out logic, it must handle
   // all verifiable SIL.
-
-  // This bailout check is also needed for non-Self arguments [including Self].
-  unsigned NumApplyArgs = Apply.getNumArguments();
-  for (unsigned Idx = 0; Idx < NumApplyArgs; ++Idx) {
-    if (Idx == ArgIdx)
-      continue;
-    if (Apply.getArgument(Idx)->getType().getASTType().findIf(
-            [&OAI](Type t) -> bool {
-              return t->isEqual(OAI.OpenedArchetype);
-            })) {
-      return SILValue();
-    }
+  if (applyInvolvesOpenedArchetypeWithRoot(Apply, OAI.OpenedArchetype,
+                                           ArgIdx)) {
+    return SILValue();
   }
+
   return Builder.createUncheckedAddrCast(
       Apply.getLoc(), Apply.getArgument(ArgIdx), CEI.ConcreteValue->getType());
 }
@@ -1293,10 +1291,11 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
 ///   %existential = alloc_stack $Protocol
 ///   %value = init_existential_addr %existential : $Concrete
 ///   copy_addr ... to %value
-///   %witness = witness_method $@opened
-///   apply %witness<T : Protocol>(%existential)
+///   %opened = open_existential_addr %existential
+///   %witness = witness_method $@opened(...) Protocol
+///   apply %witness<$@opened(...) Protocol>(%opened)
 ///
-/// ==> apply %witness<Concrete : Protocol>(%existential)
+/// ==> apply %witness<$Concrete>(%existential)
 SILInstruction *
 SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply,
                                                     WitnessMethodInst *WMI) {

lib/SILOptimizer/Utils/Devirtualize.cpp

@@ -1094,9 +1094,10 @@ static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
     return false;
   }
 
-  // FIXME: devirtualizeWitnessMethod below does not support cases with
-  // covariant 'Self' nested inside a collection type,
-  // like '[Self]' or '[* : Self]'.
+  // 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
+  // associated collection upcasts.
   const Type interfaceTy = wmi->getMember()
                                .getDecl()
                                ->getInterfaceType()
@@ -1107,35 +1108,35 @@ static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
   if (!interfaceTy->hasTypeParameter())
     return true;
 
-  class HasSelfNestedInsideCollection final : public TypeWalker {
-    unsigned CollectionDepth;
+  auto *const selfGP = wmi->getLookupProtocol()->getProtocolSelfType();
+  auto isSelfRootedTypeParameter = [selfGP](Type T) -> bool {
+    if (!T->hasTypeParameter())
+      return false;
 
-  public:
-    Action walkToTypePre(Type T) override {
-      if (!T->hasTypeParameter())
-        return Action::SkipChildren;
+    if (T->isTypeParameter()) {
+      return T->getRootGenericParam()->isEqual(selfGP);
+    }
 
-      if (auto *GP = T->getAs<GenericTypeParamType>()) {
-        // Only 'Self' will have zero depth in the type of a requirement.
-        if (GP->getDepth() == 0 && CollectionDepth)
-          return Action::Stop;
-      }
+    return false;
+  };
 
-      if (T->isArray() || T->isDictionary())
-        ++CollectionDepth;
+  return !interfaceTy.findIf([&](Type T) -> bool {
+    if (!T->hasTypeParameter())
+      return false;
 
-      return Action::Continue;
+    if (T->isArray() || T->isDictionary()) {
+      return T.findIf(isSelfRootedTypeParameter);
     }
 
-    Action walkToTypePost(Type T) override {
-      if (T->isArray() || T->isDictionary())
-        --CollectionDepth;
-
-      return Action::Continue;
+    if (auto *FT = T->getAs<FunctionType>()) {
+      for (const auto &Param : FT->getParams()) {
+        if (Param.isVariadic() && T.findIf(isSelfRootedTypeParameter))
+          return true;
+      }
     }
-  };
 
-  return !interfaceTy.walk(HasSelfNestedInsideCollection());
+    return false;
+  });
 }
 
 /// In the cases where we can statically determine the function that

lib/SILOptimizer/Utils/Existential.cpp

@@ -220,13 +220,13 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) {
     }
   }
   if (auto *Open = dyn_cast<OpenExistentialAddrInst>(openedVal)) {
-    OpenedArchetype = Open->getType().castTo<ArchetypeType>();
+    OpenedArchetype = Open->getType().castTo<OpenedArchetypeType>();
     OpenedArchetypeValue = Open;
     ExistentialValue = Open->getOperand();
     return;
   }
   if (auto *Open = dyn_cast<OpenExistentialRefInst>(openedVal)) {
-    OpenedArchetype = Open->getType().castTo<ArchetypeType>();
+    OpenedArchetype = Open->getType().castTo<OpenedArchetypeType>();
     OpenedArchetypeValue = Open;
     ExistentialValue = Open->getOperand();
     return;
@@ -235,7 +235,7 @@ OpenedArchetypeInfo::OpenedArchetypeInfo(Operand &use) {
     auto Ty = Open->getType().getASTType();
     while (auto Metatype = dyn_cast<MetatypeType>(Ty))
       Ty = Metatype.getInstanceType();
-    OpenedArchetype = cast<ArchetypeType>(Ty);
+    OpenedArchetype = cast<OpenedArchetypeType>(Ty);
     OpenedArchetypeValue = Open;
     ExistentialValue = Open->getOperand();
   }