Merge pull request #40423 from gottesmm/address-only-let-checker

[move-function] Implement move checking for address only lets

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -741,7 +741,7 @@ NOTE(sil_movekillscopyablevalue_use_here, none,
 NOTE(sil_movekillscopyablevalue_value_consumed_in_loop, none,
      "cyclic move here. move will occur multiple times in the loop", ())
 ERROR(sil_movekillscopyablevalue_move_applied_to_unsupported_move, none,
-      "_move applied to value that the compiler does not supporting checking.",
+      "_move applied to value that the compiler does not support checking",
       ())
 
 #define UNDEFINE_DIAGNOSTIC_MACROS

include/swift/SIL/BasicBlockDatastructures.h

@@ -43,6 +43,10 @@ class BasicBlockSetVector {
   iterator begin() const { return vector.begin(); }
   iterator end() const { return vector.end(); }
 
+  llvm::iterator_range<iterator> getRange() const {
+    return llvm::make_range(begin(), end());
+  }
+
   bool empty() const { return vector.empty(); }
 
   bool contains(SILBasicBlock *block) const { return set.contains(block); }

include/swift/SIL/SILBuilder.h

@@ -1271,6 +1271,13 @@ class SILBuilder {
                   MoveValueInst(getSILDebugLocation(loc), operand));
   }
 
+  MarkUnresolvedMoveAddrInst *createMarkUnresolvedMoveAddr(SILLocation loc,
+                                                           SILValue srcAddr,
+                                                           SILValue takeAddr) {
+    return insert(new (getModule()) MarkUnresolvedMoveAddrInst(
+        getSILDebugLocation(loc), srcAddr, takeAddr));
+  }
+
   UnconditionalCheckedCastInst *
   createUnconditionalCheckedCast(SILLocation Loc, SILValue op,
                                  SILType destLoweredTy,

include/swift/SIL/SILCloner.h

@@ -1341,6 +1341,16 @@ SILCloner<ImplClass>::visitCopyAddrInst(CopyAddrInst *Inst) {
                 Inst->isInitializationOfDest()));
 }
 
+template <typename ImplClass>
+void SILCloner<ImplClass>::visitMarkUnresolvedMoveAddrInst(
+    MarkUnresolvedMoveAddrInst *Inst) {
+  getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
+  auto *MVI = getBuilder().createMarkUnresolvedMoveAddr(
+      getOpLocation(Inst->getLoc()), getOpValue(Inst->getSrc()),
+      getOpValue(Inst->getDest()));
+  recordClonedInstruction(Inst, MVI);
+}
+
 template<typename ImplClass>
 void
 SILCloner<ImplClass>::visitBindMemoryInst(BindMemoryInst *Inst) {

include/swift/SIL/SILInstruction.h

@@ -1793,6 +1793,10 @@ struct SILDebugVariable {
            Implicit == V.Implicit && Type == V.Type && Loc == V.Loc &&
            Scope == V.Scope;
   }
+
+  bool isLet() const { return Name.size() && Constant; }
+
+  bool isVar() const { return Name.size() && !Constant; }
 };
 
 /// A DebugVariable where storage for the strings has been
@@ -2004,7 +2008,20 @@ class AllocStackInst final
     auto VI = TailAllocatedDebugVariable(RawValue);
     return VI.get(getDecl(), getTrailingObjects<char>(), AuxVarType, VarDeclLoc,
                   VarDeclScope, DIExprElements);
-  };
+  }
+
+  bool isLet() const {
+    if (auto varInfo = getVarInfo())
+      return varInfo->isLet();
+    return false;
+  }
+
+  bool isVar() const {
+    if (auto varInfo = getVarInfo())
+      return varInfo->isVar();
+    return false;
+  }
+
   void setArgNo(unsigned N) {
     auto RawValue = SILNode::Bits.AllocStackInst.VarInfo;
     auto VI = TailAllocatedDebugVariable(RawValue);
@@ -7424,6 +7441,35 @@ class MoveValueInst
   void setAllowsDiagnostics(bool newValue) { allowDiagnostics = newValue; }
 };
 
