[move-function] Teach debug info how to handle reinits properly of vars.

gottesmm · gottesmm · commit 65e20e7b2df2 · 2022-02-18T12:59:56.000-08:00
This fixes the issues I was in my earlier PR about seeing with vars not getting
appropriate debug info when moved. Now we get the right debug info and in the
dwarf can see the two live ranges of our moved value as we hoped for.

NOTE: I disabled alloc stack hoisting on move_function_debuginfo.swift since it
can result in us eliminating debug info at -Onone when we merge alloc_stack. I
am preparing a separate patch that fixes that issue but hit a lldb problem. This
unblocks this PR from that change since they are not /actually/ related.
diff --git a/lib/IRGen/IRGenDebugInfo.cpp b/lib/IRGen/IRGenDebugInfo.cpp
@@ -219,6 +219,7 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
   bool buildDebugInfoExpression(const SILDebugVariable &VarInfo,
                                 SmallVectorImpl<uint64_t> &Operands);
 
+  /// Emit a dbg.declare at the current insertion point in Builder.
   void emitVariableDeclaration(IRBuilder &Builder,
                                ArrayRef<llvm::Value *> Storage,
                                DebugTypeInfo Ty, const SILDebugScope *DS,
@@ -227,6 +228,7 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
                                IndirectionKind = DirectValue,
                                ArtificialKind = RealValue,
                                AddrDbgInstrKind = AddrDbgInstrKind::DbgDeclare);
