[SILOptimizer][DebugInfo] Preliminary support for DIExpression in SROA and Mem2Reg

SROA and Mem2Reg now can leverage DIExpression -- op_fragment, more
specifically -- to generate correct debug info for optimized SIL. Some
important highlights:
 - The new swift::salvageDebugInfo, similar to llvm::salvageDebugInfo,
   tries to restore / transfer debug info from a deleted instruction.
   Currently I only implemented this for store instruction whose
   destination is an alloc_stack value.
 - Since we now have source-variable-specific SIL location inside a
   `debug_value` instruction (and its friends), this patch teaches
   SILCloner and SILInliner to remap the debug scope there in addition
   to debug scope of the instruction.
 - DCE now does not remove `debug_value` instruction whose associating
   with a function argument SSA value that is not used elsewhere. Since
   that SSA value will not disappear so we should keep the debug info.

Author: mshockwave

include/swift/SIL/DebugUtils.h

@@ -308,6 +308,24 @@ struct DebugVarCarryingInst {
     }
     llvm_unreachable("covered switch");
   }
+
+  void setDebugVarScope(const SILDebugScope *NewDS) {
+    switch (kind) {
+    case Kind::Invalid:
+      llvm_unreachable("Invalid?!");
+    case Kind::DebugValue:
+      cast<DebugValueInst>(inst)->setDebugVarScope(NewDS);
+      break;
+    case Kind::DebugValueAddr:
+      cast<DebugValueAddrInst>(inst)->setDebugVarScope(NewDS);
+      break;
+    case Kind::AllocStack:
+      cast<AllocStackInst>(inst)->setDebugVarScope(NewDS);
+      break;
+    case Kind::AllocBox:
+      llvm_unreachable("Not implemented");
+    }
+  }
 };
 
 } // end namespace swift

include/swift/SIL/SILCloner.h

@@ -19,6 +19,7 @@
 
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/SIL/BasicBlockUtils.h"
+#include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/Dominance.h"
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SIL/SILDebugScope.h"
@@ -245,6 +246,16 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
     registerOpenedExistentialRemapping(archetypeTy, replacementTy);
   }
 
+  // SILCloner will take care of debug scope on the instruction
+  // and this helper will remap the auxiliary debug scope too, if there is any.
+  void remapDebugVarInfo(DebugVarCarryingInst DbgVarInst) {
+    if (!DbgVarInst)
+      return;
+    auto VarInfo = DbgVarInst.getVarInfo();
+    if (VarInfo && VarInfo->Scope)
+      DbgVarInst.setDebugVarScope(getOpScope(VarInfo->Scope));
+  }
+
   ProtocolConformanceRef getOpConformance(Type ty,
                                           ProtocolConformanceRef conformance) {
     // If we have open existentials to substitute, do so now.
@@ -748,9 +759,11 @@ SILCloner<ImplClass>::visitAllocStackInst(AllocStackInst *Inst) {
     Loc = MandatoryInlinedLocation::getAutoGeneratedLocation();
     VarInfo = None;
   }
-  recordClonedInstruction(Inst, getBuilder().createAllocStack(
-                                    Loc, getOpType(Inst->getElementType()),
-                                    VarInfo, Inst->hasDynamicLifetime()));
+  auto *NewInst =
+      getBuilder().createAllocStack(Loc, getOpType(Inst->getElementType()),
+                                    VarInfo, Inst->hasDynamicLifetime());
+  remapDebugVarInfo(DebugVarCarryingInst(NewInst));
+  recordClonedInstruction(Inst, NewInst);
 }
 
 template<typename ImplClass>