+/// Equivalent to a copy_addr to [init] except that it is used for diagnostics
+/// and should not be pattern matched. During the diagnostic passes, the "move
+/// function" checker for addresses always converts this to a copy_addr [init]
+/// (if we emitted a diagnostic and proved we could not emit a move here) or a
+/// copy_addr [take][init] if we can. So this should never occur in canonical
+/// SIL.
+class MarkUnresolvedMoveAddrInst
+    : public InstructionBase<SILInstructionKind::MarkUnresolvedMoveAddrInst,
+                             NonValueInstruction>,
+      public CopyLikeInstruction {
+  friend class SILBuilder;
+
+  FixedOperandList<2> Operands;
+
+  MarkUnresolvedMoveAddrInst(SILDebugLocation DebugLoc, SILValue srcAddr,
+                             SILValue takeAddr)
+      : InstructionBase(DebugLoc), Operands(this, srcAddr, takeAddr) {}
+
+public:
+  SILValue getSrc() const { return Operands[Src].get(); }
+  SILValue getDest() const { return Operands[Dest].get(); }
+
+  void setSrc(SILValue V) { Operands[Src].set(V); }
+  void setDest(SILValue V) { Operands[Dest].set(V); }
+
+  ArrayRef<Operand> getAllOperands() const { return Operands.asArray(); }
+  MutableArrayRef<Operand> getAllOperands() { return Operands.asArray(); }
+};
+
 /// Given an object reference, return true iff it is non-nil and refers
 /// to a native swift object with strong reference count of 1.
 class IsUniqueInst

include/swift/SIL/SILNodes.def

@@ -591,8 +591,15 @@ ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction)
                     SingleValueInstruction, None, MayRelease)
   // A move_value is an OSSA only instruction. Its result does not have any side
   // effects relative to other OSSA values like copy_value.
-  SINGLE_VALUE_INST(MoveValueInst, move_value,
-                    SingleValueInstruction, None, DoesNotRelease)
+  SINGLE_VALUE_INST(MoveValueInst, move_value, SingleValueInstruction, None,
+                    DoesNotRelease)
+  
+  // A move_addr is a Raw SIL only instruction that is equivalent to a copy_addr
+  // [init]. It is lowered during the diagnostic passes to a copy_addr [init] if
+  // the move checker found uses that prevented us from converting this to a
+  // move or if we do not find such uses, a copy_addr [init] [take].
+  NON_VALUE_INST(MarkUnresolvedMoveAddrInst, mark_unresolved_move_addr,
+                 SILInstruction, None, DoesNotRelease)
 
   // IsUnique does not actually write to memory but should be modeled
   // as such. Its operand is a pointer to an object reference. The

include/swift/SILOptimizer/PassManager/Passes.def

@@ -427,6 +427,9 @@ PASS(MoveKillsCopyableValuesChecker, "sil-move-kills-copyable-values-checker",
      "to _move do not have any uses later than the _move")
 PASS(LexicalLifetimeEliminator, "sil-lexical-lifetime-eliminator",
      "Pass that removes lexical lifetime markers from borrows and alloc stack")
+PASS(MoveKillsCopyableAddressesChecker, "sil-move-kills-copyable-addresses-checker",
+     "Pass that checks that any copyable (non-move only) address that is passed "
+     "to _move do not have any uses later than the _move")
 PASS(PruneVTables, "prune-vtables",
      "Mark class methods that do not require vtable dispatch")
 PASS_RANGE(AllPasses, AADumper, PruneVTables)

lib/IRGen/IRGenSIL.cpp

@@ -1262,6 +1262,9 @@ class IRGenSILFunction :
   void visitDeallocPartialRefInst(DeallocPartialRefInst *i);
 
   void visitCopyAddrInst(CopyAddrInst *i);
+  void visitMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *mai) {
+    llvm_unreachable("Valid only when ownership is enabled");
+  }
   void visitDestroyAddrInst(DestroyAddrInst *i);
 
   void visitBindMemoryInst(BindMemoryInst *i);

