Merge pull request #73009 from Snowy1803/debuginfo-small-fixes

[Debuginfo] Small fixes

Author: adrian-prantl

include/swift/SIL/SILCloner.h

@@ -338,14 +338,16 @@ class SILCloner : protected SILInstructionVisitor<ImplClass> {
     registerLocalArchetypeRemapping(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)
+  /// 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 remapDebugVariable(std::optional<SILDebugVariable> &VarInfo) {
+    if (!VarInfo)
       return;
-    auto VarInfo = DbgVarInst.getVarInfo();
-    if (VarInfo && VarInfo->Scope)
-      DbgVarInst.setDebugVarScope(getOpScope(VarInfo->Scope));
+    if (VarInfo->Type)
+      VarInfo->Type = getOpType(*VarInfo->Type);
+    // Don't remap locations for debug values.
+    if (VarInfo->Scope)
+      VarInfo->Scope = getOpScope(VarInfo->Scope);
   }
 
   ProtocolConformanceRef getOpConformance(Type ty,
@@ -879,8 +881,7 @@ SILCloner<ImplClass>::visitAllocStackInst(AllocStackInst *Inst) {
     Loc = MandatoryInlinedLocation::getAutoGeneratedLocation();
     VarInfo = std::nullopt;
   }
-  if (VarInfo && VarInfo->Type)
-    VarInfo->Type = getOpType(*VarInfo->Type);
+  remapDebugVariable(VarInfo);
   auto *NewInst = getBuilder().createAllocStack(
       Loc, getOpType(Inst->getElementType()), VarInfo,
       Inst->hasDynamicLifetime(), Inst->isLexical(), Inst->isFromVarDecl(),
@@ -890,7 +891,6 @@ SILCloner<ImplClass>::visitAllocStackInst(AllocStackInst *Inst) {
       true
 #endif
   );
-  remapDebugVarInfo(DebugVarCarryingInst(NewInst));
   recordClonedInstruction(Inst, NewInst);
 }
 
@@ -1428,14 +1428,12 @@ 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();
+  std::optional<SILDebugVariable> VarInfo = Inst->getVarInfo();
   getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
-  if (VarInfo.Type)
-    VarInfo.Type = getOpType(*VarInfo.Type);
+  remapDebugVariable(VarInfo);
   auto *NewInst = getBuilder().createDebugValue(
-      Inst->getLoc(), getOpValue(Inst->getOperand()), VarInfo,
+      Inst->getLoc(), getOpValue(Inst->getOperand()), *VarInfo,
       Inst->poisonRefs(), Inst->usesMoveableValueDebugInfo(), Inst->hasTrace());
-  remapDebugVarInfo(DebugVarCarryingInst(NewInst));
   recordClonedInstruction(Inst, NewInst);
 }
 template<typename ImplClass>

lib/IRGen/IRGenDebugInfo.cpp

