Cleanup SILCombine code in preparation for a bug fix.

This is not NFC because the isUseAfterFree helper and surrounding code
is rewritten. The previous code's intention is unclear, but it was at
best imprecise and unsafe w.r.t. future SIL changes.

Pattern matching code that becomes highly complicated should
be commented with motivating SIL examples.

Author: atrick

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -601,7 +601,7 @@ SILCombiner::optimizeConcatenationOfStringLiterals(ApplyInst *AI) {
 }
 
 /// Returns the address of an object with which the stack location \p ASI is
-/// initialized. This is either a init_existential_addr or the source of a
+/// initialized. This is either a init_existential_addr or the destination of a
 /// copy_addr. Returns a null value if the address does not dominate the
 /// alloc_stack user \p ASIUser.
 /// If the value is copied from another stack location, \p isCopied is set to
@@ -808,21 +808,45 @@ SILCombiner::createApplyWithConcreteType(FullApplySite AI,
   return NewAI.getInstruction();
 }
 
+namespace {
+/// Record conformance and concrete type info derived from init_existential.
+struct ConformanceAndConcreteType {
+  Optional<ProtocolConformanceRef> Conformance;
+  // Concrete type of self from the found init_existential.
+  CanType ConcreteType;
+  // For opened existentials, record the type definition.
+  SingleValueInstruction *ConcreteTypeDef = nullptr;
+  // The value of concrete type used to initialize the existential.
+  SILValue NewSelf;
+  // The value that owns the lifetime of NewSelf.
+  // init_existential_addr's source address.
+  // init_existential_ref's defined value.
+  SILValue NewSelfOwner;
+  ArrayRef<ProtocolConformanceRef> Conformances;
+
+  ConformanceAndConcreteType(ASTContext &Ctx, FullApplySite AI,
+                             SILInstruction *InitExistential,
+                             ProtocolDecl *Protocol);
+
+  bool isValid() const {
+    return Conformance.hasValue() && ConcreteType && NewSelf;
+  }
+
+  ProtocolConformanceRef getConformance() const {
+    return Conformance.getValue();
+  }
+};
+} // namespace
+
 /// Derive a concrete type of self and conformance from the init_existential
 /// instruction.
-static Optional<std::tuple<ProtocolConformanceRef, CanType,
-                           SingleValueInstruction*, SILValue>>
-getConformanceAndConcreteType(ASTContext &Ctx,
-                              FullApplySite AI,
-                              SILInstruction *InitExistential,
-                              ProtocolDecl *Protocol,
-                              ArrayRef<ProtocolConformanceRef> &Conformances) {
-  // Try to derive the concrete type of self from the found init_existential.
-  CanType ConcreteType;
+/// If successful, initializes a valid ConformanceAndConcreteType.
+ConformanceAndConcreteType::ConformanceAndConcreteType(
+    ASTContext &Ctx, FullApplySite AI, SILInstruction *InitExistential,
+    ProtocolDecl *Protocol) {
+
   // The existential type result of the found init_existential.
   CanType ExistentialType;
-  SingleValueInstruction *ConcreteTypeDef = nullptr;
-  SILValue NewSelf;
 
   // FIXME: Factor this out. All we really need here is the ExistentialSig
   // below, which should be stored directly in the SILInstruction.
@@ -846,7 +870,8 @@ getConformanceAndConcreteType(ASTContext &Ctx,
       ConcreteType = cast<MetatypeType>(ConcreteType).getInstanceType();
     }
   } else {
-    return None;
+    assert(!isValid());
+    return;
   }
 
   // Construct a substitution map from the existential type's generic
@@ -860,9 +885,8 @@ getConformanceAndConcreteType(ASTContext &Ctx,
   // If the requirement is in a base protocol that is refined by the
   // conforming protocol, fish out the exact conformance for the base
   // protocol using the substitution map.
-  auto ExactConformance = SubMap.lookupConformance(
-    CanType(ExistentialSig->getGenericParams()[0]),
-    Protocol);
+  Conformance = SubMap.lookupConformance(
+      CanType(ExistentialSig->getGenericParams()[0]), Protocol);
 
   // If the concrete type is another existential, we're "forwarding" an
   // opened existential type, so we must keep track of the original
@@ -878,127 +902,136 @@ getConformanceAndConcreteType(ASTContext &Ctx,
           InitExistential->getTypeDependentOperands()[0].get());
     }
   }
-
-  return std::make_tuple(*ExactConformance, ConcreteType,
-                         ConcreteTypeDef, NewSelf);
+  assert(isValid());
 }
 
