Merge pull request #65871 from gottesmm/release-5.9-more-batched

[5.9][move-only] A series of batched commits.

Author: gottesmm

include/swift/SILOptimizer/PassManager/Passes.def

@@ -112,9 +112,6 @@ PASS(AccessMarkerElimination, "access-marker-elim",
      "Access Marker Elimination.")
 PASS(AddressLowering, "address-lowering",
      "SIL Address Lowering")
-PASS(EarlyAllocBoxToStack, "early-allocbox-to-stack",
-     "Stack Promotion of Box Objects. Doesn't attempt to promote noncopyable "
-     "types captured by escaping closures")
 PASS(AllocBoxToStack, "allocbox-to-stack",
      "Stack Promotion of Box Objects")
 IRGEN_PASS(AllocStackHoisting, "alloc-stack-hoisting",

lib/SILGen/SILGenApply.cpp

@@ -4261,7 +4261,19 @@ static void emitBorrowedLValueRecursive(SILGenFunction &SGF,
     }
   }
 
+  // TODO: This does not take into account resilience, we should probably use
+  // getArgumentType()... but we do not have the SILFunctionType here...
   assert(param.getInterfaceType() == value.getType().getASTType());
+
+  // If we have an indirect_guaranteed argument, move this using store_borrow
+  // into an alloc_stack.
+  if (SGF.silConv.useLoweredAddresses() &&
+      param.isIndirectInGuaranteed() && value.getType().isObject()) {
+    SILValue alloca = SGF.emitTemporaryAllocation(loc, value.getType());
+    value = SGF.emitFormalEvaluationManagedStoreBorrow(loc, value.getValue(),
+                                                       alloca);
+  }
+
   args[argIndex++] = value;
 }
 

lib/SILGen/SILGenBuilder.cpp

@@ -778,9 +778,19 @@ ManagedValue SILGenBuilder::createStoreBorrow(SILLocation loc,
                                               SILValue address) {
   assert(value.getOwnershipKind() == OwnershipKind::Guaranteed);
   auto *sbi = createStoreBorrow(loc, value.getValue(), address);
+  SGF.Cleanups.pushCleanup<EndBorrowCleanup>(sbi);
   return ManagedValue(sbi, CleanupHandle::invalid());
 }
 