@@ -3180,7 +3180,7 @@ void IRGenDebugInfoImpl::emitVariableDeclaration(
 
   // Create the descriptor for the variable.
   unsigned DVarLine = DInstLine;
-  uint16_t DVarCol = 0;
+  uint16_t DVarCol = DInstLoc.Column;
   if (VarInfo.Loc) {
     auto DVarLoc = getStartLocation(VarInfo.Loc);
     DVarLine = DVarLoc.Line;

lib/SIL/IR/SILPrinter.cpp

@@ -1412,7 +1412,7 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
       else
         *this << ", var";
 
-      if ((Var->Loc || Var->Scope) && SM) {
+      if ((Var->Loc || Var->Scope) && SM && Ctx.printDebugInfo()) {
         *this << ", (name \"" << Var->Name << '"';
         if (Var->Loc)
           printDebugLocRef(*Var->Loc, *SM);

lib/SILOptimizer/SILCombiner/SILCombine.cpp

@@ -615,6 +615,7 @@ void SwiftPassInvocation::eraseInstruction(SILInstruction *inst) {
   if (silCombiner) {
     silCombiner->eraseInstFromFunction(*inst);
   } else {
+    swift::salvageDebugInfo(inst);
     if (inst->isStaticInitializerInst()) {
       inst->getParent()->erase(inst, *getPassManager()->getModule());
     } else {

lib/SILOptimizer/Transforms/SILSROA.cpp

@@ -298,7 +298,7 @@ void SROAMemoryUseAnalyzer::chopUpAlloca(std::vector<AllocStackInst *> &Worklist
     SILInstruction *User = Operand->getUser();
     auto *DVI = dyn_cast<DebugValueInst>(User);
     assert(DVI && "getDebugUses should only return DebugValueInst");
-    SILBuilderWithScope B(DVI);
+    SILBuilder B(DVI, DVI->getDebugScope());
     std::optional<SILDebugVariable> DVIVarInfo = DVI->getVarInfo();
     assert(DVIVarInfo && "debug_value without debug info");
 

test/DebugInfo/dead-store-elimination.sil

@@ -0,0 +1,26 @@
+// RUN: %target-sil-opt %s -dead-store-elimination -enable-sil-verify-all | %FileCheck %s
+
+// REQUIRES: swift_in_compiler
+
+sil_stage canonical
+
+import Swift
+import Builtin
+
+// We should be able to replace the local store with a debug_value.
+// The dead store elimination pass is written in Swift, and should call
+// salvage debug info when removing the store.
+
+// CHECK-LABEL: @trivial_local_dead_store
+// CHECK: debug_value %{{[0-9]+}} : $Int, var, name "a"
+// CHECK: return
+sil hidden @trivial_local_dead_store : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_stack $Int, var, name "a"
+  %1 = integer_literal $Builtin.Int64, 1
+  %2 = struct $Int (%1 : $Builtin.Int64)
+  store %2 to %0 : $*Int
+  %4 = tuple ()
+  dealloc_stack %0 : $*Int
+  return %4 : $()
+}

test/DebugInfo/debug_variable_varinfo_loc.sil

@@ -0,0 +1,35 @@
+// RUN: %target-swiftc_driver -g -emit-ir %s | %FileCheck %s
+sil_stage canonical
+
+import Builtin
+import Swift
+
+sil_scope 2 { loc "simple.swift":1:2 parent @test_debug_value : $@convention(thin) (Builtin.Int64) -> () }
+
+// CHECK-LABEL: @test_debug_value
+// CHECK-SAME: !dbg ![[FUNC_DI:[0-9]+]]
+sil hidden @test_debug_value : $@convention(thin) (Builtin.Int64) -> () {
+bb0(%0 : $Builtin.Int64):
+  // CHECK:      @llvm.dbg.declare(metadata ptr
+  // CHECK-SAME:                   metadata ![[VAR_DI:[0-9]+]]
+  // CHECK-SAME:                   ), !dbg ![[LOC_DI:[0-9]+]]
+  debug_value %0 : $Builtin.Int64, var, name "x", loc "simple.swift":1:16, scope 2
+  %zero = integer_literal $Builtin.Int64, 0
+  debug_value %zero : $Builtin.Int64, var, (name "x", loc "simple.swift":1:16, scope 2), loc "simple.swift":3:4, scope 2
+  %1 = tuple ()
+  return %1 : $()
+}
+
+// One debug_value uses the VarInfo way, the other uses the instruction way.
+// They should both resolve to the same variable, so we should only have one x.
+
+// CHECK: !DISubprogram(name: "test_debug_value"
+// CHECK-SAME: retainedNodes: ![[RETAINED:[0-9]+]]
+
+// Only one variable should be retained
+// CHECK: ![[RETAINED]] = !{![[VAR_DI]]}
+
+// CHECK-NOT: !DILocalVariable(name: "x"
+// CHECK: ![[VAR_DI]] = !DILocalVariable(name: "x"
+// CHECK-SAME:                           line: 1
+// CHECK-NOT: !DILocalVariable(name: "x"