[6.4] [DebugInfo] Fix consistency of op_deref handling

include/swift/SIL/SILInstruction.h

@@ -46,6 +46,7 @@
 #include "swift/Strings.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/ilist.h"
@@ -2052,8 +2053,7 @@ class AllocStackInst final
                              AllocationInst>,
       private SILDebugVariableSupplement,
       private llvm::TrailingObjects<AllocStackInst, SILType, SILLocation,
-                                    const SILDebugScope *, SILDIExprElement,
-                                    Operand, char> {
+                                    const SILDebugScope *, Operand, char> {
   friend TrailingObjects;
   friend SILBuilder;
 
@@ -2203,11 +2203,16 @@ class AllocStackInst final
     else if (complete)
       VarDeclScope = getDebugScope();
 
-    llvm::ArrayRef<SILDIExprElement> DIExprElements(
-        getTrailingObjects<SILDIExprElement>(), NumDIExprOperands);
+    // An alloc_stack always has a single implicit op_deref.
+    // It is not returned with complete = false, so that it isn't printed.
+    static const SILDIExprElement SingleDeref[] = {
+        SILDIExprElement::createOperator(SILDIExprOperator::Dereference)};
+    llvm::ArrayRef<SILDIExprElement> VarDIExpr = {};
+    if (complete)
+      VarDIExpr = SingleDeref;
 
     return VarInfo.get(getDecl(), getTrailingObjects<char>(), AuxVarType,
-                       VarDeclLoc, VarDeclScope, DIExprElements);
+                       VarDeclLoc, VarDeclScope, VarDIExpr);
   }
 
   /// True if this AllocStack has var info that a pass purposely invalidated.

lib/IRGen/AllocStackHoisting.cpp

@@ -199,8 +199,8 @@ void Partition::assignStackLocation(
     AllocStack->replaceAllUsesWith(AssignedLoc);
     if (auto VarInfo = AllocStack->getVarInfo()) {
       SILBuilder Builder(AllocStack, AllocStack->getDebugScope());
-      auto *DVI = Builder.createDebugValueAddr(AllocStack->getLoc(),
-                                               AssignedLoc, *VarInfo);
+      auto *DVI = Builder.createDebugValue(AllocStack->getLoc(),
+                                           AssignedLoc, *VarInfo);
       if (hasAtLeastOneMovedElt) {
         DVI->setUsesMoveableValueDebugInfo();
       }

lib/IRGen/IRGenDebugInfo.cpp

@@ -267,7 +267,7 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
                                DebugTypeInfo Ty, const SILDebugScope *DS,
                                std::optional<SILLocation> VarLoc,
                                SILDebugVariable VarInfo,
-                               IndirectionKind = DirectValue,
+                               bool InCoroContext = false,
                                ArtificialKind = RealValue,
                                AddrDbgInstrKind = AddrDbgInstrKind::DbgDeclare);
 
@@ -3727,7 +3727,7 @@ bool IRGenDebugInfoImpl::buildDebugInfoExpression(
 void IRGenDebugInfoImpl::emitVariableDeclaration(
     IRBuilder &Builder, ArrayRef<llvm::Value *> Storage, DebugTypeInfo DbgTy,
     const SILDebugScope *DS, std::optional<SILLocation> DbgInstLoc,
-    SILDebugVariable VarInfo, IndirectionKind Indirection,
+    SILDebugVariable VarInfo, bool InCoroContext,
     ArtificialKind Artificial, AddrDbgInstrKind AddrDInstrKind) {
   assert(DS && "variable has no scope");
 
@@ -3863,9 +3863,6 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
     if (DbgTy.getType()->isForeignReferenceType())
       Operands.push_back(llvm::dwarf::DW_OP_deref);
 
-    if (Indirection == IndirectValue || Indirection == CoroIndirectValue)
-      Operands.push_back(llvm::dwarf::DW_OP_deref);
-
     if (IsPiece) {
       // Advance the offset for the next piece.
       OffsetInBits += SizeInBits;
@@ -3899,8 +3896,7 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
     if (DIExpr)
       emitDbgIntrinsic(
           Builder, Piece, Var, DIExpr, DInstLine, DInstLoc.Column, Scope, DS,
-          Indirection == CoroDirectValue || Indirection == CoroIndirectValue,
-          AddrDInstrKind);
+          InCoroContext, AddrDInstrKind);
   }
 
   // Emit locationless intrinsic for variables that were optimized away.
@@ -3910,14 +3906,23 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
             appendDIExpression(DBuilder.createExpression(), NoFragment, false))
       emitDbgIntrinsic(Builder, llvm::ConstantInt::get(IGM.Int64Ty, 0), Var,
                        DIExpr, DInstLine, DInstLoc.Column, Scope, DS,
-                       Indirection == CoroDirectValue ||
-                           Indirection == CoroIndirectValue,
-                       AddrDInstrKind);
+                       InCoroContext, AddrDInstrKind);
   }
 }
 
 namespace {
 
+/// Strip the leading DW_OP_deref from the expression.
+/// dbg.declare implicitly provides one level of dereference, so the first
+/// DW_OP_deref is redundant.
+static llvm::DIExpression *stripLeadingDeref(llvm::DIExpression *Expr,
+                                             llvm::LLVMContext &Ctx) {
+  auto Elements = Expr->getElements();
+  if (!Elements.empty() && Elements[0] == llvm::dwarf::DW_OP_deref)
+    return llvm::DIExpression::get(Ctx, Elements.drop_front(1));
+  return Expr;
+}
+
 /// A helper struct that is used by emitDbgIntrinsic to factor redundant code.
 struct DbgIntrinsicEmitter {
   PointerUnion<llvm::BasicBlock *, llvm::Instruction *> InsertPt;
@@ -3959,15 +3964,19 @@ struct DbgIntrinsicEmitter {
                           llvm::DIExpression *Expr,
                           const llvm::DILocation *DL,
                           llvm::Instruction *InsertBefore) {
-    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclare)
+    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclare) {
+      Expr = stripLeadingDeref(Expr, InsertBefore->getContext());
       return DIBuilder.insertDeclare(Addr, VarInfo, Expr, DL,
                                      InsertBefore->getIterator());
+    }
 
-    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclareValue)
+    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclareValue) {
+      Expr = stripLeadingDeref(Expr, InsertBefore->getContext());
       return DIBuilder.insertDeclareValue(Addr, VarInfo, Expr, DL,
                                           InsertBefore->getIterator());
+    }
 