+ManagedValue SILGenBuilder::createFormalAccessStoreBorrow(SILLocation loc,
+                                                          ManagedValue value,
+                                                          SILValue address) {
+  assert(value.getOwnershipKind() == OwnershipKind::Guaranteed);
+  auto *sbi = createStoreBorrow(loc, value.getValue(), address);
+  return SGF.emitFormalEvaluationManagedBorrowedRValueWithCleanup(
+      loc, value.getValue(), sbi);
+}
+
 ManagedValue SILGenBuilder::createStoreBorrowOrTrivial(SILLocation loc,
                                                        ManagedValue value,
                                                        SILValue address) {

lib/SILGen/SILGenBuilder.h

@@ -202,6 +202,8 @@ class SILGenBuilder : public SILBuilder {
   using SILBuilder::createStoreBorrow;
   ManagedValue createStoreBorrow(SILLocation loc, ManagedValue value,
                                  SILValue address);
+  ManagedValue createFormalAccessStoreBorrow(SILLocation loc, ManagedValue value,
+                                             SILValue address);
 
   /// Create a store_borrow if we have a non-trivial value and a store [trivial]
   /// otherwise.

lib/SILGen/SILGenProlog.cpp

@@ -39,8 +39,12 @@ static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag,
 }
 
 SILValue SILGenFunction::emitSelfDeclForDestructor(VarDecl *selfDecl) {
+  SILFunctionConventions conventions = F.getConventionsInContext();
+
   // Emit the implicit 'self' argument.
-  SILType selfType = getLoweredType(selfDecl->getType());
+  SILType selfType = conventions.getSILArgumentType(
+      conventions.getNumSILArguments() - 1, F.getTypeExpansionContext());
+  selfType = F.mapTypeIntoContext(selfType);
   SILValue selfValue = F.begin()->createFunctionArgument(selfType, selfDecl);
 
   // If we have a move only type, then mark it with mark_must_check so we can't

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerTester.cpp

@@ -105,20 +105,18 @@ class MoveOnlyAddressCheckerTesterPass : public SILFunctionTransform {
                << '\n');
 
     bool madeChange = false;
-    unsigned diagCount = 0;
     if (moveIntroducersToProcess.empty()) {
       LLVM_DEBUG(llvm::dbgs() << "No move introducers found?!\n");
     } else {
       borrowtodestructure::IntervalMapAllocator allocator;
       MoveOnlyAddressChecker checker{getFunction(), diagnosticEmitter,
                                      allocator, domTree, poa};
       madeChange = checker.check(moveIntroducersToProcess);
-      diagCount = checker.diagnosticEmitter.getDiagnosticCount();
     }
 
     // If we did not emit any diagnostics, emit a diagnostic if we missed any
     // copies.
-    if (!diagCount) {
+    if (!diagnosticEmitter.emittedDiagnostic()) {
       emitCheckerMissedCopyOfNonCopyableTypeErrors(getFunction(),
                                                    diagnosticEmitter);
     }

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -225,6 +225,7 @@
 #include "swift/AST/AccessScope.h"
 #include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/DiagnosticsSIL.h"
+#include "swift/AST/SemanticAttrs.h"
 #include "swift/Basic/Debug.h"
 #include "swift/Basic/Defer.h"
 #include "swift/Basic/FrozenMultiMap.h"
@@ -433,7 +434,7 @@ static bool memInstMustConsume(Operand *memOper) {
       return false;
     ApplySite applySite(pai);
     auto convention = applySite.getArgumentConvention(*memOper);
-    return convention.isInoutConvention();
+    return !convention.isInoutConvention();
   }
   case SILInstructionKind::DestroyAddrInst:
     return true;
@@ -492,6 +493,92 @@ static bool isInOutDefThatNeedsEndOfFunctionLiveness(MarkMustCheckInst *markedAd
   return false;
 }
 
+//===----------------------------------------------------------------------===//
+//                       MARK: Partial Apply Utilities
+//===----------------------------------------------------------------------===//
+
+static bool findNonEscapingPartialApplyUses(
+    PartialApplyInst *pai, TypeTreeLeafTypeRange leafRange,
+    llvm::SmallMapVector<SILInstruction *, TypeTreeLeafTypeRange, 4>
+        &livenessUses) {
+  StackList<Operand *> worklist(pai->getFunction());
+  for (auto *use : pai->getUses())
+    worklist.push_back(use);
+
+  LLVM_DEBUG(llvm::dbgs() << "Searching for partial apply uses!\n");
+  while (!worklist.empty()) {
+    auto *use = worklist.pop_back_val();
+
+    if (use->isTypeDependent())
+      continue;
+
+    auto *user = use->getUser();
+
+    // These instructions do not cause us to escape.
+    if (isIncidentalUse(user) || isa<DestroyValueInst>(user))
+      continue;
+
+    // Look through these instructions.
+    if (isa<BeginBorrowInst>(user) || isa<CopyValueInst>(user) ||
+        isa<MoveValueInst>(user) ||
+        // If we capture this partial_apply in another partial_apply, then we
+        // know that said partial_apply must not have escaped the value since
+        // otherwise we could not have an inout_aliasable argument or be
+        // on_stack. Process it recursively so that we treat uses of that
+        // partial_apply and applies of that partial_apply as uses of our
+        // partial_apply.
+        //
+        // We have this separately from the other look through sections so that
+        // we can make it clearer what we are doing here.
+        isa<PartialApplyInst>(user)) {
+      for (auto *use : cast<SingleValueInstruction>(user)->getUses())
+        worklist.push_back(use);
+      continue;
+    }
+
+    // If we have a mark_dependence and are the value, look through the
+    // mark_dependence.
+    if (auto *mdi = dyn_cast<MarkDependenceInst>(user)) {
+      if (mdi->getValue() == use->get()) {
+        for (auto *use : mdi->getUses())
+          worklist.push_back(use);
+        continue;
+      }
+    }
+
+    if (auto apply = FullApplySite::isa(user)) {
+      // If we apply the function or pass the function off to an apply, then we
+      // need to treat the function application as a liveness use of the
+      // variable since if the partial_apply is invoked within the function
+      // application, we may access the captured variable.
+      livenessUses.insert({user, leafRange});
+      if (apply.beginsCoroutineEvaluation()) {
+        // If we have a coroutine, we need to treat the abort_apply and
+        // end_apply as liveness uses since once we execute one of those
+        // instructions, we have returned control to the coroutine which means
+        // that we could then access the captured variable again.
+        auto *bai = cast<BeginApplyInst>(user);
+        SmallVector<EndApplyInst *, 4> endApplies;
+        SmallVector<AbortApplyInst *, 4> abortApplies;
+        bai->getCoroutineEndPoints(endApplies, abortApplies);
+        for (auto *eai : endApplies)
+          livenessUses.insert({eai, leafRange});
+        for (auto *aai : abortApplies)
+          livenessUses.insert({aai, leafRange});
+      }
+      continue;
+    }
+
+    LLVM_DEBUG(
+        llvm::dbgs()
+        << "Found instruction we did not understand... returning false!\n");
+    LLVM_DEBUG(llvm::dbgs() << "Instruction: " << *user);
+    return false;
+  }
+
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 //                   MARK: Find Candidate Mark Must Checks
 //===----------------------------------------------------------------------===//
@@ -1354,7 +1441,6 @@ struct GatherUsesVisitor final : public TransitiveAddressWalker {
   MoveOnlyAddressCheckerPImpl &moveChecker;
   UseState &useState;
   MarkMustCheckInst *markedValue;
-  bool emittedEarlyDiagnostic = false;
   DiagnosticEmitter &diagnosticEmitter;
 
   // Pruned liveness used to validate that load [take]/load [copy] can be
@@ -1529,14 +1615,13 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
       if (isa<ProjectBoxInst>(stripAccessMarkers(markedValue->getOperand()))) {
         LLVM_DEBUG(llvm::dbgs()
                    << "Found mark must check [nocopy] use of escaping box: " << *user);
-        diagnosticEmitter.emitAddressEscapingClosureCaptureLoadedAndConsumed(markedValue);
-        emittedEarlyDiagnostic = true;
+        diagnosticEmitter.emitAddressEscapingClosureCaptureLoadedAndConsumed(
+            markedValue);
         return true;
       }
       LLVM_DEBUG(llvm::dbgs()
                  << "Found mark must check [nocopy] error: " << *user);
       diagnosticEmitter.emitAddressDiagnosticNoCopy(markedValue, copyAddr);
-      emittedEarlyDiagnostic = true;
       return true;
     }
 
@@ -1592,15 +1677,13 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
                  .didEmitCheckerDoesntUnderstandDiagnostic());
       LLVM_DEBUG(llvm::dbgs()
                  << "Failed to perform borrow to destructure transform!\n");
-      emittedEarlyDiagnostic = true;
       return false;
     }
 
     // If we emitted an error diagnostic, do not transform further and instead
     // mark that we emitted an early diagnostic and return true.
     if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
       LLVM_DEBUG(llvm::dbgs() << "Emitting borrow to destructure error!\n");
-      emittedEarlyDiagnostic = true;
       return true;
     }
 
@@ -1621,7 +1704,6 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
     if (numDiagnostics != moveChecker.diagnosticEmitter.getDiagnosticCount()) {
       LLVM_DEBUG(llvm::dbgs()
                  << "Emitting destructure through deinit error!\n");
-      emittedEarlyDiagnostic = true;
       return true;
     }
 
@@ -1639,14 +1721,18 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
                    << "Found mark must check [nocopy] error: " << *user);
         auto *fArg = dyn_cast<SILFunctionArgument>(
             stripAccessMarkers(markedValue->getOperand()));