-static bool isUseAfterFree(SILValue val, SILInstruction *Apply,
-                           DominanceInfo *DT) {
-  for (auto Use : val->getUses()) {
-    auto *User = Use->getUser();
-    if (!isa<DeallocStackInst>(User) && !isa<DestroyAddrInst>(User) &&
-        !isa<DeinitExistentialAddrInst>(User)) {
-      continue;
-    }
-    if (!DT->properlyDominates(Apply, User)) {
-      // we have use-after-free - Conservative heuristic
-      // Non conservative solution would require data flow analysis
+// Return true if the given value is guaranteed to be initialized across the
+// given call site.
+//
+// It's possible for an address to be initialized/deinitialized/reinitialized.
+// Rather than keeping track of liveness, we very conservatively check that all
+// deinitialization occures after the call.
+//
+// FIXME: Rather than whitelisting, use a common AllocStackAnalyzer.
+static bool isAddressInitializedAtCall(SILValue addr, SILInstruction *AI,
+                                       DominanceInfo *DT) {
+  auto isDestroy = [](Operand *use) {
+    switch (use->getUser()->getKind()) {
+    default:
+      return false;
+    case SILInstructionKind::DestroyAddrInst:
+    case SILInstructionKind::DeinitExistentialAddrInst:
       return true;
+    case SILInstructionKind::CopyAddrInst: {
+      auto *copy = cast<CopyAddrInst>(use->getUser());
+      return copy->getSrc() == use->get() && copy->isTakeOfSrc();
+    }
+    }
+  };
+  for (auto use : addr->getUses()) {
+    SILInstruction *user = use->getUser();
+    if (isDestroy(use)) {
+      if (!DT->properlyDominates(AI, user))
+        return false;
+    } else {
+      assert(isa<CopyAddrInst>(user) || isa<InitExistentialAddrInst>(user)
+             || isa<OpenExistentialAddrInst>(user)
+             || isa<DeallocStackInst>(user)
+             || isDebugInst(user) && "Unexpected instruction");
     }
   }
-  return false;
+  return true;
 }
 
 /// Propagate information about a concrete type from init_existential_addr
 /// or init_existential_ref into witness_method conformances and into
 /// apply instructions.
 /// This helps the devirtualizer to replace witness_method by
 /// class_method instructions and then devirtualize.
-SILInstruction *
-SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
-    ProtocolDecl *Protocol,
-    llvm::function_ref<void(CanType , ProtocolConformanceRef)> Propagate) {
+SILInstruction *SILCombiner::propagateConcreteTypeOfInitExistential(
+    FullApplySite Apply, ProtocolDecl *Protocol,
+    llvm::function_ref<void(CanType, ProtocolConformanceRef)> Propagate) {
 
   ASTContext &Ctx = Builder.getASTContext();
 
   // Get the self argument.
-  SILValue Self;
-  if (auto *Apply = dyn_cast<ApplyInst>(AI)) {
-    if (Apply->hasSelfArgument())
-      Self = Apply->getSelfArgument();
-  } else if (auto *Apply = dyn_cast<TryApplyInst>(AI)) {
-    if (Apply->hasSelfArgument())
-      Self = Apply->getSelfArgument();
-  }
-
-  assert(Self && "Self argument should be present");
+  assert(Apply.hasSelfArgument() && "Self argument should be present");
+  SILValue Self = Apply.getSelfArgument();
 
   // Try to find the init_existential, which could be used to
   // determine a concrete type of the self.
   ArchetypeType *OpenedArchetype = nullptr;
   SILValue OpenedArchetypeDef;
   bool isCopied = false;
-  SILInstruction *InitExistential =
-    findInitExistential(AI, Self, OpenedArchetype, OpenedArchetypeDef,
-                        isCopied);
+  SILInstruction *InitExistential = findInitExistential(
+      Apply, Self, OpenedArchetype, OpenedArchetypeDef, isCopied);
   if (!InitExistential)
     return nullptr;
 
   // Try to derive the concrete type of self and a related conformance from
   // the found init_existential.
-  ArrayRef<ProtocolConformanceRef> Conformances;
-  auto ConformanceAndConcreteType =
-    getConformanceAndConcreteType(Ctx, AI, InitExistential,
-                                  Protocol, Conformances);
-  if (!ConformanceAndConcreteType)
+  ConformanceAndConcreteType CCT(Ctx, Apply, InitExistential, Protocol);
+  if (!CCT.isValid())
     return nullptr;
 
-  ProtocolConformanceRef Conformance = std::get<0>(*ConformanceAndConcreteType);
-  CanType ConcreteType = std::get<1>(*ConformanceAndConcreteType);
-  auto ConcreteTypeDef = std::get<2>(*ConformanceAndConcreteType);
-  SILValue NewSelf = std::get<3>(*ConformanceAndConcreteType);
-
   SILOpenedArchetypesTracker *OldOpenedArchetypesTracker =
       Builder.getOpenedArchetypesTracker();
 
-  SILOpenedArchetypesTracker OpenedArchetypesTracker(AI.getFunction());
+  SILOpenedArchetypesTracker OpenedArchetypesTracker(Apply.getFunction());
 
-  if (ConcreteType->isOpenedExistential()) {
-    // Prepare a mini-mapping for opened archetypes.
-    // SILOpenedArchetypesTracker OpenedArchetypesTracker(*AI.getFunction());
+  if (CCT.ConcreteType->isOpenedExistential()) {
+    // Temporarily record this opened existential def. Don't permanently record
+    // in the Builder's tracker because this opened existential's original
+    // dominating def may not have been recorded yet.
+    // FIXME: Redesign the tracker. This is not robust.
     OpenedArchetypesTracker.addOpenedArchetypeDef(
-        cast<ArchetypeType>(ConcreteType), ConcreteTypeDef);
+        cast<ArchetypeType>(CCT.ConcreteType), CCT.ConcreteTypeDef);
     Builder.setOpenedArchetypesTracker(&OpenedArchetypesTracker);
   }
 
   // Propagate the concrete type into the callee-operand if required.
-  Propagate(ConcreteType, Conformance);
+  Propagate(CCT.ConcreteType, CCT.getConformance());
 
-  if (isCopied) {
+  auto canReplaceCopiedSelf = [&]() {
     // If the witness method is mutating self, we cannot replace self with
     // the source of a copy. Otherwise the call would modify another value than
     // the original self.
-    switch (AI.getArgumentConvention(AI.getNumArguments() - 1)) {
-      case SILArgumentConvention::ConventionType::Indirect_Inout:
-      case SILArgumentConvention::ConventionType::Indirect_InoutAliasable:
-        return nullptr;
-      default:
-        break;
-    }
-    // check if using the value in the apply would cause use-after-free
-    auto *DT = DA->get(AI.getFunction());
-    auto *apply = AI.getInstruction();
-    auto op = InitExistential->getOperand(0);
-    if (isUseAfterFree(op, apply, DT)) {
-      return nullptr;
-    }
-    if (isUseAfterFree(NewSelf, apply, DT)) {
-      return nullptr;
-    }
-  }
+    if (Apply.getOrigCalleeType()->getSelfParameter().isIndirectMutating())
+      return false;
+
+    auto *DT = DA->get(Apply.getFunction());
+    auto *AI = Apply.getInstruction();
+    // Only init_existential_addr may be copied.
+    SILValue existentialAddr =
+        cast<InitExistentialAddrInst>(InitExistential)->getOperand();
+    return isAddressInitializedAtCall(existentialAddr, AI, DT);
+  };
+
+  // FIXME: Intentionally preserve this bug to avoid changing functionality.
+  if (isCopied && !canReplaceCopiedSelf())
+    return nullptr;
+
   // Create a new apply instruction that uses the concrete type instead
   // of the existential type.
-  auto *NewAI = createApplyWithConcreteType(AI, NewSelf, Self, ConcreteType,
-                                            ConcreteTypeDef, Conformance,
-                                            OpenedArchetype);
+  auto *NewAI = createApplyWithConcreteType(
+      Apply, CCT.NewSelf, Self, CCT.ConcreteType, CCT.ConcreteTypeDef,
+      CCT.getConformance(), OpenedArchetype);
 
-  if (ConcreteType->isOpenedExistential())
+  if (CCT.ConcreteType->isOpenedExistential())
     Builder.setOpenedArchetypesTracker(OldOpenedArchetypesTracker);
 
   return NewAI;
 }
 
+/// Rewrite a witness method's lookup type from an archetype to a concrete type.
+/// Example:
+///   %existential = alloc_stack $Protocol
+///   %value = init_existential_addr %existential : $Concrete
+///   copy_addr ... to %value
+///   %witness = witness_method $@opened
+///   apply %witness<T : Protocol>(%existential)
+///
+/// ==> apply %witness<Concrete : Protocol>(%existential)
 SILInstruction *
 SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
                                                     WitnessMethodInst *WMI) {
@@ -1097,7 +1130,7 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI) {
       return nullptr;
   }
 
-  // Obtain the protocol whose which should be used by the conformance.
+  // Obtain the protocol which should be used by the conformance.
   auto *AFD = dyn_cast<AbstractFunctionDecl>(Callee->getDeclContext());
   if (!AFD)
     return nullptr;