Merge pull request #59184 from gottesmm/release/5.7-movedbginfo

[release/5.7][move-function] Remaining _move changes for 5.7

Author: gottesmm

include/swift/SIL/DebugUtils.h

@@ -237,24 +237,26 @@ bool hasNonTrivialNonDebugTransitiveUsers(
 /// operators to access functionality from the underlying instruction when
 /// needed.
 struct DebugVarCarryingInst {
-  enum class Kind {
+  enum class Kind : uint8_t {
     Invalid = 0,
     DebugValue,
     AllocStack,
     AllocBox,
   };
 
-  Kind kind;
   SILInstruction *inst;
+  Kind kind;
+  uintptr_t spareBits : (sizeof(uintptr_t) - sizeof(Kind)) * 8;
 
-  DebugVarCarryingInst() : kind(Kind::Invalid), inst(nullptr) {}
+  DebugVarCarryingInst() : inst(nullptr), kind(Kind::Invalid), spareBits(0) {}
   DebugVarCarryingInst(DebugValueInst *dvi)
-      : kind(Kind::DebugValue), inst(dvi) {}
+      : inst(dvi), kind(Kind::DebugValue), spareBits(0) {}
   DebugVarCarryingInst(AllocStackInst *asi)
-      : kind(Kind::AllocStack), inst(asi) {}
-  DebugVarCarryingInst(AllocBoxInst *abi) : kind(Kind::AllocBox), inst(abi) {}
+      : inst(asi), kind(Kind::AllocStack), spareBits(0) {}
+  DebugVarCarryingInst(AllocBoxInst *abi)
+      : inst(abi), kind(Kind::AllocBox), spareBits(0) {}
   DebugVarCarryingInst(SILInstruction *newInst)
-      : kind(Kind::Invalid), inst(nullptr) {
+      : inst(nullptr), kind(Kind::Invalid), spareBits(0) {
     switch (newInst->getKind()) {
     default:
       return;
@@ -280,6 +282,15 @@ struct DebugVarCarryingInst {
   /// '->'. This keeps the wrapper light weight.
   SILInstruction *operator->() const { return inst; }
 
+  bool operator==(const DebugVarCarryingInst &other) const {
+    return kind == other.kind && inst == other.inst &&
+           spareBits == other.spareBits;
+  }
+
+  bool operator!=(const DebugVarCarryingInst &other) const {
+    return !(*this == other);
+  }
+
   /// Add support for this struct in `if` statement.
   explicit operator bool() const { return bool(kind); }
 
@@ -351,7 +362,8 @@ struct DebugVarCarryingInst {
     case Kind::AllocStack:
       return cast<AllocStackInst>(inst)->getWasMoved();
     case Kind::AllocBox:
-      llvm_unreachable("Not implemented");
+      // We do not support moving alloc box today, so we always return false.
+      return false;
     }
   }
 

include/swift/SILOptimizer/PassManager/Passes.def

@@ -440,6 +440,8 @@ PASS(MoveFunctionCanonicalization, "sil-move-function-canon",
      "function checking.")
 PASS(DebugInfoCanonicalizer, "sil-onone-debuginfo-canonicalizer",
      "Canonicalize debug info at -Onone by propagating debug info into coroutine funclets")
+PASS(MovedAsyncVarDebugInfoPropagator, "sil-moved-async-var-dbginfo-propagator",
+     "Propagate debug info from moved async vars after coroutine funclet boundaries")
 PASS(PruneVTables, "prune-vtables",
      "Mark class methods that do not require vtable dispatch")
 PASS_RANGE(AllPasses, AADumper, PruneVTables)

lib/SILOptimizer/Mandatory/CMakeLists.txt

@@ -21,6 +21,7 @@ target_sources(swiftSILOptimizer PRIVATE
   LexicalLifetimeEliminator.cpp
   LowerHopToActor.cpp
   MandatoryInlining.cpp
+  MovedAsyncVarDebugInfoPropagator.cpp
   MoveFunctionCanonicalization.cpp
   MoveKillsCopyableAddressesChecker.cpp
   MoveKillsCopyableValuesChecker.cpp

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

@@ -1363,6 +1363,10 @@ bool GatherLexicalLifetimeUseVisitor::visitUse(Operand *op,
   if (isa<DeallocStackInst>(op->getUser()))
     return true;
 
+  // Ignore end_access.
+  if (isa<EndAccessInst>(op->getUser()))
+    return true;
+
   LLVM_DEBUG(llvm::dbgs() << "Found liveness use: " << *op->getUser());
   useState.livenessUses.insert(op->getUser());
 
@@ -1507,6 +1511,24 @@ bool DataflowState::cleanupAllDestroyAddr(
       SILBuilderWithScope builder(iter);
       auto *dvi = builder.createDestroyAddr(
           RegularLocation::getAutoGeneratedLocation(), address);
+      // Create a debug_value undef if we have debug info to stop the async dbg
+      // info propagation from creating debug info for an already destroyed
+      // value. We use a separate builder since we need to control the debug
+      // scope/location to get llvm to do the right thing.
+      if (addressDebugInst) {
+        if (auto varInfo = addressDebugInst.getVarInfo()) {
+          // We need to always insert /after/ the reinit since the value will
+          // not be defined before the value.
+          SILBuilderWithScope dbgValueInsertBuilder(dvi);
+          dbgValueInsertBuilder.setCurrentDebugScope(
+              addressDebugInst->getDebugScope());
+          dbgValueInsertBuilder.createDebugValue(
+              addressDebugInst.inst->getLoc(),
+              SILUndef::get(address->getType(), dvi->getModule()), *varInfo,
+              false,
+              /*was moved*/ true);
+        }
+      }
       useState.destroys.insert(dvi);
       continue;
     }

lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp

@@ -133,10 +133,22 @@ bool CheckerLivenessInfo::compute() {
         break;
       case OperandOwnership::Borrow: {
         if (auto *bbi = dyn_cast<BeginBorrowInst>(user)) {
-          // Only add borrows to liveness if the borrow isn't lexical. If it is
-          // a lexical borrow, we have created an entirely new source level
-          // binding that should be tracked separately.
-          if (!bbi->isLexical()) {
+          // If we have a lexical begin_borrow, we are going to check its uses
+          // separately and emit diagnostics for it. So we just need to add the
+          // liveness of the begin_borrow.
+          //
+          // NOTE: We know that semantically the use lexical lifetime must have
+          // a separate lifetime from the base lexical lifetime that we are
+          // processing. We do not want to include those uses as transitive uses
+          // of our base lexical lifetime. We just want to treat the formation
+          // of the new variable as a use. Thus we only include the begin_borrow
+          // itself as the use.
+          if (bbi->isLexical()) {
+            liveness.updateForUse(bbi, false /*lifetime ending*/);
+          } else {
+            // Otherwise, try to update liveness for a borrowing operand
+            // use. This will make it so that we add the end_borrows of the
+            // liveness use. If we have a reborrow here, we will bail.
             bool failed = !liveness.updateForBorrowingOperand(use);
             if (failed)
               return false;
@@ -351,24 +363,32 @@ bool MoveKillsCopyableValuesChecker::check() {
   SmallSetVector<SILValue, 32> valuesToCheck;
 
   for (auto *arg : fn->getEntryBlock()->getSILFunctionArguments()) {
-    if (arg->getOwnershipKind() == OwnershipKind::Owned)
+    if (arg->getOwnershipKind() == OwnershipKind::Owned) {
+      LLVM_DEBUG(llvm::dbgs() << "Found owned arg to check: " << *arg);
       valuesToCheck.insert(arg);
+    }
   }
 
   for (auto &block : *fn) {
     for (auto &ii : block) {
       if (auto *bbi = dyn_cast<BeginBorrowInst>(&ii)) {
-        if (bbi->isLexical())
+        if (bbi->isLexical()) {
+          LLVM_DEBUG(llvm::dbgs()
+                     << "Found lexical lifetime to check: " << *bbi);
           valuesToCheck.insert(bbi);
+        }
         continue;
       }
     }
   }
 
-  if (valuesToCheck.empty())
+  if (valuesToCheck.empty()) {
+    LLVM_DEBUG(llvm::dbgs() << "No values to check! Exiting early!\n");
     return false;
+  }
 
-  LLVM_DEBUG(llvm::dbgs() << "Visiting Function: " << fn->getName() << "\n");
+  LLVM_DEBUG(llvm::dbgs()
+             << "Found at least one value to check, performing checking.\n");
   auto valuesToProcess =
       llvm::makeArrayRef(valuesToCheck.begin(), valuesToCheck.end());
   auto &mod = fn->getModule();
@@ -475,6 +495,9 @@ class MoveKillsCopyableValuesCheckerPass : public SILFunctionTransform {
     assert(fn->getModule().getStage() == SILStage::Raw &&
            "Should only run on Raw SIL");
 
+    LLVM_DEBUG(llvm::dbgs() << "*** Checking moved values in fn: "
+                            << getFunction()->getName() << '\n');
+
     MoveKillsCopyableValuesChecker checker(getFunction());
 
     // If we already had dominance or loop info generated, update them when