-        if (fArg && fArg->isClosureCapture() && fArg->getType().isAddress()) {
-          moveChecker.diagnosticEmitter.emitPromotedBoxArgumentError(
-              markedValue, fArg);
+        // If we have a closure captured that we specialized, we should have a
+        // no consume or assign and should emit a normal guaranteed diagnostic.
+        if (fArg && fArg->isClosureCapture() &&
+            fArg->getArgumentConvention().isInoutConvention()) {
+          assert(checkKind == MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
+          moveChecker.diagnosticEmitter.emitObjectGuaranteedDiagnostic(
+              markedValue);
         } else {
+          // Otherwise, we need to emit an escaping closure error.
           moveChecker.diagnosticEmitter
               .emitAddressEscapingClosureCaptureLoadedAndConsumed(markedValue);
         }
-        emittedEarlyDiagnostic = true;
         return true;
       }
 
@@ -1660,7 +1746,6 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
 
       if (checkForExclusivityHazards(li)) {
         LLVM_DEBUG(llvm::dbgs() << "Found exclusivity violation?!\n");
-        emittedEarlyDiagnostic = true;
         return true;
       }
 
@@ -1693,7 +1778,6 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
       // succeeded.
       // Otherwise, emit the diagnostic.
       moveChecker.diagnosticEmitter.emitObjectOwnedDiagnostic(markedValue);
-      emittedEarlyDiagnostic = true;
       LLVM_DEBUG(llvm::dbgs() << "Emitted early object level diagnostic.\n");
       return true;
     }
