Debug Info: Fix the lowering of the SILDebugScope tree to the LLVM

inlined-at chain.

The previous implementation was only correct for cases where the inliner
inlined bottom-up in the call graph, which happened to cover the majority
of all cases.

rdar://problem/24462475

Author: adrian-prantl

include/swift/SIL/SILDebugScope.h

@@ -44,8 +44,8 @@ class SILDebugScope : public SILAllocated<SILDebugScope> {
   /// holds only the location of the call site. The parent scope will be
   /// the scope of the inlined call site.
   ///
-  /// Note that compared to the inlinedAt field in llvm::DILocation
-  /// the inlined call site chain in SILDebugScope uses the reversed order.
+  /// Note that compared to the inlinedAt chain in llvm::DILocation
+  /// SILDebugScope represents an inline tree.
   const SILDebugScope *InlinedCallSite;
 
   SILDebugScope(SILLocation Loc, SILFunction &SILFn,
@@ -70,6 +70,9 @@ class SILDebugScope : public SILAllocated<SILDebugScope> {
     assert(CallSiteScope && CalleeScope);
     assert(CalleeScope->getParentFunction()->isInlined() &&
            "function of inlined debug scope is not inlined");
+    if (InlinedCallSite)
+      assert(!InlinedCallSite->InlinedCallSite &&
+             "a call site scope cannot have an inlined call site");
   }
 
   /// Return the function this scope originated from before being inlined.
@@ -84,17 +87,36 @@ class SILDebugScope : public SILAllocated<SILDebugScope> {
     return Scope->Parent.get<SILFunction *>();
   }
 
+  /// Return this scope without inline information.
+  const SILDebugScope *getInlinedScope() const {
+    return InlinedCallSite ? Parent.get<const SILDebugScope*>() : this;
+  }
+  
   /// Return the parent function of this scope. If the scope was
   /// inlined this recursively returns the function it was inlined
   /// into.
   SILFunction *getParentFunction() const {
     if (InlinedCallSite)
-      return InlinedCallSite->getParentFunction();
+      return InlinedCallSite->Parent.get<const SILDebugScope *>()
+          ->getParentFunction();
     if (auto *ParentScope = Parent.dyn_cast<const SILDebugScope *>())
       return ParentScope->getParentFunction();
     return Parent.get<SILFunction *>();
   }
 
+  typedef SmallVector<const SILDebugScope *, 8> InlineScopeList;
+  static void flatten(const SILDebugScope *DS, InlineScopeList &List);
+
+  // Return a flattened representation of the inline scope tree that
+  // is equivalent to the reversed inlined-at chain.
+  InlineScopeList flattenedInlineTree() const {
+    InlineScopeList List;
+    flatten(this, List);
+    return List;
+  }
+
+  void dump(SourceManager &SM, llvm::raw_ostream &OS = llvm::errs(),
+            unsigned Indent = 0) const;
 };
 
 #ifndef NDEBUG

lib/IRGen/IRGenDebugInfo.cpp

@@ -284,30 +284,22 @@ static bool isAbstractClosure(const SILLocation &Loc) {
   return false;
 }
 
-/// Construct an inlined-at location from a SILScope.
-llvm::MDNode *
-IRGenDebugInfo::createInlinedAt(const SILDebugScope *CallSite) {
-  if (!CallSite)
-    return nullptr;
-
-  // In SIL the inlined-at information is part of the scopes, in LLVM
-  // IR it is part of the location. Transforming the inlined-at SIL scope
-  // to a location means skipping the inlined-at scope.
-  llvm::DIScope * ParentScope;
-  if (auto *Parent = CallSite->Parent.dyn_cast<const SILDebugScope *>())
-    ParentScope = getOrCreateScope(Parent);
-  else
-    ParentScope = getOrCreateScope(CallSite);
-
+/// Construct an LLVM inlined-at location from a SILDebugScope,
+/// reversing the order in the process.
+llvm::MDNode *IRGenDebugInfo::createInlinedAt(const SILDebugScope *DS) {
   llvm::MDNode *InlinedAt = nullptr;
-
-  // If this is itself an inlined location, recursively create the
-  // inlined-at location for it.
-  InlinedAt = createInlinedAt(CallSite->InlinedCallSite);
-
-  auto InlineLoc = getLoc(SM, CallSite->Loc.getDebugSourceLoc());
-  return llvm::DebugLoc::get(InlineLoc.Line, InlineLoc.Col, ParentScope,
-                             InlinedAt);
+  if (DS) {
+    for (auto *CS : DS->flattenedInlineTree()) {
+      // In SIL the inlined-at information is part of the scopes, in
+      // LLVM IR it is part of the location. Transforming the inlined-at
+      // SIL scope to a location means skipping the inlined-at scope.
+      auto *Parent = CS->Parent.get<const SILDebugScope *>();
+      auto *ParentScope = getOrCreateScope(Parent);
+      auto L = getLoc(SM, CS->Loc.getDebugSourceLoc());
+      InlinedAt = llvm::DebugLoc::get(L.Line, L.Col, ParentScope, InlinedAt);
+    }
+  }
+  return InlinedAt;
 }
 
 #ifndef NDEBUG
@@ -339,7 +331,9 @@ bool IRGenDebugInfo::lineNumberIsSane(IRBuilder &Builder, unsigned Line) {
 void IRGenDebugInfo::setCurrentLoc(IRBuilder &Builder, const SILDebugScope *DS,
                                    Optional<SILLocation> Loc) {
   assert(DS && "empty scope");
-  auto *Scope = getOrCreateScope(DS);
+  // Inline info is emitted as part of the location below; extract the
+  // original scope here.
+  auto *Scope = getOrCreateScope(DS->getInlinedScope());
   if (!Scope)
     return;
 
@@ -380,7 +374,7 @@ void IRGenDebugInfo::setCurrentLoc(IRBuilder &Builder, const SILDebugScope *DS,
   LastDebugLoc = L;
   LastScope = DS;
 
-  auto *InlinedAt = createInlinedAt(DS->InlinedCallSite);
+  auto *InlinedAt = createInlinedAt(DS);
   assert(((!InlinedAt) || (InlinedAt && Scope)) && "inlined w/o scope");
   assert(parentScopesAreSane(DS) && "parent scope sanity check failed");
   auto DL = llvm::DebugLoc::get(L.Line, L.Col, Scope, InlinedAt);
@@ -1082,8 +1076,9 @@ void IRGenDebugInfo::emitDbgIntrinsic(llvm::BasicBlock *BB,
                                       unsigned Col, llvm::DILocalScope *Scope,
                                       const SILDebugScope *DS) {
   // Set the location/scope of the intrinsic.
-  auto *InlinedAt = createInlinedAt(DS->InlinedCallSite);
+  auto *InlinedAt = createInlinedAt(DS);
   auto DL = llvm::DebugLoc::get(Line, Col, Scope, InlinedAt);
+
   // An alloca may only be described by exactly one dbg.declare.
   if (isa<llvm::AllocaInst>(Storage) && llvm::FindAllocaDbgDeclare(Storage))
     return;

lib/SIL/SILPrinter.cpp

@@ -611,15 +611,16 @@ class SILPrinter : public SILVisitor<SILPrinter> {
     }
 
     // Print inlined-at location, if any.
-    while (DS && DS->InlinedCallSite) {
-      *this << ": perf_inlined_at ";
-      auto CallSite = DS->InlinedCallSite->Loc;
-      if (!CallSite.isNull())
-        CallSite.getSourceLoc().print(
+    if (DS) {
+      for (auto *CS : reversed(DS->flattenedInlineTree())) {
+        *this << ": perf_inlined_at ";
+        auto CallSite = CS->Loc;//DS->InlinedCallSite->Loc;
+        if (!CallSite.isNull())
+          CallSite.getSourceLoc().print(
             PrintState.OS, M.getASTContext().SourceMgr, LastBufferID);
-      else
-        *this << "?";
-      DS = DS->Parent.dyn_cast<const SILDebugScope *>();
+        else
+          *this << "?";
+      }
     }
   }
 
@@ -1999,3 +2000,41 @@ void SILCoverageMap::print(llvm::raw_ostream &OS, bool ShouldSort,
 void SILCoverageMap::dump() const {
   print(llvm::errs());
 }
+
+void SILDebugScope::flatten(const SILDebugScope *DS,
+                            SILDebugScope::InlineScopeList &List) {
+  if (DS) {
+    if (auto *CS = DS->InlinedCallSite) {
+      flatten(CS->Parent.dyn_cast<const SILDebugScope *>(), List);
+      List.push_back(CS);
+    }
+    flatten(DS->Parent.dyn_cast<const SILDebugScope *>(), List);
+  }
+}
+
+void SILDebugScope::dump(SourceManager &SM, llvm::raw_ostream &OS,
+                         unsigned Indent) const {
+  OS << "{\n";
+  OS.indent(Indent);
+  Loc.getSourceLoc().print(OS, SM);
+  OS << "\n";
+  OS.indent(Indent + 2);
+  OS << " parent: ";
+  if (auto *P = Parent.dyn_cast<const SILDebugScope *>()) {
+    P->dump(SM, OS, Indent + 2);
+    OS.indent(Indent + 2);
+  }
+  else if (auto *F = Parent.dyn_cast<SILFunction *>())
+    OS << "@" << F->getName();
+  else
+    OS << "nullptr";
+
+  OS << "\n";
+  OS.indent(Indent + 2);
+  if (auto *CS = InlinedCallSite) {
+    OS << "inlinedCallSite: ";
+    CS->dump(SM, OS, Indent + 2);
+    OS.indent(Indent + 2);
+  }
+  OS << "}\n";
+}

lib/SILOptimizer/Utils/SILInliner.cpp

@@ -37,6 +37,9 @@ bool SILInliner::inlineFunction(FullApplySite AI, ArrayRef<SILValue> Args) {
     return false;
 
   SILFunction &F = getBuilder().getFunction();
+  if (CalleeFunction->getName() == "_TTSg5Vs4Int8___TFVs12_ArrayBufferg9_isNativeSb"
+      && F.getName() == "_TTSg5Vs4Int8___TFVs12_ArrayBufferg8endIndexSi")
+    llvm::errs();
 
   assert(AI.getFunction() && AI.getFunction() == &F &&
          "Inliner called on apply instruction in wrong function?");
@@ -72,7 +75,7 @@ bool SILInliner::inlineFunction(FullApplySite AI, ArrayRef<SILValue> Args) {
     // Performance inlining. Construct a proper inline scope pointing
     // back to the call site.
     CallSiteScope = new (F.getModule())
-      SILDebugScope(AI.getLoc(), F, AIScope, AIScope->InlinedCallSite);
+      SILDebugScope(AI.getLoc(), F, AIScope);
     assert(CallSiteScope->getParentFunction() == &F);
   }
   assert(CallSiteScope && "call site has no scope");

test/DebugInfo/inlinedAt.swift

@@ -0,0 +1,48 @@
+// RUN: %target-swift-frontend %s -O -I %t -emit-sil -emit-verbose-sil -o - \
+// RUN:    | FileCheck %s --check-prefix=CHECK-SIL
+// RUN: %target-swift-frontend %s -O -I %t -emit-ir -g -o - | FileCheck %s
+
+#line 100 "abc.swift"
+@inline(__always)
+func h(k : Int) -> Int {        // 101
+  return k                      // 102
+}
+
+#line 200 "abc.swift"
+@inline(__always)
+func g(j : Int) -> Int {        // 201
+  return h(j)                   // 202
+}
+
+#line 301 "abc.swift"
+public func f(i : Int) -> Int { // 301
+  return g(i)                   // 302
+}
+
+// CHECK-SIL: sil {{.*}}@_TF9inlinedAt1fFSiSi :
+// CHECK-SIL-NOT: return
+// CHECK-SIL: debug_value %0 : $Int, let, name "k", argno 1
+// CHECK-SIL-SAME: line:101:8:in_prologue
+// CHECK-SIL-SAME: perf_inlined_at line:202:10
+// CHECK-SIL-SAME: perf_inlined_at line:302:10
+
+// CHECK: define {{.*}}@_TF9inlinedAt1fFSiSi(i64)
+// CHECK-NOT: ret
+// CHECK: @llvm.dbg.value
+// CHECK: @llvm.dbg.value
+// CHECK: @llvm.dbg.value({{.*}}), !dbg ![[L1:.*]]
+
+// CHECK: ![[F:.*]] = distinct !DISubprogram(name: "f",
+// CHECK: ![[G:.*]] = distinct !DISubprogram(name: "g",
+// CHECK: ![[H:.*]] = distinct !DISubprogram(name: "h",
+
+// CHECK: ![[L3:.*]] = !DILocation(line: 302, column: 13,
+// CHECK-SAME:                     scope: ![[F_SCOPE:.*]])
+// CHECK: ![[F_SCOPE]] = distinct !DILexicalBlock(scope: ![[F]],
+// CHECK-SAME:                                    line: 301, column: 31)
+// CHECK: ![[L1]] = !DILocation(line: 101, column: 8, scope: ![[H]],
+// CHECK-SAME:                  inlinedAt: ![[L2:.*]])
+// CHECK: ![[L2]] = !DILocation(line: 202, column: 13, scope: ![[G_SCOPE:.*]],
+// CHECK-SAME:                  inlinedAt: ![[L3]])
+// CHECK: ![[G_SCOPE]] = distinct !DILexicalBlock(scope: ![[G]],
+// CHECK-SAME:                                    line: 201, column: 24)