@@ -1232,12 +1245,13 @@ SILCloner<ImplClass>::visitDebugValueInst(DebugValueInst *Inst) {
     return;
 
   // Since we want the debug info to survive, we do not remap the location here.
+  SILDebugVariable VarInfo = *Inst->getVarInfo();
   getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
-  recordClonedInstruction(
-      Inst, getBuilder().createDebugValue(Inst->getLoc(),
-                                          getOpValue(Inst->getOperand()),
-                                          *Inst->getVarInfo(),
-                                          Inst->poisonRefs()));
+  auto *NewInst = getBuilder().createDebugValue(Inst->getLoc(),
+                                                getOpValue(Inst->getOperand()),
+                                                VarInfo, Inst->poisonRefs());
+  remapDebugVarInfo(DebugVarCarryingInst(NewInst));
+  recordClonedInstruction(Inst, NewInst);
 }
 template<typename ImplClass>
 void
@@ -1249,11 +1263,13 @@ SILCloner<ImplClass>::visitDebugValueAddrInst(DebugValueAddrInst *Inst) {
     return;
 
   // Do not remap the location for a debug Instruction.
+  SILDebugVariable VarInfo = *Inst->getVarInfo();
   SILValue OpValue = getOpValue(Inst->getOperand());
   getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
-  recordClonedInstruction(
-      Inst, getBuilder().createDebugValueAddr(Inst->getLoc(), OpValue,
-                                              *Inst->getVarInfo()));
+  auto *NewInst = getBuilder().createDebugValueAddr(Inst->getLoc(), OpValue,
+                                                    *Inst->getVarInfo());
+  remapDebugVarInfo(DebugVarCarryingInst(NewInst));
+  recordClonedInstruction(Inst, NewInst);
 }
 
 #define NEVER_LOADABLE_CHECKED_REF_STORAGE(Name, name, ...)                    \

include/swift/SIL/SILDebugInfoExpression.h

@@ -123,6 +123,8 @@ class SILDebugInfoExpression {
   explicit SILDebugInfoExpression(llvm::ArrayRef<SILDIExprElement> EL)
       : Elements(EL.begin(), EL.end()) {}
 
+  void clear() { Elements.clear(); }
+
   size_t getNumElements() const { return Elements.size(); }
 
   using iterator = typename decltype(Elements)::iterator;
@@ -151,6 +153,15 @@ class SILDebugInfoExpression {
     Elements.push_back(Element);
   }
 
+  void appendElements(llvm::ArrayRef<SILDIExprElement> NewElements) {
+    if (NewElements.size())
+      Elements.append(NewElements.begin(), NewElements.end());
+  }
+
+  void append(const SILDebugInfoExpression &Tail) {
+    appendElements(Tail.Elements);
+  }
+
   /// The iterator for SILDIExprOperand
   class op_iterator {
     friend class SILDebugInfoExpression;
@@ -208,6 +219,9 @@ class SILDebugInfoExpression {
 
   /// Return true if this expression is not empty
   inline operator bool() const { return Elements.size(); }
+
+  /// Create a op_fragment expression
+  static SILDebugInfoExpression createFragment(VarDecl *Field);
 };
 } // end namespace swift
 #endif

include/swift/SIL/SILInstruction.h

@@ -1732,6 +1732,10 @@ struct SILDebugVariable {
   const SILDebugScope *Scope;
   SILDebugInfoExpression DIExpr;
 
+  // Use vanilla copy ctor / operator
+  SILDebugVariable(const SILDebugVariable &) = default;
+  SILDebugVariable &operator=(const SILDebugVariable &) = default;
+
   SILDebugVariable()
       : ArgNo(0), Constant(false), Implicit(false), Scope(nullptr) {}
   SILDebugVariable(bool Constant, uint16_t ArgNo)
@@ -1744,6 +1748,10 @@ struct SILDebugVariable {
       : Name(Name), ArgNo(ArgNo), Constant(Constant), Implicit(IsImplicit),
         Type(AuxType), Loc(DeclLoc), Scope(DeclScope), DIExpr(ExprElements) {}
 
+  /// Created from either AllocStack or AllocBox instruction
+  static Optional<SILDebugVariable>
+  createFromAllocation(const AllocationInst *AI);
+
   // We're not comparing DIExpr here because strictly speaking,
   // DIExpr is not part of the debug variable. We simply piggyback
   // it in this class so that's it's easier to carry DIExpr around.
@@ -1962,6 +1970,11 @@ class AllocStackInst final
     SILNode::Bits.AllocStackInst.VarInfo = VI.getRawValue();
   }
 
+  void setDebugVarScope(const SILDebugScope *NewDS) {
+    if (hasAuxDebugScope())
+      *getTrailingObjects<const SILDebugScope *>() = NewDS;
+  }
+
   /// getElementType - Get the type of the allocated memory (as opposed to the
   /// type of the instruction itself, which will be an address type).
   SILType getElementType() const {
@@ -4681,6 +4694,11 @@ class DebugValueInst final
                        VarDeclLoc, VarDeclScope, DIExprElements);
   }
 
+  void setDebugVarScope(const SILDebugScope *NewDS) {
+    if (hasAuxDebugScope())
+      *getTrailingObjects<const SILDebugScope *>() = NewDS;
+  }
+
   /// 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
@@ -4741,7 +4759,12 @@ class DebugValueAddrInst final
 
     return VarInfo.get(getDecl(), getTrailingObjects<char>(), AuxVarType,
                        VarDeclLoc, VarDeclScope, DIExprElements);
-  };
+  }
+
+  void setDebugVarScope(const SILDebugScope *NewDS) {
+    if (hasAuxDebugScope())
+      *getTrailingObjects<const SILDebugScope *>() = NewDS;
+  }
 };
 
 /// An abstract class representing a load from some kind of reference storage.

