SILCombiner: Clean up the concrete -> existential peephole a bit

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -692,54 +692,41 @@ SILCombiner::createApplyWithConcreteType(FullApplySite AI,
   // replaced by a concrete type.
   SmallVector<Substitution, 8> Substitutions;
   for (auto Subst : AI.getSubstitutions()) {
-    auto *A = Subst.getReplacement()->getAs<ArchetypeType>();
-    if (A && A == OpenedArchetype) {
-      auto Conformances = AI.getModule().getASTContext()
-                            .AllocateUninitialized<ProtocolConformanceRef>(1);
-      Conformances[0] = Conformance;
-      Substitution NewSubst(ConcreteType, Conformances);
-      Substitutions.push_back(NewSubst);
-    } else
-      Substitutions.push_back(Subst);
+    auto NewSubst = Subst.subst(
+      AI.getModule().getSwiftModule(),
+      [&](SubstitutableType *type) -> Type {
+        if (type == OpenedArchetype)
+          return ConcreteType;
+        return type;
+      },
+      [&](Type origTy, Type substTy, ProtocolType *protoType)
+        -> Optional<ProtocolConformanceRef> {
+        assert(origTy->isEqual(OpenedArchetype));
+        return Conformance;
+      });
+    Substitutions.push_back(NewSubst);
   }
 
-  SILType SubstCalleeType = AI.getSubstCalleeSILType();
-
-  SILType NewSubstCalleeType;
-
   auto FnTy = AI.getCallee()->getType().castTo<SILFunctionType>();
-  if (FnTy->isPolymorphic()) {
-    // Handle polymorphic functions by properly substituting
-    // their parameter types.
-    CanSILFunctionType SFT = FnTy->substGenericArgs(
-                                        AI.getModule(),
-                                        Substitutions);
-    NewSubstCalleeType = SILType::getPrimitiveObjectType(SFT);
-  } else {
-    NewSubstCalleeType =
-      SubstCalleeType.subst(AI.getModule(),
-                            [&](SubstitutableType *type) -> Type {
-                              if (type == OpenedArchetype)
-                                return ConcreteType;
-                              return type;
-                            },
-                            MakeAbstractConformanceForGenericType());
-  }
+  assert(FnTy->isPolymorphic());
+
+  auto SFT = FnTy->substGenericArgs(AI.getModule(), Substitutions);
+  auto NewSubstCalleeType = SILType::getPrimitiveObjectType(SFT);
 
   FullApplySite NewAI;
   Builder.setCurrentDebugScope(AI.getDebugScope());
   Builder.addOpenedArchetypeOperands(AI.getInstruction());
 
   if (auto *TAI = dyn_cast<TryApplyInst>(AI))
     NewAI = Builder.createTryApply(AI.getLoc(), AI.getCallee(),
-                                    NewSubstCalleeType,
-                                    Substitutions, Args,
-                                    TAI->getNormalBB(), TAI->getErrorBB());
+                                   NewSubstCalleeType,
+                                   Substitutions, Args,
+                                   TAI->getNormalBB(), TAI->getErrorBB());
   else
     NewAI = Builder.createApply(AI.getLoc(), AI.getCallee(),
-                                 NewSubstCalleeType,
-                                 AI.getType(), Substitutions, Args,
-                                 cast<ApplyInst>(AI)->isNonThrowing());
+                                NewSubstCalleeType,
+                                AI.getType(), Substitutions, Args,
+                                cast<ApplyInst>(AI)->isNonThrowing());
 
   if (isa<ApplyInst>(NewAI))
     replaceInstUsesWith(*AI.getInstruction(), NewAI.getInstruction());
@@ -886,21 +873,6 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
   if (WMI->getConformance().isConcrete())
     return nullptr;
 
-  // Don't specialize Apply instructions that return the Self type.
-  // Notice that it is sufficient to compare the return type to the
-  // substituted type because types that depend on the Self type are
-  // not allowed (for example [Self] is not allowed).
-  if (AI.getType().getSwiftRValueType() == WMI->getLookupType())
-    return nullptr;
-
-  // We need to handle the Self return type.
-  // In we find arguments that are not the 'self' argument and if
-  // they are of the Self type then we abort the optimization.
-  for (auto Arg : AI.getArgumentsWithoutSelf()) {
-    if (Arg->getType().getSwiftRValueType() == WMI->getLookupType())
-      return nullptr;
-  }
-
   // The lookup type is not an opened existential type,
   // thus it cannot be made more concrete.
   if (!WMI->getLookupType()->isOpenedExistential())
