Merge pull request #77655 from felipepiovezan/felipe/fix_code_gen_debug_info

[DebugInfo] Change lowering of SIL instructions to use line 0 when appropriate

Author: felipepiovezan

lib/IRGen/IRGenDebugInfo.cpp

@@ -344,6 +344,15 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
   IRGenDebugInfoFormat getDebugInfoFormat() { return Opts.DebugInfoFormat; }
 
 private:
+  /// Convert a SILLocation into the corresponding LLVM Loc.
+  FileAndLocation computeLLVMLoc(const SILDebugScope *DS, SILLocation Loc);
+
+  /// Compute the LLVM DebugLoc when targeting CodeView. In CodeView, zero is
+  /// not an artificial line location; attempt to avoid those line locations near
+  /// user code to reduce the number of breaks in the linetables.
+  FileAndLocation computeLLVMLocCodeView(const SILDebugScope *DS,
+                                         SILLocation Loc);
+
   static StringRef getFilenameFromDC(const DeclContext *DC) {
     if (auto *LF = dyn_cast<LoadedFile>(DC))
       return LF->getFilename();
@@ -2614,6 +2623,40 @@ bool IRGenDebugInfoImpl::lineEntryIsSane(FileAndLocation DL,
 }
 #endif
 
+IRGenDebugInfoImpl::FileAndLocation
+IRGenDebugInfoImpl::computeLLVMLocCodeView(const SILDebugScope *DS,
+                                           SILLocation Loc) {
+  // If the scope has not changed and the line number is either zero or
+  // artificial, we want to keep the most recent debug location.
+  if (DS == LastScope && (Loc.is<ArtificialUnreachableLocation>() ||
+                          Loc.isLineZero(SM) || Loc.isHiddenFromDebugInfo()))
+    return LastFileAndLocation;
+
+  // Decode the location.
+  return decodeFileAndLocation(Loc);
+}
+
+IRGenDebugInfoImpl::FileAndLocation
+IRGenDebugInfoImpl::computeLLVMLoc(const SILDebugScope *DS, SILLocation Loc) {
+  SILFunction *Fn = DS->getInlinedFunction();
+  if (Fn && (Fn->isThunk() || Fn->isTransparent()))
+    return {0, 0, CompilerGeneratedFile};
+
+  if (Opts.DebugInfoFormat == IRGenDebugInfoFormat::CodeView)
+    return computeLLVMLocCodeView(DS, Loc);
+
+  FileAndLocation L =
+      Loc.isInPrologue() ? FileAndLocation() : decodeFileAndLocation(Loc);
+
+  // Otherwise use a line 0 artificial location, but the file from the location.
+  if (Loc.isHiddenFromDebugInfo()) {
+    L.Line = 0;
+    L.Column = 0;
+  }
+
+  return L;
+}
+
 void IRGenDebugInfoImpl::setCurrentLoc(IRBuilder &Builder,
                                        const SILDebugScope *DS,
                                        SILLocation Loc) {
@@ -2622,38 +2665,7 @@ void IRGenDebugInfoImpl::setCurrentLoc(IRBuilder &Builder,
   if (!Scope)
     return;
 
-  // NOTE: In CodeView, zero is not an artificial line location. We try to
-  //       avoid those line locations near user code to reduce the number
-  //       of breaks in the linetables.
-  FileAndLocation L;
-  SILFunction *Fn = DS->getInlinedFunction();
-  if (Fn && (Fn->isThunk() || Fn->isTransparent())) {
-    L = {0, 0, CompilerGeneratedFile};
-  } else if (DS == LastScope && Loc.isHiddenFromDebugInfo()) {
-    // Reuse the last source location if we are still in the same
-    // scope to get a more contiguous line table.
-    L = LastFileAndLocation;
-  } else if (DS == LastScope &&
-             (Loc.is<ArtificialUnreachableLocation>() || Loc.isLineZero(SM)) &&
-             Opts.DebugInfoFormat == IRGenDebugInfoFormat::CodeView) {
-    // If the scope has not changed and the line number is either zero or
-    // artificial, we want to keep the most recent debug location.
-    L = LastFileAndLocation;
-  } else {
-    // Decode the location.
-    if (!Loc.isInPrologue() ||
-        Opts.DebugInfoFormat == IRGenDebugInfoFormat::CodeView)
-      L = decodeFileAndLocation(Loc);
-
-    // Otherwise use a line 0 artificial location, but the file from the
-    // location. If we are emitting CodeView, we do not want to use line zero
-    // since it does not represent an artificial line location.
-    if (Loc.isHiddenFromDebugInfo() &&
-        Opts.DebugInfoFormat != IRGenDebugInfoFormat::CodeView) {
-      L.Line = 0;
-      L.Column = 0;
-    }
-  }
+  FileAndLocation L = computeLLVMLoc(DS, Loc);
 
   if (L.getFilename() != Scope->getFilename()) {
     // We changed files in the middle of a scope. This happens, for

test/Backtracing/Crash.swift

@@ -60,7 +60,7 @@ struct Crash {
 // CHECK-NEXT: 4 [ra]          0x{{[0-9a-f]+}} level1() + {{[0-9]+}} in Crash at {{.*}}/Crash.swift:24:3
 // CHECK-NEXT: 5 [ra]          0x{{[0-9a-f]+}} static Crash.main() + {{[0-9]+}} in Crash at {{.*}}/Crash.swift:48:5
 // CHECK-NEXT: 6 [ra] [system] 0x{{[0-9a-f]+}} static Crash.$main() + {{[0-9]+}} in Crash at {{.*}}/<compiler-generated>
-// CHECK-NEXT: 7 [ra] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in Crash at {{.*}}/Crash.swift
+// CHECK-NEXT: 7 [ra] [system] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in Crash at {{.*}}/Crash.swift
 
 // CHECK: Registers:
 

test/Backtracing/CrashStatic.swift

@@ -51,7 +51,7 @@ struct CrashStatic {
 // CHECK-NEXT: 4 [ra]          0x{{[0-9a-f]+}} level1() + {{[0-9]+}} in CrashStatic at {{.*}}/CrashStatic.swift:15:3
 // CHECK-NEXT: 5 [ra]          0x{{[0-9a-f]+}} static CrashStatic.main() + {{[0-9]+}} in CrashStatic at {{.*}}/CrashStatic.swift:39:5
 // CHECK-NEXT: 6 [ra] [system] 0x{{[0-9a-f]+}} static CrashStatic.$main() + {{[0-9]+}} in CrashStatic at {{.*}}/<compiler-generated>
-// CHECK-NEXT: 7 [ra] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in CrashStatic at {{.*}}/CrashStatic.swift
+// CHECK-NEXT: 7 [ra] [system] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in CrashStatic at {{.*}}/CrashStatic.swift
 
 // CHECK: Registers:
 

test/Backtracing/CrashWithThunk.swift

@@ -38,7 +38,7 @@ struct CrashWithThunk {
 // CHECK-NEXT: 1 [ra] [thunk]  0x{{[0-9a-f]+}} thunk for @escaping @callee_guaranteed () -> () + {{[0-9]+}} in CrashWithThunk at {{.*}}/<compiler-generated>
 // CHECK-NEXT: 2 [ra]          0x{{[0-9a-f]+}} static CrashWithThunk.main() + {{[0-9]+}} in CrashWithThunk at {{.*}}/CrashWithThunk.swift:29:9
 // CHECK-NEXT: 3 [ra] [system] 0x{{[0-9a-f]+}} static CrashWithThunk.$main() + {{[0-9]+}} in CrashWithThunk at {{.*}}/<compiler-generated>
-// CHECK-NEXT: 4 [ra] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in CrashWithThunk at {{.*}}/CrashWithThunk.swift
+// CHECK-NEXT: 4 [ra] [system] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in CrashWithThunk at {{.*}}/CrashWithThunk.swift
 
 // CHECK: Registers:
 

test/Backtracing/FatalError.swift

@@ -51,4 +51,4 @@ struct FatalError {
 // CHECK-NEXT: 5 [ra]          0x{{[0-9a-f]+}} level1() + {{[0-9]+}} in FatalError at {{.*}}/FatalError.swift:14:3
 // CHECK-NEXT: 6 [ra]          0x{{[0-9a-f]+}} static FatalError.main() + {{[0-9]+}} in FatalError at {{.*}}/FatalError.swift:36:5
 // CHECK-NEXT: 7 [ra] [system] 0x{{[0-9a-f]+}} static FatalError.$main() + {{[0-9]+}} in FatalError at {{.*}}/<compiler-generated>
-// CHECK-NEXT: 8 [ra] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in FatalError at {{.*}}/FatalError.swift
+// CHECK-NEXT: 8 [ra] [system] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in FatalError at {{.*}}/FatalError.swift

test/Backtracing/Overflow.swift

@@ -52,7 +52,7 @@ struct Overflow {
 // CHECK-NEXT: 5 [ra]               0x{{[0-9a-f]+}} level1() + {{[0-9]+}} in Overflow at {{.*}}/Overflow.swift:15:3
 // CHECK-NEXT: 6 [ra]               0x{{[0-9a-f]+}} static Overflow.main() + {{[0-9]+}} in Overflow at {{.*}}/Overflow.swift:39:5
 // CHECK-NEXT: 7 [ra] [system]      0x{{[0-9a-f]+}} static Overflow.$main() + {{[0-9]+}} in Overflow at {{.*}}/<compiler-generated>
-// CHECK-NEXT: 8 [ra]               0x{{[0-9a-f]+}} main + {{[0-9]+}} in Overflow at {{.*}}/Overflow.swift
+// CHECK-NEXT: 8 [ra] [system]      0x{{[0-9a-f]+}} main + {{[0-9]+}} in Overflow at {{.*}}/Overflow.swift
 
 // CHECK: Registers:
 

test/Backtracing/StackOverflow.swift

@@ -100,7 +100,7 @@ struct StackOverflow {
 
 // CHECK: {{[0-9]+}} [ra]          0x{{[0-9a-f]+}} static StackOverflow.main() + {{[0-9]+}} in StackOverflow at {{.*}}/StackOverflow.swift:25:5
 // CHECK-NEXT: {{[0-9]+}} [ra] [system] 0x{{[0-9a-f]+}} static StackOverflow.$main() + {{[0-9]+}} in StackOverflow at {{.*}}/<compiler-generated>
-// CHECK-NEXT: {{[0-9]+}} [ra]          0x{{[0-9a-f]+}} main + {{[0-9]+}} in StackOverflow at {{.*}}/StackOverflow.swift
+// CHECK-NEXT: {{[0-9]+}} [ra] [system] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in StackOverflow at {{.*}}/StackOverflow.swift
 
 // CHECK: Registers:
 
@@ -129,7 +129,7 @@ struct StackOverflow {
 
 // LIMITED: {{[0-9]+}} [ra]          0x{{[0-9a-f]+}} static StackOverflow.main() + {{[0-9]+}} in StackOverflow at {{.*}}/StackOverflow.swift:25:5
 // LIMITED-NEXT: {{[0-9]+}} [ra] [system] 0x{{[0-9a-f]+}} static StackOverflow.$main() + {{[0-9]+}} in StackOverflow at {{.*}}/<compiler-generated>
-// LIMITED-NEXT: {{[0-9]+}} [ra]          0x{{[0-9a-f]+}} main + {{[0-9]+}} in StackOverflow at {{.*}}/StackOverflow.swift
+// LIMITED-NEXT: {{[0-9]+}} [ra] [system] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in StackOverflow at {{.*}}/StackOverflow.swift
 
 // FRIENDLY: *** Program crashed: Bad pointer dereference at 0x{{[0-9a-f]+}} ***
 

test/Backtracing/StaticBacktracer.swift

@@ -52,7 +52,7 @@ struct StaticBacktracer {
 // CHECK-NEXT: 4 [ra]          0x{{[0-9a-f]+}} level1() + {{[0-9]+}} in StaticBacktracer at {{.*}}/StaticBacktracer.swift:16:3
 // CHECK-NEXT: 5 [ra]          0x{{[0-9a-f]+}} static StaticBacktracer.main() + {{[0-9]+}} in StaticBacktracer at {{.*}}/StaticBacktracer.swift:40:5
 // CHECK-NEXT: 6 [ra] [system] 0x{{[0-9a-f]+}} static StaticBacktracer.$main() + {{[0-9]+}} in StaticBacktracer at {{.*}}/<compiler-generated>
-// CHECK-NEXT: 7 [ra] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in StaticBacktracer at {{.*}}/StaticBacktracer.swift
+// CHECK-NEXT: 7 [ra] [system] 0x{{[0-9a-f]+}} main + {{[0-9]+}} in StaticBacktracer at {{.*}}/StaticBacktracer.swift
 
 // CHECK: Registers:
 

test/DebugInfo/async-let.swift

@@ -17,7 +17,7 @@ public actor Alice {
   // CHECK:   load ptr, ptr {{.*}} !dbg ![[LET_HOP0:[0-9]+]]
 
   // CHECK: define {{.*}}$s1M5AliceC4callyyYaFSiyYaYbcfu_TY2_{{.*}} !dbg ![[LET_SCOPE1:[0-9]+]]
-  // CHECK:   load ptr, ptr {{.*}} !dbg ![[LET_HOP1:[0-9]+]]
+  // CHECK:   store i64 %{{.*}} !dbg ![[LET_HOP1:[0-9]+]]
   public func call() async {
     // CHECK: ![[SCOPE0]] = distinct !DISubprogram({{.*}}line: [[@LINE-1]]
     // CHECK: ![[HOP0]] = !DILocation(line: [[@LINE+1]], column: 11

test/DebugInfo/autoclosure.swift

@@ -19,11 +19,10 @@ func &&&&&(lhs: Bool, rhs: @autoclosure () -> Bool) -> Bool {
 }
 
 func call_me(_ input: Int64) -> Void {
-// rdar://problem/14627460
 // An autoclosure should have a line number in the debug info and a scope line of 0.
 // CHECK-DAG: !DISubprogram({{.*}}linkageName: "$s11autoclosure7call_meyys5Int64VFSbyXEfu_",{{.*}} spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
-// But not in the line table.
-// CHECK-DAG: ![[DBG]] = !DILocation(line: [[@LINE+1]],
+// Instructions setting up the closure should have a line number of 0.
+// CHECK-DAG: ![[DBG]] = !DILocation(line: 0,
   if input != 0 &&&&& ( get_truth (input * 2 + 1) > 0 ) {
   }
 

test/DebugInfo/callexpr.swift

@@ -19,10 +19,8 @@ markUsed(r)
 struct MyType {}
 func bar(x: MyType = MyType()) {}
 
-// Room for improvement:
-// Because the default argument is implicit it inherits the previous source location.
 // CHECK2: call {{.*}}MyType{{.*}}, !dbg ![[DEFAULTARG:.*]]
 // CHECK2: call {{.*}}bar{{.*}}, !dbg ![[BARCALL:.*]]
-bar() // CHECK2: ![[DEFAULTARG]] = !DILocation(line: [[@LINE-9]]
+bar() // CHECK2: ![[DEFAULTARG]] = !DILocation(line: 0
       // CHECK2: ![[BARCALL]] = !DILocation(line: [[@LINE-1]], column: 1
 

test/DebugInfo/returnlocation.swift

@@ -184,9 +184,10 @@ public class Class1 {
   public required init?() {
     print("hello")
     // CHECK_INIT: call {{.*}}@"$ss5print_9separator10terminatoryypd_S2StF"{{.*}}, !dbg ![[PRINTLOC:[0-9]+]]
-    // FIXME: Why doesn't ret have the correct line number?
-    // CHECK_INIT: ret i{{32|64}} 0, !dbg ![[PRINTLOC]]
+    // FIXME: ret has an incorrect line number because it is generated with "isHiddenFromDebugInfo"
+    // CHECK_INIT: ret i{{32|64}} 0, !dbg ![[LINE_0:[0-9]+]]
     // CHECK_INIT-DAG: [[PRINTLOC]] = !DILocation(line: [[@LINE-4]]
+    // CHECK_INIT-DAG: [[LINE_0]] = !DILocation(line: 0
     return nil
   }
 }