+
   void emitDbgIntrinsic(IRBuilder &Builder, llvm::Value *Storage,
                         llvm::DILocalVariable *Var, llvm::DIExpression *Expr,
                         unsigned Line, unsigned Col, llvm::DILocalScope *Scope,
@@ -2646,7 +2648,6 @@ void IRGenDebugInfoImpl::emitDbgIntrinsic(
   auto *InlinedAt = createInlinedAt(DS);
   auto DL =
       llvm::DILocation::get(IGM.getLLVMContext(), Line, Col, Scope, InlinedAt);
-  auto *BB = Builder.GetInsertBlock();
 
   // An alloca may only be described by exactly one dbg.declare.
   if (isa<llvm::AllocaInst>(Storage) &&
@@ -2695,17 +2696,25 @@ void IRGenDebugInfoImpl::emitDbgIntrinsic(
   };
   DbgInserter inserter{DBuilder, AddrDInstKind};
 
-  // If we have a single alloca, just insert the debug in
+  // If we have a single alloca...
   if (auto *Alloca = dyn_cast<llvm::AllocaInst>(Storage)) {
-    auto *ParentBB = Alloca->getParent();
-    auto InsertBefore = std::next(Alloca->getIterator());
-    if (InsertBefore != ParentBB->end())
+    auto *ParentBB = Builder.GetInsertBlock();
+    auto InsertBefore = Builder.GetInsertPoint();
+
+    if (AddrDInstKind == AddrDbgInstrKind::DbgDeclare) {
+      ParentBB = Alloca->getParent();
+      InsertBefore = std::next(Alloca->getIterator());
+    }
+
+    if (InsertBefore != ParentBB->end()) {
       inserter.insert(Alloca, Var, Expr, DL, &*InsertBefore);
-    else
+    } else {
       inserter.insert(Alloca, Var, Expr, DL, ParentBB);
+    }
     return;
   }
 
+  auto *BB = Builder.GetInsertBlock();
   if ((isa<llvm::IntrinsicInst>(Storage) &&
        cast<llvm::IntrinsicInst>(Storage)->getIntrinsicID() ==
            llvm::Intrinsic::coro_alloca_get)) {
diff --git a/lib/IRGen/IRGenSIL.cpp b/lib/IRGen/IRGenSIL.cpp
@@ -893,19 +893,44 @@ class IRGenSILFunction :
   /// Unconditionally emit a stack shadow copy of an \c llvm::Value.
   llvm::Value *emitShadowCopy(llvm::Value *Storage, const SILDebugScope *Scope,
                               SILDebugVariable VarInfo,
-                              llvm::Optional<Alignment> _Align, bool Init) {
+                              llvm::Optional<Alignment> _Align, bool Init,
+                              bool WasMoved) {
     auto Align = _Align.getValueOr(IGM.getPointerAlignment());
     unsigned ArgNo = VarInfo.ArgNo;
     auto &Alloca = ShadowStackSlots[{ArgNo, {Scope, VarInfo.Name}}];
+
     if (!Alloca.isValid())
       Alloca = createAlloca(Storage->getType(), Align, VarInfo.Name + ".debug");
-    assert(canAllocaStoreValue(Alloca.getAddress(), Storage, VarInfo, Scope) &&
+
+    // If our value was ever moved, we may be reinitializing the shadow
+    // copy. Insert the bit cast so that the types line up and we do not get the
+    // duplicate shadow copy error (which triggers based off of type
+    // differences).
+    auto *Address = Alloca.getAddress();
+    if (WasMoved) {
+      auto nonPtrAllocaType =
+          cast<llvm::PointerType>(Address->getType())->getElementType();
+      if (nonPtrAllocaType != Storage->getType())
+        Address = Builder.CreateBitOrPointerCast(
+            Address, llvm::PointerType::get(Storage->getType(), 0));
+    }
+
+    assert(canAllocaStoreValue(Address, Storage, VarInfo, Scope) &&
            "bad scope?");
+
     if (Init) {
+      // Zero init our bare allocation.
       zeroInit(cast<llvm::AllocaInst>(Alloca.getAddress()));
       ArtificialLocation AutoRestore(Scope, IGM.DebugInfo.get(), Builder);
-      Builder.CreateStore(Storage, Alloca.getAddress(), Align);
+      // But store into the address with maybe bitcast.
+      Builder.CreateStore(Storage, Address, Align);
     }
+
+    // If this allocation was moved at some point, we might be reinitializing a
+    // shadow copy. In such a case, lets insert an identity bit cast so that our
+    // callers will use this address with respect to the place where we
+    // reinit. Otherwise, callers may use the alloca's insert point. The
+    // optimizer will eliminate these later without issue.
     return Alloca.getAddress();
   }
 
@@ -928,7 +953,7 @@ class IRGenSILFunction :
   llvm::Value *emitShadowCopyIfNeeded(llvm::Value *Storage,
                                       const SILDebugScope *Scope,
                                       SILDebugVariable VarInfo,
-                                      bool IsAnonymous,
+                                      bool IsAnonymous, bool WasMoved,
                                       llvm::Optional<Alignment> Align = None) {
     // Never emit shadow copies when optimizing, or if already on the stack.  No
     // debug info is emitted for refcounts either
@@ -950,7 +975,8 @@ class IRGenSILFunction :
     }
 
     // Emit a shadow copy.
-    auto shadow = emitShadowCopy(Storage, Scope, VarInfo, Align, true);
+    auto shadow =
+        emitShadowCopy(Storage, Scope, VarInfo, Align, true, WasMoved);
 
     // Mark variables in async functions for lifetime extension, so they get
     // spilled into the async context.
@@ -969,14 +995,16 @@ class IRGenSILFunction :
   llvm::Value *emitShadowCopyIfNeeded(Address Storage,
                                       const SILDebugScope *Scope,
                                       SILDebugVariable VarInfo,
-                                      bool IsAnonymous) {
+                                      bool IsAnonymous, bool WasMoved) {
     return emitShadowCopyIfNeeded(Storage.getAddress(), Scope, VarInfo,
-                                  IsAnonymous, Storage.getAlignment());
+                                  IsAnonymous, WasMoved,
+                                  Storage.getAlignment());
   }
 
   /// Like \c emitShadowCopyIfNeeded() but takes an exploded value.
   void emitShadowCopyIfNeeded(SILValue &SILVal, const SILDebugScope *Scope,
                               SILDebugVariable VarInfo, bool IsAnonymous,
+                              bool WasMoved,
                               llvm::SmallVectorImpl<llvm::Value *> &copy) {
     Explosion e = getLoweredExplosion(SILVal);
 
@@ -1003,8 +1031,8 @@ class IRGenSILFunction :
       return;
     
     if (e.size() == 1) {
-      copy.push_back(
-        emitShadowCopyIfNeeded(e.claimNext(), Scope, VarInfo, IsAnonymous));
+      copy.push_back(emitShadowCopyIfNeeded(e.claimNext(), Scope, VarInfo,
+                                            IsAnonymous, WasMoved));
       return;
     }
 
@@ -1053,12 +1081,11 @@ class IRGenSILFunction :
   
   /// Emit debug info for a function argument or a local variable.
   template <typename StorageType>
-  void emitDebugVariableDeclaration(StorageType Storage, DebugTypeInfo Ty,
-                                    SILType SILTy, const SILDebugScope *DS,
-                                    SILLocation VarLoc,
-                                    SILDebugVariable VarInfo,
-                                    IndirectionKind Indirection,
-                                    AddrDbgInstrKind DbgInstrKind = AddrDbgInstrKind::DbgDeclare) {
+  void emitDebugVariableDeclaration(
+      StorageType Storage, DebugTypeInfo Ty, SILType SILTy,
+      const SILDebugScope *DS, SILLocation VarLoc, SILDebugVariable VarInfo,
+      IndirectionKind Indirection,
+      AddrDbgInstrKind DbgInstrKind = AddrDbgInstrKind::DbgDeclare) {
     // TODO: fix demangling for C++ types (SR-13223).
     if (swift::TypeBase *ty = SILTy.getASTType().getPointer()) {
       if (MetatypeType *metaTy = dyn_cast<MetatypeType>(ty))
@@ -1070,15 +1097,18 @@ class IRGenSILFunction :
     }
 
     assert(IGM.DebugInfo && "debug info not enabled");
+
     if (VarInfo.ArgNo) {
       PrologueLocation AutoRestore(IGM.DebugInfo.get(), Builder);
       IGM.DebugInfo->emitVariableDeclaration(Builder, Storage, Ty, DS, VarLoc,
                                              VarInfo, Indirection, ArtificialKind::RealValue,
                                              DbgInstrKind);
-    } else
-      IGM.DebugInfo->emitVariableDeclaration(Builder, Storage, Ty, DS, VarLoc,
-                                             VarInfo, Indirection, ArtificialKind::RealValue,
-                                             DbgInstrKind);
+      return;
+    }
+
+    IGM.DebugInfo->emitVariableDeclaration(
+        Builder, Storage, Ty, DS, VarLoc, VarInfo, Indirection,
+        ArtificialKind::RealValue, DbgInstrKind);
   }
 
   void emitFailBB() {
@@ -4787,8 +4817,9 @@ void IRGenSILFunction::emitErrorResultVar(CanSILFunctionType FnTy,
       ErrorInfo, FnTy, IGM.getMaximalTypeExpansionContext()));
   auto Var = DbgValue->getVarInfo();
   assert(Var && "error result without debug info");
-  auto Storage = emitShadowCopyIfNeeded(ErrorResultSlot.getAddress(),
-                                        getDebugScope(), *Var, false);
+  auto Storage =
+      emitShadowCopyIfNeeded(ErrorResultSlot.getAddress(), getDebugScope(),
+                             *Var, false, false /*was move*/);
   if (!IGM.DebugInfo)
     return;
   auto DbgTy = DebugTypeInfo::getErrorResult(
@@ -4853,7 +4884,8 @@ void IRGenSILFunction::emitPoisonDebugValueInst(DebugValueInst *i) {
   // vs. multi-value explosions.
   llvm::Value *shadowAddress;
   if (singleValueExplosion) {
-    shadowAddress = emitShadowCopy(storage, scope, *varInfo, ptrAlign, false);
+    shadowAddress = emitShadowCopy(storage, scope, *varInfo, ptrAlign, false,
+                                   false /*was moved*/);
   } else {
     assert(refTy->isClassExistentialType() && "unknown multi-value explosion");
     // FIXME: Handling Optional existentials requires TypeInfo
@@ -4984,12 +5016,12 @@ void IRGenSILFunction::visitDebugValueInst(DebugValueInst *i) {
   // Put the value into a shadow-copy stack slot at -Onone.
   llvm::SmallVector<llvm::Value *, 8> Copy;
   if (IsAddrVal)
-    Copy.emplace_back(
-      emitShadowCopyIfNeeded(getLoweredAddress(SILVal).getAddress(),
-                             i->getDebugScope(), *VarInfo, IsAnonymous));
+    Copy.emplace_back(emitShadowCopyIfNeeded(
+        getLoweredAddress(SILVal).getAddress(), i->getDebugScope(), *VarInfo,
+        IsAnonymous, i->getWasMoved()));
   else
     emitShadowCopyIfNeeded(SILVal, i->getDebugScope(), *VarInfo, IsAnonymous,
-                           Copy);
+                           i->getWasMoved(), Copy);
 
   bindArchetypes(DbgTy.getType());
   if (!IGM.DebugInfo)
@@ -5314,7 +5346,7 @@ void IRGenSILFunction::emitDebugInfoForAllocStack(AllocStackInst *i,
       if (!Alloca->isStaticAlloca()) {
         // Store the address of the dynamic alloca on the stack.
         addr = emitShadowCopy(addr, DS, *VarInfo, IGM.getPointerAlignment(),
-                              /*init*/ true);
+                              /*init*/ true, i->getWasMoved());
         Indirection =
             InCoroContext(*CurSILFn, *i) ? CoroIndirectValue : IndirectValue;
       }
@@ -5356,17 +5388,18 @@ void IRGenSILFunction::emitDebugInfoForAllocStack(AllocStackInst *i,
       !isa<llvm::UndefValue>(shadow)) {
     if (ValueVariables.insert(shadow).second)
       ValueDomPoints.push_back({shadow, getActiveDominancePoint()});
-    auto inst = cast<llvm::Instruction>(shadow);
-    llvm::IRBuilder<> builder(inst->getNextNode());
+    auto shadowInst = cast<llvm::Instruction>(shadow);
+    llvm::IRBuilder<> builder(shadowInst->getNextNode());
     addr =
         builder.CreateLoad(shadow->getType()->getPointerElementType(), shadow);
   }
 
   bindArchetypes(DbgTy.getType());
-  if (IGM.DebugInfo)
-    emitDebugVariableDeclaration(addr, DbgTy, SILTy, DS,
-                                 i->getLoc(), *VarInfo,
-                                 Indirection);
+  if (IGM.DebugInfo) {
+    emitDebugVariableDeclaration(addr, DbgTy, SILTy, DS, i->getLoc(), *VarInfo,
+                                 Indirection,
+                                 AddrDbgInstrKind(i->getWasMoved()));
+  }
 }
 
 void IRGenSILFunction::visitAllocStackInst(swift::AllocStackInst *i) {
@@ -5585,8 +5618,9 @@ void IRGenSILFunction::visitAllocBoxInst(swift::AllocBoxInst *i) {
   auto VarInfo = i->getVarInfo();
   assert(VarInfo && "debug_value without debug info");
 
-  auto Storage = emitShadowCopyIfNeeded(
-      boxWithAddr.getAddress(), i->getDebugScope(), *VarInfo, IsAnonymous);
+  auto Storage =
+      emitShadowCopyIfNeeded(boxWithAddr.getAddress(), i->getDebugScope(),
+                             *VarInfo, IsAnonymous, false /*was moved*/);
 
   if (!IGM.DebugInfo)
     return;
diff --git a/lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp b/lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp
@@ -1429,6 +1429,8 @@ bool DataflowState::cleanupAllDestroyAddr(
   bool madeChange = false;
   BasicBlockWorklist worklist(fn);
 
+  auto debugVarInst = DebugVarCarryingInst::getFromValue(address);
+
   LLVM_DEBUG(llvm::dbgs() << "Cleanup up destroy addr!\n");
   LLVM_DEBUG(llvm::dbgs() << "    Visiting destroys!\n");
   LLVM_DEBUG(llvm::dbgs() << "    Destroy Indices: " << destroyIndices << "\n");
@@ -1531,6 +1533,15 @@ bool DataflowState::cleanupAllDestroyAddr(
       continue;
     LLVM_DEBUG(llvm::dbgs() << "Converting reinit to init: " << *reinit);
     convertMemoryReinitToInitForm(*reinit);
+
+    // Make sure to create a new debug_value for the reinit value.
+    if (debugVarInst) {
+      if (auto varInfo = debugVarInst.getVarInfo()) {
+        SILBuilderWithScope reinitBuilder(*reinit);
+        reinitBuilder.createDebugValue(debugVarInst.inst->getLoc(), address,
+                                       *varInfo, false, /*was moved*/ true);
+      }
+    }
     madeChange = true;
   }
 
@@ -2025,6 +2036,14 @@ static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState,
             SILUndef::get(address->getType(), builder.getModule()), *varInfo,
             false,
             /*was moved*/ true);
+        {
+          // Make sure at the reinit point to create a new debug value after the
+          // reinit instruction so we reshow the variable.
+          auto *next = interestingUser->getNextInstruction();
+          SILBuilderWithScope reinitBuilder(next);
+          reinitBuilder.createDebugValue(debug.inst->getLoc(), address,
+                                         *varInfo, false, /*was moved*/ true);
+        }
       }
       debug.markAsMoved();
     }
diff --git a/test/DebugInfo/move_function_dbginfo.swift b/test/DebugInfo/move_function_dbginfo.swift