@@ -916,10 +888,10 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
     // Keep around the dependence on the open instruction unless we've
     // actually eliminated the use.
     auto *NewWMI = Builder.createWitnessMethod(WMI->getLoc(),
-                                                ConcreteType,
-                                                Conformance, WMI->getMember(),
-                                                WMI->getType(),
-                                                WMI->isVolatile());
+                                               ConcreteType,
+                                               Conformance, WMI->getMember(),
+                                               WMI->getType(),
+                                               WMI->isVolatile());
     // Replace only uses of the witness_method in the apply that is going to
     // be changed.
     MutableArrayRef<Operand> Operands = AI.getInstruction()->getAllOperands();
@@ -957,15 +929,6 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI) {
   if (!Self)
     return nullptr;
 
-  // We need to handle the Self return type.
-  // In we find arguments that are not the 'self' argument and if
-  // they are of the Self type then we abort the optimization.
-  for (auto Arg : AI.getArgumentsWithoutSelf()) {
-    if (Arg->getType().getSwiftRValueType() ==
-        AI.getArguments().back()->getType().getSwiftRValueType())
-      return nullptr;
-  }
-
   // Obtain the protocol whose which should be used by the conformance.
   auto *AFD = dyn_cast<AbstractFunctionDecl>(Callee->getDeclContext());
   if (!AFD)

test/SILOptimizer/devirtualize_existential.swift

@@ -8,12 +8,61 @@ class Foo : Pingable {
 }
 
 // Everything gets devirtualized, inlined, and promoted to the stack.
-//CHECK: @_T024devirtualize_existential17interesting_stuffyyF
-//CHECK-NOT: init_existential_addr
-//CHECK-NOT: apply
-//CHECK: return
+// CHECK-LABEL: @_T024devirtualize_existential17interesting_stuffyyF
+// CHECK-NOT: init_existential_addr
+// CHECK-NOT: apply
+// CHECK: return
 public func interesting_stuff() {
- var x : Pingable = Foo()
+ var x: Pingable = Foo()
  x.ping(1)
 }
 
+protocol Cloneable {
+  func clone() -> Self
+  func maybeClone() -> Self?
+}
+
+struct Bar : Cloneable {
+  @inline(never)
+  func clone() -> Bar { return self }
+
+  @inline(never)
+  func maybeClone() -> Bar? { return self }
+}
+
+// In this example, we don't eliminate the init_existential_addr, because
+// of the stack allocated existential value used for the return.
+//
+// If the combiner was generalized to replace the opened existential type
+// with the concrete type in all instructions that use it, instead of just
+// special-casing witness_method and apply, we could eliminate the opened
+// existential type completely.
+//
+// However even though IRGen still performs more work at runtime than is
+// necessary here, the call is devirtualized.
+
+// CHECK-LABEL: sil @_T024devirtualize_existential22more_interesting_stuffyyF
+// CHECK: [[EXISTENTIAL:%.*]] = alloc_stack $Cloneable
+// CHECK: [[EXISTENTIAL_ADDR:%.*]] = init_existential_addr [[EXISTENTIAL]]
+// CHECK: [[VALUE:%.*]] = struct $Bar ()
+// CHECK: [[RESULT_ADDR:%.*]] = unchecked_addr_cast [[EXISTENTIAL_ADDR:%.*]]
+// CHECK: [[FN:%.*]] = function_ref @_T024devirtualize_existential3BarV5cloneACyF
+// CHECK: [[RETURN:%.*]] = apply [[FN]]([[VALUE]])
+// CHECK: store [[RETURN]] to [[RESULT_ADDR]]
+
+// CHECK: [[ENUM:%.*]] = alloc_stack $Optional<Cloneable>
+// CHECK: [[ENUM_ADDR:%.*]] = init_enum_data_addr [[ENUM]]
+// CHECK: [[EXISTENTIAL_ADDR:%.*]] = init_existential_addr [[ENUM_ADDR]]
+// CHECK: [[RESULT_ADDR:%.*]] = unchecked_addr_cast [[EXISTENTIAL_ADDR:%.*]]
+// CHECK: [[FN:%.*]] = function_ref @_T024devirtualize_existential3BarV10maybeCloneACSgyF
+// CHECK: [[RETURN:%.*]] = apply [[FN]]([[VALUE]])
+// CHECK: store [[RETURN]] to [[RESULT_ADDR]]
+
+// CHECK: return
+
+public func more_interesting_stuff() {
+  var x: Cloneable = Bar()
+
+  x.clone()
+  x.maybeClone()
+}