lib/SIL/IR/OperandOwnership.cpp

@@ -143,6 +143,7 @@ OPERAND_OWNERSHIP(TrivialUse, CheckedCastAddrBranch)
 OPERAND_OWNERSHIP(TrivialUse, CondBranch)
 OPERAND_OWNERSHIP(TrivialUse, CondFail)
 OPERAND_OWNERSHIP(TrivialUse, CopyAddr)
+OPERAND_OWNERSHIP(TrivialUse, MarkUnresolvedMoveAddr)
 OPERAND_OWNERSHIP(TrivialUse, DeallocStack)
 OPERAND_OWNERSHIP(TrivialUse, DeinitExistentialAddr)
 OPERAND_OWNERSHIP(TrivialUse, DestroyAddr)

lib/SIL/IR/SILInstruction.cpp

@@ -1167,6 +1167,10 @@ bool SILInstruction::mayRelease() const {
     return CopyAddr->isInitializationOfDest() ==
            IsInitialization_t::IsNotInitialization;
   }
+  // mark_unresolved_move_addr is equivalent to a copy_addr [init], so a release
+  // does not occur.
+  case SILInstructionKind::MarkUnresolvedMoveAddrInst:
+    return false;
 
   case SILInstructionKind::BuiltinInst: {
     auto *BI = cast<BuiltinInst>(this);

lib/SIL/IR/SILPrinter.cpp

@@ -1703,6 +1703,11 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
     *this << getIDAndType(CI->getDest());
   }
 
+  void visitMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *CI) {
+    *this << Ctx.getID(CI->getSrc()) << " to ";
+    *this << getIDAndType(CI->getDest());
+  }
+
   void visitBindMemoryInst(BindMemoryInst *BI) {
     *this << getIDAndType(BI->getBase()) << ", ";
     *this << getIDAndType(BI->getIndex()) << " to ";

lib/SIL/Parser/ParseSIL.cpp

@@ -4771,6 +4771,34 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
                                    IsInitialization_t(IsInit));
       break;
     }
+    case SILInstructionKind::MarkUnresolvedMoveAddrInst: {
+      UnresolvedValueName SrcLName;
+      SILValue DestLVal;
+      SourceLoc ToLoc, DestLoc;
+      Identifier ToToken;
+      if (parseValueName(SrcLName) ||
+          parseSILIdentifier(ToToken, ToLoc, diag::expected_tok_in_sil_instr,
+                             "to") ||
+          parseTypedValueRef(DestLVal, DestLoc, B) ||
+          parseSILDebugLocation(InstLoc, B))
+        return true;
+
+      if (ToToken.str() != "to") {
+        P.diagnose(ToLoc, diag::expected_tok_in_sil_instr, "to");
+        return true;
+      }
+
+      if (!DestLVal->getType().isAddress()) {
+        P.diagnose(DestLoc, diag::sil_invalid_instr_operands);
+        return true;
+      }
+
+      SILValue SrcLVal =
+          getLocalValue(SrcLName, DestLVal->getType(), InstLoc, B);
+      ResultVal = B.createMarkUnresolvedMoveAddr(InstLoc, SrcLVal, DestLVal);
+      break;
+    }
+
     case SILInstructionKind::BindMemoryInst: {
       SILValue IndexVal;
       SILType EltTy;

lib/SIL/Utils/InstructionUtils.cpp

@@ -631,6 +631,11 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
       return RuntimeEffect::MetaData | RuntimeEffect::RefCounting;
     return RuntimeEffect::MetaData;
   }
