[move-only] Update the non-address move checker part of the optimizer to handle mark_must_check on addresses.

Author: gottesmm

lib/SIL/Utils/MemAccessUtils.cpp

@@ -1368,12 +1368,14 @@ class AccessPathVisitor : public FindAccessVisitorImpl<AccessPathVisitor> {
     } else {
       // Ignore everything in getAccessProjectionOperand that is an access
       // projection with no affect on the access path.
-      assert(isa<OpenExistentialAddrInst>(projectedAddr)
-             || isa<InitEnumDataAddrInst>(projectedAddr)
-             || isa<UncheckedTakeEnumDataAddrInst>(projectedAddr)
+      assert(isa<OpenExistentialAddrInst>(projectedAddr) ||
+             isa<InitEnumDataAddrInst>(projectedAddr) ||
+             isa<UncheckedTakeEnumDataAddrInst>(projectedAddr)
              // project_box is not normally an access projection but we treat it
              // as such when it operates on unchecked_take_enum_data_addr.
-             || isa<ProjectBoxInst>(projectedAddr));
+             || isa<ProjectBoxInst>(projectedAddr)
+             // Ignore mark_must_check, we just look through it when we see it.
+             || isa<MarkMustCheckInst>(projectedAddr));
     }
     return sourceAddr->get();
   }
@@ -1862,7 +1864,14 @@ AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi,
     return IgnoredUse;
   }
 
-  // MARK: Access projections
+  case SILInstructionKind::MarkMustCheckInst: {
+    // Mark must check goes on the project_box, so it isn't a ref.
+    assert(!dfs.isRef());
+    pushUsers(svi, dfs);
+    return IgnoredUse;
+  }
+
+    // MARK: Access projections
 
   case SILInstructionKind::StructElementAddrInst:
   case SILInstructionKind::TupleElementAddrInst:

lib/SIL/Utils/OwnershipUtils.cpp

@@ -936,7 +936,7 @@ swift::findTransitiveUsesForAddress(SILValue projectedAddress,
         isa<InitExistentialAddrInst>(user) || isa<InitEnumDataAddrInst>(user) ||
         isa<BeginAccessInst>(user) || isa<TailAddrInst>(user) ||
         isa<IndexAddrInst>(user) || isa<StoreBorrowInst>(user) ||
-        isa<UncheckedAddrCastInst>(user)) {
+        isa<UncheckedAddrCastInst>(user) || isa<MarkMustCheckInst>(user)) {
       transitiveResultUses(op);
       continue;
     }

lib/SIL/Verifier/LoadBorrowImmutabilityChecker.cpp

@@ -169,6 +169,17 @@ bool GatherWritesVisitor::visitUse(Operand *op, AccessUseType useTy) {
     }
     return true;
 