@@ -1702,7 +1786,6 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
       LLVM_DEBUG(llvm::dbgs() << "Found potential borrow inst: " << *user);
       if (checkForExclusivityHazards(li)) {
         LLVM_DEBUG(llvm::dbgs() << "Found exclusivity violation?!\n");
-        emittedEarlyDiagnostic = true;
         return true;
       }
 
@@ -1751,7 +1834,6 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
         moveChecker.diagnosticEmitter
             .emitAddressEscapingClosureCaptureLoadedAndConsumed(markedValue);
       }
-      emittedEarlyDiagnostic = true;
       return true;
     }
 
@@ -1802,8 +1884,22 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
   }
 
   if (auto *pas = dyn_cast<PartialApplyInst>(user)) {
-    if (pas->isOnStack()) {
-      LLVM_DEBUG(llvm::dbgs() << "Found on stack partial apply!\n");
+    if (auto *fArg = dyn_cast<SILFunctionArgument>(
+            stripAccessMarkers(markedValue->getOperand()))) {
+      // If we are processing an inout convention and we emitted an error on the
+      // partial_apply, we shouldn't process this mark_must_check, but squelch
+      // the compiler doesn't understand error.
+      if (fArg->getArgumentConvention().isInoutConvention() &&
+          pas->getCalleeFunction()->hasSemanticsAttr(
+              semantics::NO_MOVEONLY_DIAGNOSTICS)) {
+        diagnosticEmitter.emitEarlierPassEmittedDiagnostic(markedValue);
+        return false;
+      }
+    }
+
+    if (pas->isOnStack() ||
+        ApplySite(pas).getArgumentConvention(*op).isInoutConvention()) {
+      LLVM_DEBUG(llvm::dbgs() << "Found on stack partial apply or inout usage!\n");
       // On-stack partial applications and their final consumes are always a
       // liveness use of their captures.
       auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
@@ -1812,13 +1908,18 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
         return false;
       }
 
-      useState.livenessUses.insert({user, *leafRange});
-      for (auto *use : pas->getConsumingUses()) {
-        LLVM_DEBUG(llvm::dbgs()
-                   << "Adding consuming use of partial apply as liveness use: "
-                   << *use->getUser());
-        useState.livenessUses.insert({use->getUser(), *leafRange});
+      // Attempt to find calls of the non-escaping partial apply and places
+      // where the partial apply is passed to a function. We treat those as
+      // liveness uses. If we find a use we don't understand, we return false
+      // here.
+      if (!findNonEscapingPartialApplyUses(pas, *leafRange,
+                                           useState.livenessUses)) {
+        LLVM_DEBUG(
+            llvm::dbgs()
+            << "Failed to understand use of a non-escaping partial apply?!\n");
+        return false;
       }
+
       return true;
     }
   }
@@ -2543,9 +2644,19 @@ bool MoveOnlyAddressChecker::check(
     LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *markedValue);
 
     // Perform our address check.
+    unsigned diagnosticEmittedByEarlierPassCount =
+      diagnosticEmitter.getDiagnosticEmittedByEarlierPassCount();
     if (!pimpl.performSingleCheck(markedValue)) {
-      LLVM_DEBUG(llvm::dbgs()
-                 << "Failed to perform single check! Emitting error!\n");
+      if (diagnosticEmittedByEarlierPassCount !=
+          diagnosticEmitter.getDiagnosticEmittedByEarlierPassCount()) {
+        LLVM_DEBUG(
+            llvm::dbgs()
+            << "Failed to perform single check but found earlier emitted "
+               "error. Not emitting checker doesn't understand diagnostic!\n");
+        continue;
+      }
+      LLVM_DEBUG(llvm::dbgs() << "Failed to perform single check! Emitting "
+                                 "compiler doesn't understand diagnostic!\n");
       // If we fail the address check in some way, set the diagnose!
       diagnosticEmitter.emitCheckerDoesntUnderstandDiagnostic(markedValue);
     }

lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp

@@ -179,7 +179,7 @@ class MoveOnlyCheckerPass : public SILFunctionTransform {
     // remain. If we emitted a diagnostic, we just want to rewrite all of the
     // non-copyable copies into explicit variants below and let the user
     // recompile.
-    if (!checker.diagnosticEmitter.getDiagnosticCount()) {
+    if (!checker.diagnosticEmitter.emittedDiagnostic()) {
       emitCheckerMissedCopyOfNonCopyableTypeErrors(getFunction(),
                                                    checker.diagnosticEmitter);
     }