Merge pull request #59549 from adrian-prantl/89776340

Emit debug locations for hop_to_executor instructions.

Author: adrian-prantl

include/swift/SIL/SILLocation.h

@@ -267,7 +267,7 @@ class SILLocation {
 
   SILLocation(ExtendedASTNodeLoc *ext, LocationKind K)
       : storage(ext), kindAndFlags(K, ExtendedASTNodeKind) {
-    assert(ext && !ext->primary.isNull() && !ext->forDebugging.isNull());
+    assert(ext && !ext->forDebugging.isNull());
   }
 
   SILLocation(SourceLoc L, LocationKind K)
@@ -299,9 +299,9 @@ class SILLocation {
   /// Artificial locations and the top-level module locations will be null.
   bool isNull() const {
     switch (getStorageKind()) {
-    case ASTNodeKind:             return storage.ASTNodeLoc.isNull();
-    case ExtendedASTNodeKind:     return false;
-    case FilenameAndLocationKind: return storage.filePositionLoc == nullptr;;
+    case ASTNodeKind:
+    case ExtendedASTNodeKind:     return !getPrimaryASTNode();
+    case FilenameAndLocationKind: return storage.filePositionLoc == nullptr;
     case SourceLocKind:           return storage.sourceLoc.isInvalid();
     }
     llvm_unreachable("covered switch");
@@ -344,12 +344,13 @@ class SILLocation {
 
   /// Returns true if the location represents an artificially generated
   /// body, such as thunks or default destructors.
-  ///
-  /// These locations should not be included in the debug line table.
-  /// These might also need special handling by the debugger since they might
-  /// contain calls, which the debugger could be able to step into.
   bool isAutoGenerated() const { return kindAndFlags.fields.autoGenerated; }
 
+  /// Returns false if the location should be represented in debuginfo.
+  bool isHiddenFromDebugInfo() const {
+    return isAutoGenerated() && !hasASTNodeForDebugging();
+  }
+
   /// Returns true if the line number of this location is zero.
   bool isLineZero(const SourceManager &SM) const {
     return decodeForDebugging(SM).line == 0;
@@ -454,6 +455,7 @@ class RegularLocation : public SILLocation {
   RegularLocation(Decl *D) : SILLocation(ASTNodeTy(D), RegularKind) {}
   RegularLocation(Pattern *P) : SILLocation(ASTNodeTy(P), RegularKind) {}
   RegularLocation(Stmt *S, Pattern *P, SILModule &Module);
+  RegularLocation(SILLocation ForDebuggingOnly, SILModule &Module);
   RegularLocation(SourceLoc L) : SILLocation(L, RegularKind) {}
   RegularLocation(FilenameAndLocation *filePos)
     : SILLocation(filePos, RegularKind) {}
@@ -485,12 +487,22 @@ class RegularLocation : public SILLocation {
     return AL;
   }
 
+  /// Returns a location that is empty for diagnostics, and L for the debug info
+  /// Used for \c hop_to_executor instructions.
+  static RegularLocation getDebugOnlyLocation(SILLocation L, SILModule &M) {
+    if (L.isASTNode())
+      return RegularLocation(L, M);
+    return getAutoGeneratedLocation(L);
+  }
+
   static bool isKind(const SILLocation& L) {
     return L.getKind() == RegularKind;
   }
 
 private:
   RegularLocation() : SILLocation(RegularKind) {}
+  static SILLocation::ExtendedASTNodeLoc *
+  getDebugOnlyExtendedASTNodeLoc(SILLocation L, SILModule &Module);
 };
 
 /// Used to represent a return instruction in user code.

lib/IRGen/IRGenDebugInfo.cpp

@@ -2049,7 +2049,7 @@ void IRGenDebugInfoImpl::setCurrentLoc(IRBuilder &Builder,
   SILFunction *Fn = DS->getInlinedFunction();
   if (Fn && (Fn->isThunk() || Fn->isTransparent())) {
     L = *SILLocation::getCompilerGeneratedLoc();
-  } else if (DS == LastScope && Loc.isAutoGenerated()) {
+  } 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 = LastFilenameAndLocation;
@@ -2068,7 +2068,7 @@ void IRGenDebugInfoImpl::setCurrentLoc(IRBuilder &Builder,
     // 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.isAutoGenerated() &&
+    if (Loc.isHiddenFromDebugInfo() &&
         Opts.DebugInfoFormat != IRGenDebugInfoFormat::CodeView) {
       L.line = 0;
       L.column = 0;

lib/SIL/IR/SILLocation.cpp

@@ -73,26 +73,15 @@ SourceLoc SILLocation::getSourceLoc(ASTNodeTy N) const {
 }
 
 SourceLoc SILLocation::getSourceLocForDebugging() const {
+  if (hasASTNodeForDebugging())
+    return getSourceLoc(storage.extendedASTNodeLoc->forDebugging);
+
   if (isNull())
     return SourceLoc();
 
   if (isSILFile())
     return storage.sourceLoc;
 
-  if (auto *expr = getPrimaryASTNode().dyn_cast<Expr*>()) {
-    // Code that has an autoclosure as location should not show up in
-    // the line table (rdar://problem/14627460). Note also that the
-    // closure function still has a valid DW_AT_decl_line.  Depending
-    // on how we decide to resolve rdar://problem/14627460, we may
-    // want to use the regular getLoc instead and rather use the
-    // column info.
-    if (isa<AutoClosureExpr>(expr))
-      return SourceLoc();
-  }
-
-  if (hasASTNodeForDebugging())
-    return getSourceLoc(storage.extendedASTNodeLoc->forDebugging);
-
   return getSourceLoc(getPrimaryASTNode());
 }
 
@@ -220,6 +209,26 @@ void SILLocation::print(raw_ostream &OS, const SourceManager &SM) const {
 RegularLocation::RegularLocation(Stmt *S, Pattern *P, SILModule &Module) :
   SILLocation(new (Module) ExtendedASTNodeLoc(S, P), RegularKind) {}
 
+SILLocation::ExtendedASTNodeLoc *
+RegularLocation::getDebugOnlyExtendedASTNodeLoc(SILLocation L,
+                                                SILModule &Module) {
+  if (auto D = L.getAsASTNode<Decl>())
+    return new (Module) ExtendedASTNodeLoc((Decl *)nullptr, D);
+  if (auto E = L.getAsASTNode<Expr>())
+    return new (Module) ExtendedASTNodeLoc((Decl *)nullptr, E);
+  if (auto S = L.getAsASTNode<Stmt>())
+    return new (Module) ExtendedASTNodeLoc((Decl *)nullptr, S);
+  auto P = L.getAsASTNode<Pattern>();
+  return new (Module) ExtendedASTNodeLoc((Decl *)nullptr, P);
+}
+
+RegularLocation::RegularLocation(SILLocation ForDebuggingOnly,
+                                 SILModule &Module)
+    : SILLocation(getDebugOnlyExtendedASTNodeLoc(ForDebuggingOnly, Module),
+                  RegularKind) {
+  markAutoGenerated();
+}
+
 ReturnLocation::ReturnLocation(ReturnStmt *RS) :
   SILLocation(ASTNodeTy(RS), ReturnKind) {}
 

lib/SILGen/SILGen.cpp

@@ -735,12 +735,21 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant,
     return emitted;
   }
 
+  auto getBestLocation = [](SILDeclRef ref) -> SILLocation {
+    if (ref.hasDecl())
+      return ref.getDecl();
+    if (ref.loc.isNull())
+      return {(Decl *)nullptr};
+    if (auto *ace = ref.getAbstractClosureExpr())
+      return {ace};
+    return {(Decl *)nullptr};
+  };
+
   // Note: Do not provide any SILLocation. You can set it afterwards.
   SILGenFunctionBuilder builder(*this);
   auto &IGM = *this;
   auto *F = builder.getOrCreateFunction(
-      constant.hasDecl() ? constant.getDecl() : (Decl *)nullptr, constant,
-      forDefinition,
+      getBestLocation(constant), constant, forDefinition,
       [&IGM](SILLocation loc, SILDeclRef constant) -> SILFunction * {
         return IGM.getFunction(constant, NotForDefinition);
       });

lib/SILGen/SILGenProlog.cpp

@@ -675,8 +675,9 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
     if (F.isAsync()) {
       // For an async function, hop to the executor.
       B.createHopToExecutor(
-                    RegularLocation::getAutoGeneratedLocation(F.getLocation()),
-                    ExpectedExecutor, /*mandatory*/ false);
+          RegularLocation::getDebugOnlyLocation(F.getLocation(), getModule()),
+          ExpectedExecutor,
+          /*mandatory*/ false);
     } else {
       // For a synchronous function, check that we're on the same executor.
       // Note: if we "know" that the code is completely Sendable-safe, this
@@ -733,7 +734,8 @@ SILValue SILGenFunction::emitGenericExecutor(SILLocation loc) {
 void SILGenFunction::emitPrologGlobalActorHop(SILLocation loc,
                                               Type globalActor) {
   ExpectedExecutor = emitLoadGlobalActorExecutor(globalActor);
-  B.createHopToExecutor(loc, ExpectedExecutor, /*mandatory*/ false);
+  B.createHopToExecutor(RegularLocation::getDebugOnlyLocation(loc, getModule()),
+                        ExpectedExecutor, /*mandatory*/ false);
 }
 
 SILValue SILGenFunction::emitLoadGlobalActorExecutor(Type globalActor) {
@@ -794,7 +796,8 @@ ExecutorBreadcrumb SILGenFunction::emitHopToTargetExecutor(
     SILLocation loc, SILValue executor) {
   // Record that we need to hop back to the current executor.
   auto breadcrumb = ExecutorBreadcrumb(true);
-  B.createHopToExecutor(loc.asAutoGenerated(), executor, /*mandatory*/ false);
+  B.createHopToExecutor(RegularLocation::getDebugOnlyLocation(loc, getModule()),
+                        executor, /*mandatory*/ false);
   return breadcrumb;
 }
 
@@ -835,7 +838,8 @@ void SILGenFunction::emitHopToActorValue(SILLocation loc, ManagedValue actor) {
       "Builtin.hopToActor must be in an actor-independent function");
   }
   SILValue executor = emitLoadActorExecutor(loc, actor);
-  B.createHopToExecutor(loc.asAutoGenerated(), executor, /*mandatory*/ true);
+  B.createHopToExecutor(RegularLocation::getDebugOnlyLocation(loc, getModule()),
+                        executor, /*mandatory*/ true);
 }
 
 void SILGenFunction::emitPreconditionCheckExpectedExecutor(
@@ -874,8 +878,9 @@ void ExecutorBreadcrumb::emit(SILGenFunction &SGF, SILLocation loc) {
   if (mustReturnToExecutor) {
     assert(SGF.ExpectedExecutor || SGF.unsafelyInheritsExecutor());
     if (auto executor = SGF.ExpectedExecutor)
-      SGF.B.createHopToExecutor(loc.asAutoGenerated(), executor,
-                                /*mandatory*/ false);
+      SGF.B.createHopToExecutor(
+          RegularLocation::getDebugOnlyLocation(loc, SGF.getModule()), executor,
+          /*mandatory*/ false);
   }
 }
 

lib/SILOptimizer/Mandatory/LowerHopToActor.cpp

@@ -48,7 +48,6 @@ namespace {
 /// IRGen expects hops to be to executors before it runs.
 class LowerHopToActor {
   SILFunction *F;
-  SILBuilder B;
   DominanceInfo *Dominance;
 
   /// A map from an actor value to the executor we've derived for it.
@@ -57,11 +56,12 @@ class LowerHopToActor {
   bool processHop(HopToExecutorInst *hop);
   bool processExtract(ExtractExecutorInst *extract);
 
-  SILValue emitGetExecutor(SILLocation loc, SILValue actor, bool makeOptional);
+  SILValue emitGetExecutor(SILBuilderWithScope &B, SILLocation loc,
+                           SILValue actor, bool makeOptional);
 
 public:
   LowerHopToActor(SILFunction *f, DominanceInfo *dominance)
-    : F(f), B(*F), Dominance(dominance) { }
+    : F(f), Dominance(dominance) { }
 
   /// The entry point to the transformation.
   bool run();
@@ -99,9 +99,7 @@ bool LowerHopToActor::processHop(HopToExecutorInst *hop) {
   if (isOptionalBuiltinExecutor(actor->getType()))
     return false;
 
-  B.setInsertionPoint(hop);
-  B.setCurrentDebugScope(hop->getDebugScope());
-
+  SILBuilderWithScope B(hop);
   SILValue executor;
   if (actor->getType().is<BuiltinExecutorType>()) {
     // IRGen expects an optional Builtin.Executor, not a Builtin.Executor
@@ -112,7 +110,7 @@ bool LowerHopToActor::processHop(HopToExecutorInst *hop) {
   } else {
     // Get the dominating executor value for this actor, if available,
     // or else emit code to derive it.
-    executor = emitGetExecutor(hop->getLoc(), actor, /*optional*/true);
+    executor = emitGetExecutor(B, hop->getLoc(), actor, /*optional*/true);
   }
   assert(executor && "executor not set");
 
@@ -127,9 +125,9 @@ bool LowerHopToActor::processExtract(ExtractExecutorInst *extract) {
   // Dig out the executor.
   auto executor = extract->getExpectedExecutor();
   if (!isOptionalBuiltinExecutor(executor->getType())) {
-    B.setInsertionPoint(extract);
-    B.setCurrentDebugScope(extract->getDebugScope());
-    executor = emitGetExecutor(extract->getLoc(), executor, /*optional*/false);
+    SILBuilderWithScope B(extract);
+    executor =
+        emitGetExecutor(B, extract->getLoc(), executor, /*optional*/ false);
   }
 
   // Unconditionally replace the extract with the executor.
@@ -156,7 +154,8 @@ static AccessorDecl *getUnownedExecutorGetter(ASTContext &ctx,
   return nullptr;
 }
 
-SILValue LowerHopToActor::emitGetExecutor(SILLocation loc, SILValue actor,
+SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
+                                          SILLocation loc, SILValue actor,
                                           bool makeOptional) {
   // Get the dominating executor value for this actor, if available,
   // or else emit code to derive it.

test/DebugInfo/async-let.swift

@@ -0,0 +1,38 @@
+// RUN: %target-swift-frontend %s -emit-ir -g -o - \
+// RUN:    -module-name M  -disable-availability-checking \
+// RUN:    -parse-as-library | %FileCheck %s --check-prefix=CHECK
+// REQUIRES: concurrency
+
+public actor Alice {
+  let bob = Bob()
+
+  // CHECK: define {{.*}}$s1M5AliceC4callyyYaFTY0_{{.*}} !dbg ![[SCOPE0:[0-9]+]]
+  // CHECK: call i8* @__swift_async_resume_get_context{{.*}}!dbg ![[HOP0:[0-9]+]]
+
+  // CHECK: define {{.*}}$s1M5AliceC4callyyYaFTY1_{{.*}} !dbg ![[SCOPE1:[0-9]+]]
+  // CHECK: call i8* @__swift_async_resume_get_context{{.*}}!dbg ![[HOP1:[0-9]+]]
+
+  // CHECK: define {{.*}}$s1M5AliceC4callyyYaFSiyYaYbcfu_TY0_{{.*}} !dbg ![[LET_SCOPE0:[0-9]+]]
+  // CHECK: call i8* @__swift_async_resume_get_context{{.*}}!dbg ![[LET_HOP0:[0-9]+]]
+
+  // CHECK: define {{.*}}$s1M5AliceC4callyyYaFSiyYaYbcfu_TY2_{{.*}} !dbg ![[LET_SCOPE1:[0-9]+]]
+  // CHECK: call i8* @__swift_async_resume_get_context{{.*}}!dbg ![[LET_HOP1:[0-9]+]]
+  public func call() async {
+    // CHECK: ![[SCOPE0]] = distinct !DISubprogram({{.*}}line: [[@LINE-1]]
+    // CHECK: ![[HOP0]] = !DILocation(line: [[@LINE-2]], column: 15
+    async let a = bob.call(x: 1)
+    // CHECK: ![[SCOPE1]] = distinct !DISubprogram({{.*}}line: [[@LINE-4]]
+    // CHECK: ![[HOP1]] = !DILocation(line: [[@LINE+5]], column: 17
+    // CHECK: ![[LET_SCOPE0]] = distinct !DISubprogram({{.*}}line: [[@LINE-3]]
+    // CHECK: ![[LET_HOP0]] = !DILocation(line: [[@LINE-4]], column: 19
+    // CHECK: ![[LET_SCOPE1]] = distinct !DISubprogram({{.*}}line: [[@LINE-5]]
+    // CHECK: ![[LET_HOP1]] = !DILocation(line: [[@LINE-6]], column: 23
+    print(await a)
+  }
+}
+
+public actor Bob {
+  public func call(x: Int) async -> Int {
+    return x + x
+  }
+}

test/SILGen/concurrent_prologue.swift

@@ -10,7 +10,7 @@
 // CHECK: %1 = function_ref  {{.*}}loc "{{.*}}.swift":[[@LINE-3]]:17,{{.*}}:auto_gen
 // CHECK: %2 = apply %1(%0)  {{.*}}loc "{{.*}}.swift":[[@LINE-4]]:17,{{.*}}:auto_gen
 // CHECK: begin_borrow       {{.*}}loc "{{.*}}.swift":[[@LINE-5]]:17,{{.*}}:auto_gen
-// CHECK: hop_to_executor    {{.*}}loc "{{.*}}.swift":[[@LINE-6]]:17,{{.*}}:auto_gen
+// CHECK: hop_to_executor    {{.*}}loc "{{.*}}.swift":[[@LINE-6]]:17,{{.*}}
 // CHECK: // end sil function '$s1a1fSiyYaF'
   return 23
 }