include/swift/SILOptimizer/Utils/DebugOptUtils.h

@@ -43,6 +43,11 @@ inline void deleteAllDebugUses(SILInstruction *inst,
   }
 }
 
+/// Transfer debug info associated with (the result of) \p I to a
+/// new `debug_value` or `debug_value_addr` instruction before \p I is
+/// deleted.
+void salvageDebugInfo(SILInstruction *I);
+
 /// Erases the instruction \p I from it's parent block and deletes it, including
 /// all debug instructions which use \p I.
 /// Precondition: The instruction may only have debug instructions as uses.
@@ -76,6 +81,7 @@ eraseFromParentWithDebugInsts(SILInstruction *inst,
   // Just matching what eraseFromParentWithDebugInsts is today.
   if (nextII == inst->getIterator())
     ++nextII;
+  swift::salvageDebugInfo(inst);
   callbacks.deleteInst(inst, false /*do not notify*/);
   return nextII;
 }

lib/IRGen/IRGenDebugInfo.cpp

@@ -191,9 +191,11 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
   void emitArtificialFunction(IRBuilder &Builder, llvm::Function *Fn,
                               SILType SILTy);
 
-  void handleFragmentDIExpr(const SILDIExprOperand &CurDIExprOp,
+  /// Return false if we fail to create the right DW_OP_LLVM_fragment operand.
+  bool handleFragmentDIExpr(const SILDIExprOperand &CurDIExprOp,
                             SmallVectorImpl<uint64_t> &Operands);
-  void buildDebugInfoExpression(const SILDebugVariable &VarInfo,
+  /// Return false if we fail to create the desired !DIExpression.
+  bool buildDebugInfoExpression(const SILDebugVariable &VarInfo,
                                 SmallVectorImpl<uint64_t> &Operands);
 
   void emitVariableDeclaration(IRBuilder &Builder,
@@ -2336,9 +2338,8 @@ void IRGenDebugInfoImpl::emitArtificialFunction(IRBuilder &Builder,
   setCurrentLoc(Builder, Scope, ALoc);
 }
 
-void IRGenDebugInfoImpl::handleFragmentDIExpr(
-    const SILDIExprOperand &CurDIExprOp,
-    SmallVectorImpl<uint64_t> &Operands) {
+bool IRGenDebugInfoImpl::handleFragmentDIExpr(
+    const SILDIExprOperand &CurDIExprOp, SmallVectorImpl<uint64_t> &Operands) {
   assert(CurDIExprOp.getOperator() == SILDIExprOperator::Fragment);
   // Expecting a VarDecl that points to a field in an struct
   auto DIExprArgs = CurDIExprOp.args();
@@ -2348,14 +2349,22 @@ void IRGenDebugInfoImpl::handleFragmentDIExpr(
                "DIExprOperator::Fragment");
   // Translate the based type
   DeclContext *ParentDecl = VD->getDeclContext();
+  assert(ParentDecl && "VarDecl has no parent context?");
   SILType ParentSILType =
       IGM.getLoweredType(ParentDecl->getDeclaredTypeInContext());
   // Retrieve the offset & size of the field
   llvm::Constant *Offset =
       emitPhysicalStructMemberFixedOffset(IGM, ParentSILType, VD);
-  llvm::Type *FieldTy =
-      getPhysicalStructFieldTypeInfo(IGM, ParentSILType, VD)->getStorageType();
-  assert(Offset && FieldTy && "Non-fixed type?");
+  auto *FieldTypeInfo = getPhysicalStructFieldTypeInfo(IGM, ParentSILType, VD);
+  // FIXME: This will only happen if IRGen hasn't processed ParentSILType
+  // (into its own representation) but we probably should ask IRGen to process
+  // it right now.
+  if (!FieldTypeInfo)
+    return false;
+  llvm::Type *FieldTy = FieldTypeInfo->getStorageType();
+  // Doesn't support non-fixed type right now
+  if (!Offset || !FieldTy)
+    return false;
 
   uint64_t SizeOfByte = CI.getTargetInfo().getCharWidth();
   uint64_t SizeInBits = IGM.DataLayout.getTypeSizeInBits(FieldTy);
@@ -2366,22 +2375,26 @@ void IRGenDebugInfoImpl::handleFragmentDIExpr(
   Operands.push_back(llvm::dwarf::DW_OP_LLVM_fragment);
   Operands.push_back(OffsetInBits);
   Operands.push_back(SizeInBits);
+
+  return true;
 }
 
-void IRGenDebugInfoImpl::buildDebugInfoExpression(
+bool IRGenDebugInfoImpl::buildDebugInfoExpression(
     const SILDebugVariable &VarInfo, SmallVectorImpl<uint64_t> &Operands) {
   assert(VarInfo.DIExpr && "SIL debug info expression not found");
 
   const auto &DIExpr = VarInfo.DIExpr;
   for (const SILDIExprOperand &ExprOperand : DIExpr.operands()) {
     switch (ExprOperand.getOperator()) {
     case SILDIExprOperator::Fragment:
-      handleFragmentDIExpr(ExprOperand, Operands);
+      if (!handleFragmentDIExpr(ExprOperand, Operands))
+        return false;
       break;
     default:
-      llvm_unreachable("Unrecoginized operator");
+      llvm_unreachable("Unrecognized operator");
     }
   }
+  return true;
 }
 
 void IRGenDebugInfoImpl::emitVariableDeclaration(
@@ -2455,7 +2468,8 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
       [&VarInfo, this](llvm::DIExpression *DIExpr) -> llvm::DIExpression * {
     if (VarInfo.DIExpr) {
       llvm::SmallVector<uint64_t, 2> Operands;
-      buildDebugInfoExpression(VarInfo, Operands);
+      if (!buildDebugInfoExpression(VarInfo, Operands))
+        return nullptr;
       if (Operands.size())
         return llvm::DIExpression::append(DIExpr, Operands);
     }
@@ -2493,22 +2507,24 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
       Operands.push_back(SizeInBits);
     }
     llvm::DIExpression *DIExpr = DBuilder.createExpression(Operands);
-    emitDbgIntrinsic(Builder, Piece, Var,
-                     // DW_OP_LLVM_fragment must be the last part of an DIExpr
-                     // so we can't append more if IsPiece is true.
-                     IsPiece ? DIExpr : appendDIExpression(DIExpr), DInstLine,
-                     DInstLoc.column, Scope, DS,
-                     Indirection == CoroDirectValue ||
-                         Indirection == CoroIndirectValue);
+    // DW_OP_LLVM_fragment must be the last part of an DIExpr
+    // so we can't append more if IsPiece is true.
+    if (!IsPiece)
+      DIExpr = appendDIExpression(DIExpr);
+    if (DIExpr)
+      emitDbgIntrinsic(
+          Builder, Piece, Var, DIExpr, DInstLine, DInstLoc.column, Scope, DS,
+          Indirection == CoroDirectValue || Indirection == CoroIndirectValue);
   }
 
   // Emit locationless intrinsic for variables that were optimized away.
-  if (Storage.empty())
-    emitDbgIntrinsic(Builder, llvm::ConstantInt::get(IGM.Int64Ty, 0), Var,
-                     appendDIExpression(DBuilder.createExpression()), DInstLine,
-                     DInstLoc.column, Scope, DS,
-                     Indirection == CoroDirectValue ||
-                         Indirection == CoroIndirectValue);
+  if (Storage.empty()) {
+    if (auto *DIExpr = appendDIExpression(DBuilder.createExpression()))
+      emitDbgIntrinsic(Builder, llvm::ConstantInt::get(IGM.Int64Ty, 0), Var,
+                       DIExpr, DInstLine, DInstLoc.column, Scope, DS,
+                       Indirection == CoroDirectValue ||
+                           Indirection == CoroIndirectValue);
+  }
 }
 
 void IRGenDebugInfoImpl::emitDbgIntrinsic(
@@ -2554,7 +2570,19 @@ void IRGenDebugInfoImpl::emitDbgIntrinsic(
       DBuilder.insertDeclare(Storage, Var, Expr, DL, &EntryBlock);
   } else {
     // Insert a dbg.value at the current insertion point.
-    DBuilder.insertDbgValueIntrinsic(Storage, Var, Expr, DL, BB);
+    if (isa<llvm::Argument>(Storage) && !Var->getArg() &&
+        BB->getFirstNonPHIOrDbg())
+      // SelectionDAGISel only generates debug info for a dbg.value
+      // that is associated with a llvm::Argument if either its !DIVariable
+      // is marked as argument or there is no non-debug intrinsic instruction
+      // before it. So In the case of associating a llvm::Argument with a
+      // non-argument debug variable -- usually via a !DIExpression -- we
+      // need to make sure that dbg.value is before any non-phi / no-dbg
+      // instruction.
+      DBuilder.insertDbgValueIntrinsic(Storage, Var, Expr, DL,
+                                       BB->getFirstNonPHIOrDbg());
+    else
+      DBuilder.insertDbgValueIntrinsic(Storage, Var, Expr, DL, BB);
   }
 }
 

lib/IRGen/IRGenSIL.cpp

@@ -4752,8 +4752,7 @@ void IRGenSILFunction::visitDebugValueInst(DebugValueInst *i) {
   if (VarDecl *Decl = i->getDecl()) {
     DbgTy = DebugTypeInfo::getLocalVariable(
         Decl, RealTy, getTypeInfo(SILVal->getType()));
-  } else if (i->getFunction()->isBare() && !SILTy.hasArchetype() &&
-             !VarInfo->Name.empty()) {
+  } else if (!SILTy.hasArchetype() && !VarInfo->Name.empty()) {
     // Preliminary support for .sil debug information.
     DbgTy = DebugTypeInfo::getFromTypeInfo(RealTy, getTypeInfo(SILTy));
   } else

lib/SIL/IR/SILBasicBlock.cpp

@@ -292,8 +292,15 @@ transferNodesFromList(llvm::ilist_traits<SILBasicBlock> &SrcTraits,
     First->Parent = Parent;
     First->index = -1;
     First->lastInitializedBitfieldID = 0;
-    for (auto &II : *First)
+    for (auto &II : *First) {
       II.setDebugScope(ScopeCloner.getOrCreateClonedScope(II.getDebugScope()));
+      // Special handling for SILDebugVariable.
+      if (auto DVI = DebugVarCarryingInst(&II))
+        if (auto VarInfo = DVI.getVarInfo())
+          if (VarInfo->Scope)
+            DVI.setDebugVarScope(
+                ScopeCloner.getOrCreateClonedScope(VarInfo->Scope));
+    }
   }
 }