-    Expr = llvm::DIExpression::append(Expr, llvm::dwarf::DW_OP_deref);
+    // DbgValue: keep expression as-is (all derefs are explicit).
     return DIBuilder.insertDbgValueIntrinsic(Addr, VarInfo, Expr, DL,
                                              InsertBefore->getIterator());
   }
@@ -3976,13 +3985,17 @@ struct DbgIntrinsicEmitter {
                           llvm::DIExpression *Expr,
                           const llvm::DILocation *DL,
                           llvm::BasicBlock *Block) {
-    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclare)
+    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclare) {
+      Expr = stripLeadingDeref(Expr, Block->getContext());
       return DIBuilder.insertDeclare(Addr, VarInfo, Expr, DL, Block);
+    }
 
-    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclareValue)
+    if (ForceDbgDeclareOrDeclareValue == AddrDbgInstrKind::DbgDeclareValue) {
+      Expr = stripLeadingDeref(Expr, Block->getContext());
       return DIBuilder.insertDeclareValue(Addr, VarInfo, Expr, DL, Block);
+    }
 
-    Expr = llvm::DIExpression::append(Expr, llvm::dwarf::DW_OP_deref);
+    // DbgValue: keep expression as-is (all derefs are explicit).
     return DIBuilder.insertDbgValueIntrinsic(Addr, VarInfo, Expr, DL, Block);
   }
 };