+  case SILInstructionKind::ExplicitCopyAddrInst:
+    if (cast<ExplicitCopyAddrInst>(user)->getDest() == op->get()) {
+      writeAccumulator.push_back(op);
+      return true;
+    }
+    // This operand is the copy source. Check if it is taken.
+    if (cast<ExplicitCopyAddrInst>(user)->isTakeOfSrc()) {
+      writeAccumulator.push_back(op);
+    }
+    return true;
+
   case SILInstructionKind::MarkUnresolvedMoveAddrInst:
     if (cast<MarkUnresolvedMoveAddrInst>(user)->getDest() == op->get()) {
       writeAccumulator.push_back(op);

lib/SILGen/SILGenDecl.cpp

@@ -376,6 +376,11 @@ class LocalVariableInitialization : public SingleBufferInitialization {
     }
 
     Addr = SGF.B.createProjectBox(decl, Box, 0);
+    if (Addr->getType().isMoveOnly()) {
+      // TODO: Handle no implicit copy here.
+      Addr = SGF.B.createMarkMustCheckInst(
+          decl, Addr, MarkMustCheckInst::CheckKind::NoImplicitCopy);
+    }
 
     // Push a cleanup to destroy the local variable.  This has to be
     // inactive until the variable is initialized.

lib/SILOptimizer/Mandatory/AccessEnforcementSelection.cpp

@@ -31,6 +31,7 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include "swift/SIL/SILInstruction.h"
 #define DEBUG_TYPE "access-enforcement-selection"
 #include "swift/Basic/Defer.h"
 #include "swift/SIL/ApplySite.h"
@@ -194,7 +195,8 @@ class SelectEnforcement {
 
 private:
   void analyzeUsesOfBox(SingleValueInstruction *source);
-  void analyzeProjection(ProjectBoxInst *projection);
+  // Used for project_box and mark_must_initialize.
+  void analyzeProjection(SingleValueInstruction *project);
 
   /// Note that the given instruction is a use of the box (or a use of
   /// a projection from it) in which the address escapes.
@@ -237,13 +239,13 @@ void SelectEnforcement::analyzeUsesOfBox(SingleValueInstruction *source) {
   for (auto use : source->getUses()) {
     auto user = use->getUser();
 
-    if (auto BBI = dyn_cast<BeginBorrowInst>(user)) {
-      analyzeUsesOfBox(BBI);
+    if (auto bbi = dyn_cast<BeginBorrowInst>(user)) {
+      analyzeUsesOfBox(bbi);
       continue;
     }
 
-    if (auto MUI = dyn_cast<MarkUninitializedInst>(user)) {
-      analyzeUsesOfBox(MUI);
+    if (auto mui = dyn_cast<MarkUninitializedInst>(user)) {
+      analyzeUsesOfBox(mui);
       continue;
     }
 
@@ -281,10 +283,16 @@ static void checkUsesOfAccess(BeginAccessInst *access) {
 #endif
 }
 
-void SelectEnforcement::analyzeProjection(ProjectBoxInst *projection) {
+void SelectEnforcement::analyzeProjection(SingleValueInstruction *projection) {
   for (auto *use : projection->getUses()) {
     auto user = use->getUser();
 
+    // Look through mark must check.
+    if (auto *mmi = dyn_cast<MarkMustCheckInst>(user)) {
+      analyzeProjection(mmi);
+      continue;
+    }
+
     // Collect accesses.
     if (auto *access = dyn_cast<BeginAccessInst>(user)) {
       if (access->getEnforcement() == SILAccessEnforcement::Unknown)
@@ -687,8 +695,12 @@ AccessEnforcementSelection::getAccessKindForBox(ProjectBoxInst *projection) {
 
 SourceAccess AccessEnforcementSelection::getSourceAccess(SILValue address) {
   // Recurse through MarkUninitializedInst.
-  if (auto *MUI = dyn_cast<MarkUninitializedInst>(address))
-    return getSourceAccess(MUI->getOperand());
+  if (auto *mui = dyn_cast<MarkUninitializedInst>(address))
+    return getSourceAccess(mui->getOperand());
+
+  // Recurse through mark must check.
+  if (auto *mmci = dyn_cast<MarkMustCheckInst>(address))
+    return getSourceAccess(mmci->getOperand());
 
   if (auto box = dyn_cast<ProjectBoxInst>(address))
     return getAccessKindForBox(box);

lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp

@@ -747,6 +747,12 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
       continue;
     }
 
+    // Look through mark_must_check. To us, it is not interesting.
+    if (auto *mmi = dyn_cast<MarkMustCheckInst>(User)) {
+      collectUses(mmi, BaseEltNo);
+      continue;
+    }
+
     // Loads are a use of the value.
     if (isa<LoadInst>(User)) {
       addElementUses(BaseEltNo, PointeeType, User, DIUseKind::Load);
@@ -1588,6 +1594,12 @@ collectDelegatingInitUses(const DIMemoryObjectInfo &TheMemory,
       continue;
     }
 
+    // Look through mark_must_check.
+    if (auto *MMCI = dyn_cast<MarkMustCheckInst>(User)) {
+      collectDelegatingInitUses(TheMemory, UseInfo, MMCI);
+      continue;
+    }
+
     // Ignore end_access
     if (isa<EndAccessInst>(User))
       continue;

lib/SILOptimizer/Mandatory/DiagnoseInvalidEscapingCaptures.cpp

@@ -148,6 +148,10 @@ static bool checkNoEscapePartialApplyUse(Operand *oper, FollowUse followUses) {
 }
 
 const ParamDecl *getParamDeclFromOperand(SILValue value) {
+  // Look through mark must check.
+  if (auto *mmci = dyn_cast<MarkMustCheckInst>(value))
+    value = mmci->getOperand();
+
   if (auto *arg = dyn_cast<SILArgument>(value))
     if (auto *decl = dyn_cast_or_null<ParamDecl>(arg->getDecl()))
       return decl;
@@ -263,6 +267,13 @@ static void diagnoseCaptureLoc(ASTContext &Context, DeclContext *DC,
     if (isIncidentalUse(user) || onlyAffectsRefCount(user))
       continue;
 
+    // Look through mark must check inst.
+    if (auto *mmci = dyn_cast<MarkMustCheckInst>(user)) {
+      for (auto *use : mmci->getUses())
+        uselistInsert(use);
+      continue;
+    }
+
     // Look through copies, borrows, and conversions.
     if (SingleValueInstruction *copy = getSingleValueCopyOrCast(user)) {
       // Only follow the copied operand. Other operands are incidental,

lib/SILOptimizer/Mandatory/MoveOnlyObjectChecker.cpp

@@ -299,8 +299,13 @@ bool MoveOnlyChecker::searchForCandidateMarkMustChecks() {
       // We then RAUW the mark_must_check once we have emitted the error since
       // later passes expect that mark_must_check has been eliminated by
       // us. Since we are failing already, this is ok to do.
-      diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
-               diag::sil_moveonlychecker_not_understand_mark_move);
+      if (mmci->getType().isMoveOnlyWrapped()) {
+        diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
+                 diag::sil_moveonlychecker_not_understand_no_implicit_copy);
+      } else {
+        diagnose(fn->getASTContext(), mmci->getLoc().getSourceLoc(),
+                 diag::sil_moveonlychecker_not_understand_moveonly);
+      }
       mmci->replaceAllUsesWith(mmci->getOperand());
       mmci->eraseFromParent();
       changed = true;

lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "allocbox-to-stack"
+
 #include "swift/AST/DiagnosticsSIL.h"
 #include "swift/Basic/BlotMapVector.h"
 #include "swift/Basic/GraphNodeWorklist.h"
@@ -591,9 +592,16 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) {
   for (auto LastRelease : FinalReleases) {
     SILBuilderWithScope Builder(LastRelease);
     if (!isa<DeallocBoxInst>(LastRelease)&& !Lowering.isTrivial()) {
+      // If we have a mark_must_check use of our stack box, we want to destroy
+      // that.
+      SILValue valueToDestroy = StackBox;
+      if (auto *mmci = StackBox->getSingleUserOfType<MarkMustCheckInst>()) {
+        valueToDestroy = mmci;
+      }
+
       // For non-trivial types, insert destroys for each final release-like
       // instruction we found that isn't an explicit dealloc_box.
-      Builder.emitDestroyAddrAndFold(Loc, StackBox);
+      Builder.emitDestroyAddrAndFold(Loc, valueToDestroy);
     }
     Builder.createDeallocStack(Loc, ASI);
   }