[DebugInfo] PATCH 2/3: Duplicate logics regarding debug_value_addr

include/swift/SIL/SILBuilder.h

@@ -927,8 +927,8 @@ class SILBuilder {
   DebugValueInst *createDebugValue(SILLocation Loc, SILValue src,
                                    SILDebugVariable Var,
                                    bool poisonRefs = false);
-  DebugValueAddrInst *createDebugValueAddr(SILLocation Loc, SILValue src,
-                                           SILDebugVariable Var);
+  DebugValueInst *createDebugValueAddr(SILLocation Loc, SILValue src,
+                                       SILDebugVariable Var);
 
   /// Create a debug_value_addr if \p src is an address; a debug_value if not.
   SILInstruction *emitDebugDescription(SILLocation Loc, SILValue src,

include/swift/SIL/SILDebugInfoExpression.h

@@ -162,6 +162,15 @@ class SILDebugInfoExpression {
     appendElements(Tail.Elements);
   }
 
+  void prependElements(llvm::ArrayRef<SILDIExprElement> NewElements) {
+    Elements.insert(Elements.begin(),
+                    NewElements.begin(), NewElements.end());
+  }
+
+  void eraseElement(const_iterator It) {
+    Elements.erase(It);
+  }
+
   /// The iterator for SILDIExprOperand
   class op_iterator {
     friend class SILDebugInfoExpression;

include/swift/SIL/SILInstruction.h

@@ -4672,6 +4672,8 @@ class DebugValueInst final
   static DebugValueInst *create(SILDebugLocation DebugLoc, SILValue Operand,
                                 SILModule &M, SILDebugVariable Var,
                                 bool poisonRefs);
+  static DebugValueInst *createAddr(SILDebugLocation DebugLoc, SILValue Operand,
+                                    SILModule &M, SILDebugVariable Var);
 
   SIL_DEBUG_VAR_SUPPLEMENT_TRAILING_OBJS_IMPL()
 
@@ -4706,6 +4708,23 @@ class DebugValueInst final
       *getTrailingObjects<const SILDebugScope *>() = NewDS;
   }
 
+  /// Whether the SSA value associated with the current debug_value
+  /// instruction has an address type.
+  bool hasAddrVal() const {
+    return getOperand()->getType().isAddress();
+  }
+
+  /// An utility to check if \p I is DebugValueInst and
+  /// whether it's associated with address type SSA value.
+  static DebugValueInst *hasAddrVal(SILInstruction *I) {
+    auto *DVI = dyn_cast_or_null<DebugValueInst>(I);
+    return DVI && DVI->hasAddrVal()? DVI : nullptr;
+  }
+
+  /// Whether the attached di-expression (if there is any) starts
+  /// with `op_deref`.
+  bool exprStartsWithDeref() const;
+
   /// True if all references within this debug value will be overwritten with a
   /// poison sentinel at this point in the program. This is used in debug builds
   /// when shortening non-trivial value lifetimes to ensure the debugger cannot

lib/IRGen/IRGenDebugInfo.cpp

@@ -2410,6 +2410,9 @@ bool IRGenDebugInfoImpl::buildDebugInfoExpression(
       if (!handleFragmentDIExpr(ExprOperand, Operands))
         return false;
       break;
+    case SILDIExprOperator::Dereference:
+      Operands.push_back(llvm::dwarf::DW_OP_deref);
+      break;
     default:
       llvm_unreachable("Unrecognized operator");
     }

lib/IRGen/IRGenSIL.cpp

@@ -1637,7 +1637,7 @@ void LoweredValue::getExplosion(IRGenFunction &IGF, SILType type,
   case Kind::DynamicallyEnforcedAddress:
   case Kind::CoroutineState:
     llvm_unreachable("not a value");
-      
+
   case Kind::ExplosionVector:
     ex.add(Storage.get<ExplosionVector>(kind));
     return;
@@ -4713,7 +4713,11 @@ static bool InCoroContext(SILFunction &f, SILInstruction &i) {
 }
 
 void IRGenSILFunction::visitDebugValueInst(DebugValueInst *i) {
+  auto SILVal = i->getOperand();
+  bool IsAddrVal = SILVal->getType().isAddress();
   if (i->poisonRefs()) {
+    assert(!IsAddrVal &&
+           "SIL values with address type should not have poison");
     emitPoisonDebugValueInst(i);
     return;
   }
@@ -4722,16 +4726,17 @@ void IRGenSILFunction::visitDebugValueInst(DebugValueInst *i) {
 
   auto VarInfo = i->getVarInfo();
   assert(VarInfo && "debug_value without debug info");
-  auto SILVal = i->getOperand();
   if (isa<SILUndef>(SILVal)) {
     // We cannot track the location of inlined error arguments because it has no
     // representation in SIL.
-    if (!i->getDebugScope()->InlinedCallSite && VarInfo->Name == "$error") {
+    if (!IsAddrVal &&
+        !i->getDebugScope()->InlinedCallSite && VarInfo->Name == "$error") {
       auto funcTy = CurSILFn->getLoweredFunctionType();
       emitErrorResultVar(funcTy, funcTy->getErrorResult(), i);
     }
     return;
   }
+  bool IsInCoro = InCoroContext(*CurSILFn, *i);
 
   bool IsAnonymous = false;
   VarInfo->Name = getVarName(i, IsAnonymous);
@@ -4742,27 +4747,47 @@ void IRGenSILFunction::visitDebugValueInst(DebugValueInst *i) {
     SILTy = *MaybeSILTy;
   else
     SILTy = SILVal->getType();
+
   auto RealTy = SILTy.getASTType();
+  if (IsAddrVal && IsInCoro)
+    if (auto *PBI = dyn_cast<ProjectBoxInst>(i->getOperand())) {
+      // Usually debug info only ever describes the *result* of a projectBox
+      // call. To allow the debugger to display a boxed parameter of an async
+      // continuation object, however, the debug info can only describe the box
+      // itself and thus also needs to emit a box type for it so the debugger
+      // knows to call into Remote Mirrors to unbox the value.
+      RealTy = PBI->getOperand()->getType().getASTType();
+      assert(isa<SILBoxType>(RealTy));
+    }
+
+  // Figure out the debug variable type
   if (VarDecl *Decl = i->getDecl()) {
     DbgTy = DebugTypeInfo::getLocalVariable(
         Decl, RealTy, getTypeInfo(SILVal->getType()));
   } else if (!SILTy.hasArchetype() && !VarInfo->Name.empty()) {
-    // Preliminary support for .sil debug information.
+    // Handle the cases that read from a SIL file
     DbgTy = DebugTypeInfo::getFromTypeInfo(RealTy, getTypeInfo(SILTy));
   } else
     return;
 
-  // Put the value into a stack slot at -Onone.
+  // Since debug_value is expressing indirection explicitly via op_deref,
+  // we're not using either IndirectValue or CoroIndirectValue here.
+  IndirectionKind Indirection = IsInCoro? CoroDirectValue : DirectValue;
+
+  // Put the value into a shadow-copy stack slot at -Onone.
   llvm::SmallVector<llvm::Value *, 8> Copy;
-  emitShadowCopyIfNeeded(SILVal, i->getDebugScope(), *VarInfo, IsAnonymous,
-                         Copy);
+  if (IsAddrVal)
+    Copy.emplace_back(
+      emitShadowCopyIfNeeded(getLoweredAddress(SILVal).getAddress(),
+                             i->getDebugScope(), *VarInfo, IsAnonymous));
+  else
+    emitShadowCopyIfNeeded(SILVal, i->getDebugScope(), *VarInfo, IsAnonymous,
+                           Copy);
+
   bindArchetypes(DbgTy.getType());
   if (!IGM.DebugInfo)
     return;
 
-  IndirectionKind Indirection =
-    InCoroContext(*CurSILFn, *i) ? CoroDirectValue : DirectValue;
-
   emitDebugVariableDeclaration(Copy, DbgTy, SILTy, i->getDebugScope(),
                                i->getLoc(), *VarInfo, Indirection);
 }

lib/SIL/IR/SILBuilder.cpp

@@ -569,20 +569,19 @@ void SILBuilder::emitDestructureValueOperation(
 DebugValueInst *SILBuilder::createDebugValue(SILLocation Loc, SILValue src,
                                              SILDebugVariable Var,
                                              bool poisonRefs) {
-  assert(isLoadableOrOpaque(src->getType()));
   // Debug location overrides cannot apply to debug value instructions.
   DebugLocOverrideRAII LocOverride{*this, None};
   return insert(
     DebugValueInst::create(getSILDebugLocation(Loc), src, getModule(), Var,
                            poisonRefs));
 }
 
-DebugValueAddrInst *SILBuilder::createDebugValueAddr(SILLocation Loc,
-                                                     SILValue src,
-                                                     SILDebugVariable Var) {
+DebugValueInst *SILBuilder::createDebugValueAddr(SILLocation Loc,
+                                                 SILValue src,
+                                                 SILDebugVariable Var) {
   // Debug location overrides cannot apply to debug addr instructions.
   DebugLocOverrideRAII LocOverride{*this, None};
-  return insert(DebugValueAddrInst::create(getSILDebugLocation(Loc), src,
+  return insert(DebugValueInst::createAddr(getSILDebugLocation(Loc), src,
                                            getModule(), Var));
 }
 

lib/SIL/IR/SILInstructions.cpp

@@ -357,6 +357,29 @@ DebugValueInst *DebugValueInst::create(SILDebugLocation DebugLoc,
   return ::new (buf) DebugValueInst(DebugLoc, Operand, Var, poisonRefs);
 }
 
+DebugValueInst *DebugValueInst::createAddr(SILDebugLocation DebugLoc,
+                                           SILValue Operand, SILModule &M,
+                                           SILDebugVariable Var) {
+  // For alloc_stack, debug_value is used to annotate the associated
+  // memory location, so we shouldn't attach op_deref.
+  if (!isa<AllocStackInst>(Operand))
+    Var.DIExpr.prependElements(
+      {SILDIExprElement::createOperator(SILDIExprOperator::Dereference)});
+  void *buf = allocateDebugVarCarryingInst<DebugValueInst>(M, Var);
+  return ::new (buf) DebugValueInst(DebugLoc, Operand, Var,
+                                    /*poisonRefs=*/false);
+}
+
+bool DebugValueInst::exprStartsWithDeref() const {
+  if (!NumDIExprOperands)
+    return false;
+
+  llvm::ArrayRef<SILDIExprElement> DIExprElements(
+      getTrailingObjects<SILDIExprElement>(), NumDIExprOperands);
+  return DIExprElements.front().getAsOperator()
+          == SILDIExprOperator::Dereference;
+}
+
 DebugValueAddrInst::DebugValueAddrInst(SILDebugLocation DebugLoc,
                                        SILValue Operand, SILDebugVariable Var)
     : UnaryInstructionBase(DebugLoc, Operand),

lib/SIL/IR/SILPrinter.cpp

@@ -1179,31 +1179,37 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
 
   void printDebugVar(Optional<SILDebugVariable> Var,
                      const SourceManager *SM = nullptr) {
-    if (!Var || Var->Name.empty())
+    if (!Var)
       return;
-    if (Var->Constant)
-      *this << ", let";
-    else
-      *this << ", var";
-
-    if ((Var->Loc || Var->Scope) && SM) {
-      *this << ", (name \"" << Var->Name << '"';
-      if (Var->Loc)
-        printDebugLocRef(*Var->Loc, *SM);
-      if (Var->Scope)
-        printDebugScopeRef(Var->Scope, *SM);
-      *this << ")";
-    } else
-      *this << ", name \"" << Var->Name << '"';
 
-    if (Var->ArgNo)
-      *this << ", argno " << Var->ArgNo;
-    if (Var->Implicit)
-      *this << ", implicit";
-    if (Var->Type) {
-      *this << ", type ";
-      Var->Type->print(PrintState.OS, PrintState.ASTOptions);
+    if (!Var->Name.empty()) {
+      if (Var->Constant)
+        *this << ", let";
+      else
+        *this << ", var";
+
+      if ((Var->Loc || Var->Scope) && SM) {
+        *this << ", (name \"" << Var->Name << '"';
+        if (Var->Loc)
+          printDebugLocRef(*Var->Loc, *SM);
+        if (Var->Scope)
+          printDebugScopeRef(Var->Scope, *SM);
+        *this << ")";
+      } else
+        *this << ", name \"" << Var->Name << '"';
+
+      if (Var->ArgNo)
+        *this << ", argno " << Var->ArgNo;
+      if (Var->Implicit)
+        *this << ", implicit";
+      if (Var->Type) {
+        *this << ", type ";
+        Var->Type->print(PrintState.OS, PrintState.ASTOptions);
+      }
     }
+    // Although it's rare in real-world use cases, but during testing,
+    // sometimes we want to print out di-expression, even the debug
+    // variable name is empty.
     if (Var->DIExpr)
       printDebugInfoExpression(Var->DIExpr);
   }

lib/SIL/Parser/ParseSIL.cpp

@@ -1649,7 +1649,10 @@ bool SILParser::parseSILDebugInfoExpression(SILDebugInfoExpression &DIExpr) {
     return true;
 
   // All operators that we currently support
-  static const SILDIExprOperator AllOps[] = {SILDIExprOperator::Fragment};
+  static const SILDIExprOperator AllOps[] = {
+    SILDIExprOperator::Dereference,
+    SILDIExprOperator::Fragment
+  };
 
   do {
     P.consumeToken();

lib/SIL/Utils/MemAccessUtils.cpp

@@ -1629,6 +1629,10 @@ swift::getSingleInitAllocStackUse(AllocStackInst *asi,
       if (destroyingUses)
         destroyingUses->push_back(use);
       continue;
+    case SILInstructionKind::DebugValueInst:
+      if (cast<DebugValueInst>(user)->hasAddrVal())
+        continue;
+      break;
     case SILInstructionKind::DeallocStackInst:
     case SILInstructionKind::LoadBorrowInst:
     case SILInstructionKind::DebugValueAddrInst:

lib/SIL/Utils/MemoryLocations.cpp

@@ -362,6 +362,10 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
         if (!cast<LoadBorrowInst>(user)->getUsersOfType<BranchInst>().empty())
           return false;
         break;
+      case SILInstructionKind::DebugValueInst:
+        if (cast<DebugValueInst>(user)->hasAddrVal())
+          break;
+        return false;
       case SILInstructionKind::InjectEnumAddrInst:
       case SILInstructionKind::SelectEnumAddrInst:
       case SILInstructionKind::ExistentialMetatypeInst:

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -613,6 +613,11 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
       case SILInstructionKind::DebugValueAddrInst:
         requireBitsSet(bits, I.getOperand(0), &I);
         break;
+      case SILInstructionKind::DebugValueInst:
+        if (cast<DebugValueInst>(&I)->hasAddrVal() &&
+            cast<DebugValueInst>(&I)->exprStartsWithDeref())
+          requireBitsSet(bits, I.getOperand(0), &I);
+        break;
       case SILInstructionKind::UncheckedTakeEnumDataAddrInst: {
         // Note that despite the name, unchecked_take_enum_data_addr does _not_
         // "take" the payload of the Swift.Optional enum. This is a terrible

lib/SIL/Verifier/SILVerifier.cpp

@@ -545,6 +545,13 @@ struct ImmutableAddressUseVerifier {
       case SILInstructionKind::SwitchEnumAddrInst:
       case SILInstructionKind::SelectEnumAddrInst:
         break;
+      case SILInstructionKind::DebugValueInst:
+        if (cast<DebugValueInst>(inst)->hasAddrVal())
+          break;
+        else {
+          llvm::errs() << "Unhandled, unexpected instruction: " << *inst;
+          llvm_unreachable("invoking standard assertion failure");
+        }
       case SILInstructionKind::AddressToPointerInst:
         // We assume that the user is attempting to do something unsafe since we
         // are converting to a raw pointer. So just ignore this use.

lib/SILOptimizer/Analysis/AccessSummaryAnalysis.cpp

@@ -124,6 +124,10 @@ void AccessSummaryAnalysis::processArgument(FunctionInfo *info,
       processPartialApply(info, argumentIndex, cast<PartialApplyInst>(user),
                           operand, order);
       break;
+    case SILInstructionKind::DebugValueInst:
+      if (DebugValueInst::hasAddrVal(user))
+        break;
+      LLVM_FALLTHROUGH;
     default:
       // FIXME: These likely represent scenarios in which we're not generating
       // begin access markers. Ignore these for now. But we really should

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

@@ -374,6 +374,10 @@ static bool hasEscapingUses(SILValue address, int &numChecks) {
       case SILInstructionKind::EndAccessInst:
         // Those instructions have no result and cannot escape the address.
         break;
+      case SILInstructionKind::DebugValueInst:
+        if (DebugValueInst::hasAddrVal(user))
+          break;
+        return true;
       case SILInstructionKind::ApplyInst:
       case SILInstructionKind::TryApplyInst:
       case SILInstructionKind::BeginApplyInst:

lib/SILOptimizer/Mandatory/CapturePromotion.cpp

@@ -313,6 +313,7 @@ class ClosureCloner : public SILClonerWithScopes<ClosureCloner> {
 
   SILValue getProjectBoxMappedVal(SILValue operandValue);
 
+  void visitDebugValueInst(DebugValueInst *inst);
   void visitDebugValueAddrInst(DebugValueAddrInst *inst);
   void visitDestroyValueInst(DestroyValueInst *inst);
   void visitStructElementAddrInst(StructElementAddrInst *inst);
@@ -581,6 +582,17 @@ void ClosureCloner::visitDebugValueAddrInst(DebugValueAddrInst *inst) {
   }
   SILCloner<ClosureCloner>::visitDebugValueAddrInst(inst);
 }
+/// Doing the same thing as ClosureCloner::visitDebugValueAddrInst. Only used
+/// when transitioning away from debug_value_addr.
+void ClosureCloner::visitDebugValueInst(DebugValueInst *inst) {
+  if (inst->hasAddrVal())
+    if (SILValue value = getProjectBoxMappedVal(inst->getOperand())) {
+      getBuilder().setCurrentDebugScope(getOpScope(inst->getDebugScope()));
+      getBuilder().createDebugValue(inst->getLoc(), value, *inst->getVarInfo());
+      return;
+    }
+  SILCloner<ClosureCloner>::visitDebugValueInst(inst);
+}
 
 /// Handle a destroy_value instruction during cloning of a closure; if it is a
 /// destroy_value of a promoted box argument, then it is replaced with a
@@ -855,6 +867,7 @@ getPartialApplyArgMutationsAndEscapes(PartialApplyInst *pai,
     }
 
     if (isa<DebugValueAddrInst>(addrUser) ||
+        DebugValueInst::hasAddrVal(addrUser) ||
         isa<MarkFunctionEscapeInst>(addrUser) || isa<EndAccessInst>(addrUser)) {
       return false;
     }