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

This patch replace all in-memory objects of DebugValueAddrInst with
DebugValueInst + op_deref, and duplicates logics that handles
DebugValueAddrInst with the latter. All related check in the tests
have been updated as well.

Note that this patch neither remove the DebugValueAddrInst class nor
remove `debug_value_addr` syntax in the test inputs.

Author: mshockwave

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()
 
@@ -4712,6 +4714,17 @@ class DebugValueInst final
     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/IRGenSIL.cpp

@@ -4770,14 +4770,11 @@ void IRGenSILFunction::visitDebugValueInst(DebugValueInst *i) {
   } else
     return;
 
-  // Calculate the indirection
-  IndirectionKind Indirection = DirectValue;
-  if (IsInCoro)
-    Indirection = IsAddrVal ? CoroIndirectValue : CoroDirectValue;
-  else if (IsAddrVal && !isa<AllocStackInst>(SILVal))
-    Indirection = IndirectValue;
-
-  // 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;
   if (IsAddrVal)
     Copy.emplace_back(

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/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;
     }

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -514,6 +514,10 @@ bool SILCombiner::optimizeStackAllocatedEnum(AllocStackInst *AS) {
       case SILInstructionKind::InjectEnumAddrInst:
         // We'll check init_enum_addr below.
         break;
+      case SILInstructionKind::DebugValueInst:
+        if (DebugValueInst::hasAddrVal(user))
+          break;
+        return false;
       case SILInstructionKind::InitEnumDataAddrInst: {
         auto *ieda = cast<InitEnumDataAddrInst>(user);
         auto *el = ieda->getElement();
@@ -599,6 +603,12 @@ bool SILCombiner::optimizeStackAllocatedEnum(AllocStackInst *AS) {
         eraseInstFromFunction(*svi);
         break;
       }
+      case SILInstructionKind::DebugValueInst:
+        if (DebugValueInst::hasAddrVal(user)) {
+          eraseInstFromFunction(*user);
+          break;
+        }
+        LLVM_FALLTHROUGH;
       default:
         llvm_unreachable("unexpected alloc_stack user");
     }