@@ -4039,9 +4052,9 @@ void IRGenDebugInfoImpl::emitDbgIntrinsic(
 
   bool optimized = DS->getParentFunction()->shouldOptimize();
   if (optimized && (!InCoroContext || !Var->isParameter()))
-    AddrDInstKind = AddrDbgInstrKind::DbgValueDeref;
+    AddrDInstKind = AddrDbgInstrKind::DbgValue;
 
-  if (InCoroContext && AddrDInstKind != AddrDbgInstrKind::DbgValueDeref)
+  if (InCoroContext && AddrDInstKind != AddrDbgInstrKind::DbgValue)
     AddrDInstKind = AddrDbgInstrKind::DbgDeclareValue;
 
   DbgIntrinsicEmitter inserter{Builder, DBuilder, AddrDInstKind};
@@ -4200,10 +4213,7 @@ void IRGenDebugInfoImpl::emitTypeMetadata(IRGenFunction &IGF,
       Alignment(CI.getTargetInfo().getPointerAlign(clang::LangAS::Default)));
   emitVariableDeclaration(IGF.Builder, Metadata, DbgTy, IGF.getDebugScope(),
                           {}, {OS.str().str(), 0, false},
-                          // swift.type is already a pointer type,
-                          // having a shadow copy doesn't add another
-                          // layer of indirection.
-                          IGF.isAsync() ? CoroDirectValue : DirectValue,
+                          /*InCoroContext=*/IGF.isAsync(),
                           ArtificialValue);
 }
 
@@ -4224,7 +4234,7 @@ void IRGenDebugInfoImpl::emitPackCountParameter(IRGenFunction &IGF,
   auto DbgTy = *CompletedDebugTypeInfo::getFromTypeInfo(IntTy, TI, IGM);
   emitVariableDeclaration(
       IGF.Builder, Metadata, DbgTy, IGF.getDebugScope(), {}, VarInfo,
-      IGF.isAsync() ? CoroDirectValue : DirectValue, ArtificialValue);
+      /*InCoroContext=*/IGF.isAsync(), ArtificialValue);
 }
 
 } // anonymous namespace
@@ -4316,10 +4326,10 @@ void IRGenDebugInfo::emitOutlinedFunction(IRBuilder &Builder,
 void IRGenDebugInfo::emitVariableDeclaration(
     IRBuilder &Builder, ArrayRef<llvm::Value *> Storage, DebugTypeInfo Ty,
     const SILDebugScope *DS, std::optional<SILLocation> VarLoc,
-    SILDebugVariable VarInfo, IndirectionKind Indirection,
+    SILDebugVariable VarInfo, bool InCoroContext,
     ArtificialKind Artificial, AddrDbgInstrKind AddrDInstKind) {
   static_cast<IRGenDebugInfoImpl *>(this)->emitVariableDeclaration(
-      Builder, Storage, Ty, DS, VarLoc, VarInfo, Indirection, Artificial,
+      Builder, Storage, Ty, DS, VarLoc, VarInfo, InCoroContext, Artificial,
       AddrDInstKind);
 }
 

lib/IRGen/IRGenDebugInfo.h

@@ -38,21 +38,19 @@ class IRBuilder;
 class IRGenFunction;
 class IRGenModule;
 
-enum IndirectionKind {
-  DirectValue,
-  IndirectValue,
-  CoroDirectValue,
-  CoroIndirectValue
-};
 enum ArtificialKind : bool { RealValue = false, ArtificialValue = true };
 
-/// Used to signal to emitDbgIntrinsic that we actually want to emit dbg.declare
-/// instead of dbg.value + op_deref. By default, we now emit dbg.value instead of
-/// dbg.declare for normal variables. This is not true for metadata which
-/// truly are function wide and should be llvm.dbg.declare.
+/// Used to signal to emitDbgIntrinsic which kind of debug intrinsic to emit.
+/// - DbgDeclare: llvm.dbg.declare (variable lives at this address for its
+///   entire lifetime). The leading DW_OP_deref is stripped because declare
+///   implicitly provides one level of indirection.
+/// - DbgValue: llvm.dbg.value (expression evaluated as-is; all derefs are
+///   explicit).
+/// - DbgDeclareValue: llvm.dbg.declare_value (coro context variant). The
+///   leading DW_OP_deref is stripped like DbgDeclare.
 enum class AddrDbgInstrKind : uint8_t {
   DbgDeclare,
-  DbgValueDeref,
+  DbgValue,
   DbgDeclareValue,
 };
 
@@ -164,7 +162,7 @@ class IRGenDebugInfo {
                                DebugTypeInfo Ty, const SILDebugScope *DS,
                                std::optional<SILLocation> VarLoc,
                                SILDebugVariable VarInfo,
-                               IndirectionKind Indirection = DirectValue,
+                               bool InCoroContext = false,
                                ArtificialKind Artificial = RealValue,
                                AddrDbgInstrKind = AddrDbgInstrKind::DbgDeclare);