Merge pull request #65047 from adrian-prantl/107764966

swift-ci · web-flow · commit 703222583c64 · 2023-04-12T21:36:02.000-07:00
Ignore profile counter instructions in the dihole verifier.
diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp
@@ -6265,6 +6265,12 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     for (SILInstruction &SI : *BB) {
       if (SI.isMetaInstruction())
         continue;
+      // FIXME: Profile counters for loop bodies may be emitted before the
+      // instructions for the loop variable, but in a deeper scope.
+      if (isa<IncrementProfilerCounterInst>(SI))
+        continue;
+      if (!SI.getLoc().hasValidLineNumber())
+        continue;
       if (SI.getLoc().getKind() == SILLocation::CleanupKind)
         continue;
       // FIXME: These still leave holes in the scopes. We should make them
diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp
@@ -176,7 +176,7 @@ SILGenFunction::getSILDebugLocation(SILBuilder &B, SILLocation Loc,
   const SILDebugScope *Scope = B.getCurrentDebugScope();
   if (!Scope)
     Scope = F.getDebugScope();
-  if (auto *SILScope = getScopeOrNull(Loc)) {
+  if (auto *SILScope = getScopeOrNull(Loc, ForMetaInstruction)) {
     Scope = SILScope;
     // Metainstructions such as a debug_value may break the flow of scopes and
     // should not change the state of the builder.
@@ -187,14 +187,16 @@ SILGenFunction::getSILDebugLocation(SILBuilder &B, SILLocation Loc,
   return SILDebugLocation(overriddenLoc, Scope);
 }
 
-const SILDebugScope *SILGenFunction::getScopeOrNull(SILLocation Loc) {
-  if (Loc.getKind() == SILLocation::CleanupKind ||
-      Loc.getKind() == SILLocation::ImplicitReturnKind ||
-      // The source locations produced by the ResultBuilder transformation are
-      // all over the place.
-      Loc.isImplicit() ||
-      Loc.isAutoGenerated())
-    return nullptr;
+const SILDebugScope *SILGenFunction::getScopeOrNull(SILLocation Loc,
+                                                    bool ForMetaInstruction) {
+  if (!ForMetaInstruction) {
+    if (Loc.getKind() == SILLocation::CleanupKind ||
+        Loc.getKind() == SILLocation::ImplicitReturnKind ||
+        // The source locations produced by the ResultBuilder transformation are
+        // all over the place.
+        Loc.isImplicit() || Loc.isAutoGenerated())
+      return nullptr;
+  }
 
   SourceLoc SLoc = Loc.getSourceLoc();
   if (!SF || LastSourceLoc == SLoc)
@@ -218,35 +220,44 @@ const SILDebugScope *SILGenFunction::getOrCreateScope(SourceLoc SLoc) {
   return Scope;
 }
 
-static std::pair<SILLocation, std::string>
-getMacroName(GeneratedSourceInfo &Info) {
+namespace {
+struct MacroInfo {
+  MacroInfo(SourceLoc SLoc) : Loc(SLoc) {}
+  RegularLocation Loc;
+  std::string Name;
+  bool Freestanding = false;
+};
+}
+
+static MacroInfo getMacroInfo(GeneratedSourceInfo &Info) {
   SourceLoc MacroSLoc = Info.generatedSourceRange.getStart();
-  RegularLocation Loc(MacroSLoc);
-  Mangle::ASTMangler mangler;
-  std::string Name = "__unknown_macro__";
+  MacroInfo Result(MacroSLoc);
+  Result.Name = "__unknown_macro__";
   if (!Info.astNode)
-    return {Loc, Name};
+    return Result;
 
   // Keep this in sync with ASTMangler::appendMacroExpansionContext().
+  Mangle::ASTMangler mangler;
   switch (Info.kind) {
   case GeneratedSourceInfo::ExpressionMacroExpansion: {
     auto parent = ASTNode::getFromOpaqueValue(Info.astNode);
     if (auto expr =
             cast_or_null<MacroExpansionExpr>(parent.dyn_cast<Expr *>())) {
-      Loc = RegularLocation(expr);
-      Name = mangler.mangleMacroExpansion(expr);
+      Result.Loc = RegularLocation(expr);
+      Result.Name = mangler.mangleMacroExpansion(expr);
     } else {
       auto decl = cast<MacroExpansionDecl>(parent.get<Decl *>());
-      Loc = RegularLocation(decl);
-      Name = mangler.mangleMacroExpansion(decl);
+      Result.Loc = RegularLocation(decl);
+      Result.Name = mangler.mangleMacroExpansion(decl);
     }
     break;
   }
   case GeneratedSourceInfo::FreestandingDeclMacroExpansion: {
     auto expansion = cast<MacroExpansionDecl>(
         ASTNode::getFromOpaqueValue(Info.astNode).get<Decl *>());
-    Loc = RegularLocation(expansion);
-    Name = mangler.mangleMacroExpansion(expansion);
+    Result.Loc = RegularLocation(expansion);
+    Result.Name = mangler.mangleMacroExpansion(expansion);
+    Result.Freestanding = true;
     break;
   }
   case GeneratedSourceInfo::AccessorMacroExpansion:
@@ -257,16 +268,17 @@ getMacroName(GeneratedSourceInfo &Info) {
     auto decl = ASTNode::getFromOpaqueValue(Info.astNode).get<Decl *>();
     auto attr = Info.attachedMacroCustomAttr;
     if (auto *macroDecl = decl->getResolvedMacro(attr)) {
-      Loc = RegularLocation(macroDecl);
-      Name = macroDecl->getBaseName().userFacingName();
+      Result.Loc = RegularLocation(macroDecl);
+      Result.Name = macroDecl->getBaseName().userFacingName();
+      Result.Freestanding = true;
     }
     break;
   }
   case GeneratedSourceInfo::PrettyPrinted:
   case GeneratedSourceInfo::ReplacedFunctionBody:
     break;
   }
-  return {Loc, Name};
+  return Result;
 }
 
 const SILDebugScope *SILGenFunction::getMacroScope(SourceLoc SLoc) {
@@ -276,6 +288,15 @@ const SILDebugScope *SILGenFunction::getMacroScope(SourceLoc SLoc) {
   if (!GeneratedSourceInfo)
     return nullptr;
 
+  // There is no good way to represent freestanding macros as inlined functions,
+  // because entire function would need to be "inlined" into a top-level
+  // declaration that isn't part of a real function. By not handling them here,
+  // source locations will still point into the macro expansion buffer, but
+  // debug info doesn't know what macro that buffer was expanded from.
+  auto Macro = getMacroInfo(*GeneratedSourceInfo);
+  if (Macro.Freestanding)
+    return nullptr;
+  
   SourceLoc OrigSLoc = GeneratedSourceInfo->originalSourceRange.getStart();
   if (!OrigSLoc)
     return nullptr;
@@ -296,22 +317,24 @@ const SILDebugScope *SILGenFunction::getMacroScope(SourceLoc SLoc) {
       /*yields*/
       {},
       /*Results*/ {}, None, SubstitutionMap(), SubstitutionMap(), ASTContext);
-  auto LocName = getMacroName(*GeneratedSourceInfo);
-  RegularLocation MacroLoc(LocName.first);
-  StringRef MacroName = ASTContext.getIdentifier(LocName.second).str();
+  StringRef MacroName = ASTContext.getIdentifier(Macro.Name).str();
+  
   SILFunction *MacroFn = B.getOrCreateFunction(
-      MacroLoc, MacroName, SILLinkage::DefaultForDeclaration, FunctionType,
+      Macro.Loc, MacroName, SILLinkage::DefaultForDeclaration, FunctionType,
       IsNotBare, IsNotTransparent, IsNotSerialized, IsNotDynamic,
       IsNotDistributed, IsNotRuntimeAccessible);
   // At the end of the chain OrigSLoc should be a macro expansion node.
-  const SILDebugScope *InlinedAt = getOrCreateScope(OrigSLoc);
+  const SILDebugScope *InlinedAt = nullptr;
+  const SILDebugScope *OrigScope = getOrCreateScope(OrigSLoc);
+  RegularLocation OrigLoc(OrigSLoc);
   // Inject an extra scope to hold the inlined call site.
-  if (InlinedAt)
-    InlinedAt =
-        new (SGM.M) SILDebugScope(RegularLocation(OrigSLoc), nullptr, InlinedAt,
-                                  InlinedAt->InlinedCallSite);
+  if (OrigScope)
+    InlinedAt = new (SGM.M)
+        SILDebugScope(Macro.Freestanding ? Macro.Loc : OrigLoc, nullptr,
+                      OrigScope, OrigScope->InlinedCallSite);
+
   const SILDebugScope *Scope =
-      new (SGM.M) SILDebugScope(MacroLoc, MacroFn, nullptr, InlinedAt);
+      new (SGM.M) SILDebugScope(Macro.Loc, MacroFn, nullptr, InlinedAt);
 
   InlinedScopeMap.insert({BufferID, Scope});
   return Scope;
diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h
@@ -713,7 +713,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                       Optional<SILLocation> CurDebugLocOverride,
                       bool ForMetaInstruction);
 
-  const SILDebugScope *getScopeOrNull(SILLocation Loc);
+  const SILDebugScope *getScopeOrNull(SILLocation Loc,
+                                      bool ForMetaInstruction = false);
 
 private:
   const SILDebugScope *getOrCreateScope(SourceLoc SLoc);
diff --git a/test/DebugInfo/case-scope.swift b/test/DebugInfo/case-scope.swift
@@ -1,6 +1,5 @@
 // RUN: %target-swift-frontend -module-name a -parse-as-library -emit-sil -g %s | %FileCheck %s
 
-
 public enum E<T> {
 case A(T)
 case B(T)
@@ -13,15 +12,15 @@ func sink<T>(_ t: T) {}
 public func f<T>(_ e: E<T>) -> [T] {
   switch e {
   case .A(let a), .B(let a): return [a]
-  case .D(let a, _, let c): return [a, c]
-  default:
-    return []
+  case .D(let a, _, let c):  return [a, c]
+  default:                   return []
   }
 }
-// CHECK: sil_scope [[F:[0-9]+]] { loc "{{.*}}":13:13 parent @$s1a1fySayxGAA1EOyxGlF
-// CHECK: sil_scope [[S0:[0-9]+]] { loc "{{.*}}":14:3 parent [[F]] }
-// CHECK: sil_scope [[A0:[0-9]+]] { loc "{{.*}}":15:3 parent [[S0]] }
-// CHECK: sil_scope [[A1:[0-9]+]] { loc "{{.*}}":16:3 parent [[S0]] }
-// CHECK: alloc_stack {{.*}} $T, let, name "a", {{.*}}:15:15, scope [[A0]]
-// CHECK: alloc_stack {{.*}} $T, let, name "a", {{.*}}:15:26, scope [[A0]]
-// CHECK: alloc_stack {{.*}} $T, let, name "a", {{.*}}:16:15, scope [[A1]]
+
+// CHECK: sil_scope [[F:[0-9]+]] { loc "{{.*}}":12:13 parent @$s1a1fySayxGAA1EOyxGlF
+// CHECK: sil_scope [[S0:[0-9]+]] { loc "{{.*}}":13:3 parent [[F]] }
+// CHECK: sil_scope [[A0:[0-9]+]] { loc "{{.*}}":14:3 parent [[S0]] }
+// CHECK: sil_scope [[A1:[0-9]+]] { loc "{{.*}}":15:3 parent [[S0]] }
+// CHECK: alloc_stack {{.*}} $T, let, name "a", {{.*}}:14:15, scope [[A0]]
+// CHECK: alloc_stack {{.*}} $T, let, name "a", {{.*}}:14:26, scope [[A0]]
+// CHECK: alloc_stack {{.*}} $T, let, name "a", {{.*}}:15:15, scope [[A1]]
diff --git a/test/DebugInfo/case-scope2.swift b/test/DebugInfo/case-scope2.swift
@@ -0,0 +1,18 @@
+// RUN: %target-swift-frontend -module-name a -parse-as-library -emit-sil -g %s | %FileCheck %s
+func consume<T>(_ t: T) {}
+
+// CHECK: sil_scope [[F:[0-9]+]] { loc "{{.*}}":9:13 parent @$s1a1fyyShySiGSg_ADtF
+// CHECK: sil_scope [[S0:[0-9]+]] { loc "{{.*}}":10:3 parent [[F]] }
+// CHECK: sil_scope [[S1:[0-9]+]] { loc "{{.*}}":11:3 parent [[S0]] }
+// CHECK: sil_scope [[S2:[0-9]+]] { loc "{{.*}}":13:5 parent [[S1]] }
+// CHECK: sil_scope [[S3:[0-9]+]] { loc "{{.*}}":14:3 parent [[S0]] }
+public func f(_ s1: Set<Int>?, _ s2: Set<Int>?) {
+  switch (s1, s2) {
+  case (nil, let a), (let a, nil):
+  // CHECK: debug_value {{.*}} $Optional<Set<Int>>, let, name "a", {{.*}}:[[@LINE-1]]:18, scope [[S1]]
+    consume(a)
+  case (let a?, _):
+  // CHECK: debug_value {{.*}} $Set<Int>, let, name "a", {{.*}}:[[@LINE-1]]:13, scope [[S3]]
+    consume((a))
+  }
+}
diff --git a/test/DebugInfo/profile_counter.swift b/test/DebugInfo/profile_counter.swift
@@ -0,0 +1,13 @@
+// RUN: %target-swift-frontend %s -g -emit-sil  -profile-generate -profile-coverage-mapping -parse-as-library -o - | %FileCheck %s
+
+func consume<T>(_ t: T) {}
+
+public func f<T>(collection : [T]) {
+  for element in collection {
+    // CHECK: increment_profiler_counter {{.*}}:[[@LINE-1]]:29, scope [[SCOPE:[0-9]+]]
+    // FIXME: Ideally, these would share the same scope, or the increment should come below the variable initialization code.
+    // CHECK: unchecked_take_enum_data_addr {{.*}}:[[@LINE-3]]:3, scope
+    // CHECK: copy_addr {{.*}}:[[@LINE-4]]:29, scope [[SCOPE]]
+    consume(element)
+  }
+}
diff --git a/test/Macros/macro_expand.swift b/test/Macros/macro_expand.swift
@@ -253,6 +253,9 @@ func testNestedDeclInExpr() {
 @freestanding(declaration, names: named(A), named(B), named(foo), named(addOne))
 macro defineDeclsWithKnownNames() = #externalMacro(module: "MacroDefinition", type: "DefineDeclsWithKnownNamesMacro")
 
+// Freestanding macros are not in inlined scopes.
+// CHECK-SIL: sil_scope {{.*}} { loc "@__swiftmacro_9MacroUser016testFreestandingA9ExpansionyyF4Foo2L_V25defineDeclsWithKnownNamesfMf0_.swift":9:14 {{.*}} -> Int }
+
 // FIXME: Macros producing arbitrary names are not supported yet
 #if false
 #bitwidthNumberedStructs("MyIntGlobal")