+  // Equialent to a copy_addr [init]
+  case SILInstructionKind::MarkUnresolvedMoveAddrInst: {
+    return RuntimeEffect::MetaData | RuntimeEffect::RefCounting;
+  }
+
   case SILInstructionKind::StoreInst:
     switch (cast<StoreInst>(inst)->getOwnershipQualifier()) {
       case StoreOwnershipQualifier::Unqualified:

lib/SIL/Utils/MemAccessUtils.cpp

@@ -1904,6 +1904,9 @@ bool swift::memInstMustInitialize(Operand *memOper) {
     auto *CAI = cast<CopyAddrInst>(memInst);
     return CAI->getDest() == address && CAI->isInitializationOfDest();
   }
+  case SILInstructionKind::MarkUnresolvedMoveAddrInst: {
+    return cast<MarkUnresolvedMoveAddrInst>(memInst)->getDest() == address;
+  }
   case SILInstructionKind::InitExistentialAddrInst:
   case SILInstructionKind::InitEnumDataAddrInst:
   case SILInstructionKind::InjectEnumAddrInst:
@@ -2346,6 +2349,11 @@ void swift::visitAccessedAddress(SILInstruction *I,
     visitor(&I->getAllOperands()[CopyAddrInst::Dest]);
     return;
 
+  case SILInstructionKind::MarkUnresolvedMoveAddrInst:
+    visitor(&I->getAllOperands()[MarkUnresolvedMoveAddrInst::Src]);
+    visitor(&I->getAllOperands()[MarkUnresolvedMoveAddrInst::Dest]);
+    return;
+
 #define NEVER_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \
   case SILInstructionKind::Store##Name##Inst:
 #include "swift/AST/ReferenceStorage.def"

lib/SIL/Utils/MemoryLocations.cpp

@@ -365,6 +365,11 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
       case SILInstructionKind::SwitchEnumAddrInst:
       case SILInstructionKind::WitnessMethodInst:
         break;
+      case SILInstructionKind::MarkUnresolvedMoveAddrInst:
+        // We do not want the memory lifetime verifier to verify move_addr inst
+        // since the MarkUnresolvedMoveAddrChecker will validate that its uses
+        // are correct.
+        return false;
       default:
         return false;
     }

lib/SIL/Utils/OwnershipUtils.cpp

@@ -701,17 +701,18 @@ swift::findTransitiveUsesForAddress(SILValue projectedAddress,
     }
     // First, eliminate "end point uses" that we just need to check liveness at
     // and do not need to check transitive uses of.
-    if (isa<LoadInst>(user) || isa<CopyAddrInst>(user) || isIncidentalUse(user)
-        || isa<StoreInst>(user) || isa<DestroyAddrInst>(user)
-        || isa<AssignInst>(user) || isa<YieldInst>(user)
-        || isa<LoadUnownedInst>(user) || isa<StoreUnownedInst>(user)
-        || isa<EndApplyInst>(user) || isa<LoadWeakInst>(user)
-        || isa<StoreWeakInst>(user) || isa<AssignByWrapperInst>(user)
-        || isa<BeginUnpairedAccessInst>(user)
-        || isa<EndUnpairedAccessInst>(user) || isa<WitnessMethodInst>(user)
-        || isa<SwitchEnumAddrInst>(user) || isa<CheckedCastAddrBranchInst>(user)
-        || isa<SelectEnumAddrInst>(user) || isa<InjectEnumAddrInst>(user)
-        || isa<IsUniqueInst>(user)) {
+    if (isa<LoadInst>(user) || isa<CopyAddrInst>(user) ||
+        isa<MarkUnresolvedMoveAddrInst>(user) || isIncidentalUse(user) ||
+        isa<StoreInst>(user) || isa<DestroyAddrInst>(user) ||
+        isa<AssignInst>(user) || isa<YieldInst>(user) ||
+        isa<LoadUnownedInst>(user) || isa<StoreUnownedInst>(user) ||
+        isa<EndApplyInst>(user) || isa<LoadWeakInst>(user) ||
+        isa<StoreWeakInst>(user) || isa<AssignByWrapperInst>(user) ||
+        isa<BeginUnpairedAccessInst>(user) ||
+        isa<EndUnpairedAccessInst>(user) || isa<WitnessMethodInst>(user) ||
+        isa<SwitchEnumAddrInst>(user) || isa<CheckedCastAddrBranchInst>(user) ||
+        isa<SelectEnumAddrInst>(user) || isa<InjectEnumAddrInst>(user) ||
+        isa<IsUniqueInst>(user)) {
       leafUse(op);
       continue;
     }

lib/SIL/Verifier/LoadBorrowImmutabilityChecker.cpp

@@ -170,6 +170,17 @@ bool GatherWritesVisitor::visitUse(Operand *op, AccessUseType useTy) {
     }
     return true;
 
+  case SILInstructionKind::MarkUnresolvedMoveAddrInst:
+    if (cast<MarkUnresolvedMoveAddrInst>(user)->getDest() == op->get()) {
+      writeAccumulator.push_back(op);
+      return true;
+    }
+
+    // This operand is the move source, we just return true. This is because
+    // mark_unresolved_move_addr semantically is treated as a copy_addr of the
+    // source. The checker determines if we can convert it to a move.
+    return true;
+
   // If this value is dependent on another, conservatively consider it a write.
   //
   // FIXME: explain why a mark_dependence effectively writes to storage.

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -354,6 +354,14 @@ void MemoryLifetimeVerifier::initDataflowInBlock(SILBasicBlock *block,
         genBits(state, CAI->getDest());
         break;
       }
+      case SILInstructionKind::MarkUnresolvedMoveAddrInst: {
+        auto *MMAI = cast<MarkUnresolvedMoveAddrInst>(&I);
+        // We do not treat the move addr inst as invalidating its src since we
+        // are going to prove that we do not inappropriately reuse the memory
+        // later.
+        genBits(state, MMAI->getDest());
+        break;
+      }
       case SILInstructionKind::InjectEnumAddrInst: {
         auto *IEAI = cast<InjectEnumAddrInst>(&I);
         int enumIdx = locations.getLocationIdx(IEAI->getOperand());

lib/SIL/Verifier/SILVerifier.cpp

@@ -592,6 +592,11 @@ struct ImmutableAddressUseVerifier {
         break;
       case SILInstructionKind::EndAccessInst:
         break;
+      case SILInstructionKind::MarkUnresolvedMoveAddrInst:
+        // We model mark_unresolved_move_addr as a copy_addr [init]. So no
+        // mutation can happen. The checker will prove eventually that we can
+        // convert it to a copy_addr [take] [init].
+        break;
       case SILInstructionKind::CopyAddrInst:
         if (isConsumingOrMutatingCopyAddrUse(use))
           return true;
@@ -2637,6 +2642,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
             "cannot directly copy type with inaccessible ABI");
   }
 
+  void checkMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *SI) {
+    require(F.hasOwnership(), "Only valid in OSSA.");
+    require(F.getModule().getStage() == SILStage::Raw, "Only valid in Raw SIL");
+    require(SI->getSrc()->getType().isAddress(), "Src value should be lvalue");
+    require(SI->getDest()->getType().isAddress(),
+            "Dest address should be lvalue");
+    requireSameType(SI->getDest()->getType(), SI->getSrc()->getType(),
+                    "Store operand type and dest type mismatch");
+    require(F.isTypeABIAccessible(SI->getDest()->getType()),
+            "cannot directly copy type with inaccessible ABI");
+  }
+
   void checkRetainValueInst(RetainValueInst *I) {
     require(I->getOperand()->getType().isObject(),
             "Source value should be an object value");

lib/SILGen/SILGenLazyConformance.cpp

@@ -195,6 +195,11 @@ class LazyConformanceEmitter : public SILInstructionVisitor<LazyConformanceEmitt
     SGM.useConformancesFromType(CAI->getDest()->getType().getASTType());
   }
 
+  void visitMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *MAI) {
+    SGM.useConformancesFromType(MAI->getSrc()->getType().getASTType());
+    SGM.useConformancesFromType(MAI->getDest()->getType().getASTType());
+  }
+
   void visitCopyValueInst(CopyValueInst *CVI) {
     SGM.useConformancesFromType(CVI->getOperand()->getType().getASTType());
   }