[move-function] Add a new instruction called mark_unresolved_move_addr.

gottesmm · gottesmm · commit d74299e68d44 · 2021-12-06T12:47:29.000-08:00
This instruction is similar to a copy_addr except that it marks a move of an
address that has to be checked. In order to keep the memory lifetime verifier
happy, the semantics before the checker runs are the mark_unresolved_move_addr is
equivalent to copy_addr [init] (not copy_addr [take][init]).

The use of this instruction is that Mandatory Inlining converts builtin "move"
to a mark_unresolved_move_addr when inlining the function "_move" (the only
place said builtin is invoked).

This is then run through a special checker (that is later in this PR) that
either proves that the mark_unresolved_move_addr can actually be a move in which
case it converts it to copy_addr [take][init] or if it can not be a move, emit
an error and convert the instruction to a copy_addr [init]. After this is done
for all instructions, we loop back through again and emit an error on any
mark_unresolved_move_addr that were not processed earlier allowing for us to
know that we have completeness.

NOTE: The move kills checker for addresses is going to run after Mandatory
Inlining, but before predictable memory opts and friends.
diff --git a/include/swift/SIL/SILBuilder.h b/include/swift/SIL/SILBuilder.h
@@ -1271,6 +1271,12 @@ 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,
diff --git a/include/swift/SIL/SILCloner.h b/include/swift/SIL/SILCloner.h
@@ -1341,6 +1341,15 @@ 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) {
diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h
@@ -7424,6 +7424,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
diff --git a/include/swift/SIL/SILNodes.def b/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
diff --git a/lib/IRGen/IRGenSIL.cpp b/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);
diff --git a/lib/SIL/IR/OperandOwnership.cpp b/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)
diff --git a/lib/SIL/IR/SILInstruction.cpp b/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);
diff --git a/lib/SIL/IR/SILPrinter.cpp b/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 ";
diff --git a/lib/SIL/Parser/ParseSIL.cpp b/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;
diff --git a/lib/SIL/Utils/InstructionUtils.cpp b/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:
diff --git a/lib/SIL/Utils/MemAccessUtils.cpp b/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"
diff --git a/lib/SIL/Utils/MemoryLocations.cpp b/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;
     }
diff --git a/lib/SIL/Utils/OwnershipUtils.cpp b/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;
     }
diff --git a/lib/SIL/Verifier/LoadBorrowImmutabilityChecker.cpp b/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.
diff --git a/lib/SIL/Verifier/MemoryLifetimeVerifier.cpp b/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());
diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/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");
diff --git a/lib/SILGen/SILGenLazyConformance.cpp b/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());
   }
diff --git a/lib/SILOptimizer/Analysis/MemoryBehavior.cpp b/lib/SILOptimizer/Analysis/MemoryBehavior.cpp
@@ -171,6 +171,7 @@ class MemoryBehaviorVisitor
   MemBehavior visitLoadInst(LoadInst *LI);
   MemBehavior visitStoreInst(StoreInst *SI);
   MemBehavior visitCopyAddrInst(CopyAddrInst *CAI);
+  MemBehavior visitMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *MAI);
   MemBehavior visitApplyInst(ApplyInst *AI);
   MemBehavior visitTryApplyInst(TryApplyInst *AI);
   MemBehavior visitBeginApplyInst(BeginApplyInst *AI);
@@ -301,6 +302,25 @@ MemBehavior MemoryBehaviorVisitor::visitCopyAddrInst(CopyAddrInst *CAI) {
   return MemBehavior::None;
 }
 
+MemBehavior
+MemoryBehaviorVisitor::visitMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *MAI) {
+  bool mayWrite = mayAlias(MAI->getDest());
+  bool mayRead = mayAlias(MAI->getSrc());
+
+  if (mayRead) {
+    if (mayWrite)
+      return MemBehavior::MayReadWrite;
+
+    // mark_unresolved_move_addr doesn't semantically perform a take of src.
+    return MemBehavior::MayRead;
+  }
+
+  if (mayWrite)
+    return MemBehavior::MayWrite;
+
+  return MemBehavior::None;
+}
+
 MemBehavior MemoryBehaviorVisitor::visitBuiltinInst(BuiltinInst *BI) {
   // If our callee is not a builtin, be conservative and return may have side
   // effects.
@@ -368,6 +388,7 @@ static bool hasEscapingUses(SILValue address, int &numChecks) {
       case SILInstructionKind::LoadInst:
       case SILInstructionKind::StoreInst:
       case SILInstructionKind::CopyAddrInst:
+      case SILInstructionKind::MarkUnresolvedMoveAddrInst:
       case SILInstructionKind::DestroyAddrInst:
       case SILInstructionKind::DeallocStackInst:
       case SILInstructionKind::EndAccessInst:
diff --git a/lib/SILOptimizer/Mandatory/CapturePromotion.cpp b/lib/SILOptimizer/Mandatory/CapturePromotion.cpp
@@ -1108,7 +1108,14 @@ class NonEscapingUserVisitor
 #undef RECURSIVE_INST_VISITOR
 
   bool visitCopyAddrInst(CopyAddrInst *cai) {
-    if (currentOp.get()->getOperandNumber() == 1 || cai->isTakeOfSrc())
+    if (currentOp.get()->getOperandNumber() == CopyAddrInst::Dest ||
+        cai->isTakeOfSrc())
+      markCurrentOpAsMutation();
+    return true;
+  }
+
+  bool visitMarkUnresolvedMoveAddrInst(MarkUnresolvedMoveAddrInst *mai) {
+    if (currentOp.get()->getOperandNumber() == MarkUnresolvedMoveAddrInst::Dest)
       markCurrentOpAsMutation();
     return true;
   }
diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp b/lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp
diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
diff --git a/lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp b/lib/SILOptimizer/UtilityPasses/SerializeSILPass.cpp
diff --git a/lib/SILOptimizer/Utils/SILInliner.cpp b/lib/SILOptimizer/Utils/SILInliner.cpp
diff --git a/lib/Serialization/DeserializeSIL.cpp b/lib/Serialization/DeserializeSIL.cpp
diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h
diff --git a/lib/Serialization/SerializeSIL.cpp b/lib/Serialization/SerializeSIL.cpp
