Merge pull request #61351 from hamishknight/undercover

Author: hamishknight

include/swift/SIL/SILDeclRef.h

@@ -483,9 +483,6 @@ struct SILDeclRef {
     return result;
   }
 
-  /// True is the decl ref references any kind of thunk.
-  bool isAnyThunk() const;
-
   /// True if the decl ref references a thunk from a natively foreign
   /// declaration to Swift calling convention.
   bool isForeignToNativeThunk() const;

include/swift/SIL/SILProfiler.h

@@ -31,9 +31,6 @@ class SILCoverageMap;
 class SILFunction;
 class SILModule;
 
-/// Returns whether the given AST node requires profiling instrumentation.
-bool doesASTRequireProfiling(SILModule &M, ASTNode N, SILDeclRef Constant);
-
 /// SILProfiler - Maps AST nodes to profile counters.
 class SILProfiler : public SILAllocated<SILProfiler> {
 private:

lib/SIL/IR/SILDeclRef.cpp

@@ -313,6 +313,25 @@ bool SILDeclRef::hasUserWrittenCode() const {
   // user code into their body.
   switch (kind) {
   case Kind::Func: {
+    if (getAbstractClosureExpr()) {
+      // Auto-closures have user-written code.
+      if (auto *ACE = getAutoClosureExpr()) {
+        // Currently all types of auto-closures can contain user code. Note this
+        // logic does not affect delayed emission, as we eagerly emit all
+        // closure definitions. This does however affect profiling.
+        switch (ACE->getThunkKind()) {
+        case AutoClosureExpr::Kind::None:
+        case AutoClosureExpr::Kind::SingleCurryThunk:
+        case AutoClosureExpr::Kind::DoubleCurryThunk:
+        case AutoClosureExpr::Kind::AsyncLet:
+          return true;
+        }
+        llvm_unreachable("Unhandled case in switch!");
+      }
+      // Otherwise, assume an implicit closure doesn't have user code.
+      return false;
+    }
+
     // Lazy getters splice in the user-written initializer expr.
     if (auto *accessor = dyn_cast<AccessorDecl>(getFuncDecl())) {
       auto *storage = accessor->getStorage();
@@ -914,11 +933,6 @@ bool SILDeclRef::isBackDeployed() const {
   return false;
 }
 
-bool SILDeclRef::isAnyThunk() const {
-  return isForeignToNativeThunk() || isNativeToForeignThunk() ||
-         isDistributedThunk() || isBackDeploymentThunk();
-}
-
 bool SILDeclRef::isForeignToNativeThunk() const {
   // If this isn't a native entry-point, it's not a foreign-to-native thunk.
   if (isForeign)
@@ -1082,7 +1096,7 @@ std::string SILDeclRef::mangle(ManglingKind MKind) const {
     // Use the SILGen name only for the original non-thunked, non-curried entry
     // point.
     if (auto NameA = getDecl()->getAttrs().getAttribute<SILGenNameAttr>())
-      if (!NameA->Name.empty() && !isAnyThunk()) {
+      if (!NameA->Name.empty() && !isThunk()) {
         return NameA->Name.str();
       }
 

lib/SIL/IR/SILFunction.cpp

@@ -335,6 +335,20 @@ void SILFunction::deleteSnapshot(int ID) {
 void SILFunction::createProfiler(ASTNode Root, SILDeclRef Ref) {
   assert(!Profiler && "Function already has a profiler");
   Profiler = SILProfiler::create(Module, Root, Ref);
+  if (!Profiler || Ref.isNull())
+    return;
+
+  // If we loaded a profile, set the entry counts for functions and closures
+  // for PGO to use.
+  if (Ref.isFunc()) {
+    if (auto *Closure = Ref.getAbstractClosureExpr()) {
+      setEntryCount(Profiler->getExecutionCount(Closure));
+    } else {
+      auto *FD = Ref.getFuncDecl();
+      assert(FD);
+      setEntryCount(Profiler->getExecutionCount(FD->getBody()));
+    }
+  }
 }
 
 bool SILFunction::hasForeignBody() const {

lib/SIL/IR/SILProfiler.cpp

@@ -32,15 +32,6 @@
 
 using namespace swift;
 
-/// Check if a closure has a body.
-static bool doesClosureHaveBody(AbstractClosureExpr *ACE) {
-  if (auto *CE = dyn_cast<ClosureExpr>(ACE))
-    return CE->getBody();
-  if (auto *autoCE = dyn_cast<AutoClosureExpr>(ACE))
-    return autoCE->getBody();
-  return false;
-}
-
 /// Check whether a root AST node should be profiled.
 static bool shouldProfile(ASTNode N, SILDeclRef Constant) {
   // Do not profile AST nodes with invalid source locations.
@@ -67,62 +58,15 @@ static bool shouldProfile(ASTNode N, SILDeclRef Constant) {
     }
   }
 
-  if (auto *E = N.dyn_cast<Expr *>()) {
-    if (auto *CE = dyn_cast<AbstractClosureExpr>(E)) {
-      // Only profile closure expressions with bodies.
-      if (!doesClosureHaveBody(CE)) {
-        LLVM_DEBUG(llvm::dbgs() << "Skipping ASTNode: closure without body\n");
-        return false;
-      }
-
-      // Don't profile implicit closures, unless they're autoclosures.
-      if (!isa<AutoClosureExpr>(CE) && CE->isImplicit()) {
-        LLVM_DEBUG(llvm::dbgs() << "Skipping ASTNode: implicit closure expr\n");
-        return false;
-      }
-    }
-
-    // Profile all other kinds of expressions.
-    return true;
-  }
-
-  auto *D = N.get<Decl *>();
-  if (auto *AFD = dyn_cast<AbstractFunctionDecl>(D)) {
-    // Don't profile functions without bodies.
-    if (!AFD->hasBody()) {
-      LLVM_DEBUG(llvm::dbgs() << "Skipping ASTNode: function without body\n");
-      return false;
-    }
-
-    // Profile implicit getters for lazy variables.
-    if (auto *accessor = dyn_cast<AccessorDecl>(AFD)) {
-      if (accessor->isImplicit() && accessor->isGetter() &&
-          accessor->getStorage()->getAttrs().hasAttribute<LazyAttr>()) {
-        return true;
-      }
-    }
-  }
-
-  // Skip any remaining implicit, or otherwise unsupported decls.
-  if (D->isImplicit() || isa<EnumCaseDecl>(D)) {
-    LLVM_DEBUG(llvm::dbgs() << "Skipping ASTNode: implicit/unsupported decl\n");
+  // Do not profile code that hasn't been written by the user.
+  if (!Constant.hasUserWrittenCode()) {
+    LLVM_DEBUG(llvm::dbgs() << "Skipping ASTNode: no user-written code\n");
     return false;
   }
 
   return true;
 }
 
-namespace swift {
-bool doesASTRequireProfiling(SILModule &M, ASTNode N, SILDeclRef Constant) {
-  // If profiling isn't enabled, don't profile anything.
-  auto &Opts = M.getOptions();
-  if (Opts.UseProfile.empty() && !Opts.GenerateProfile)
-    return false;
-
-  return shouldProfile(N, Constant);
-}
-} // namespace swift
-
 /// Get the DeclContext for the decl referenced by \p forDecl.
 DeclContext *getProfilerContextForDecl(ASTNode N, SILDeclRef forDecl) {
   if (auto *D = N.dyn_cast<Decl *>())
@@ -152,29 +96,34 @@ static bool hasASTBeenTypeChecked(ASTNode N, SILDeclRef forDecl) {
   return !SF || SF->ASTStage >= SourceFile::TypeChecked;
 }
 
-/// Check whether a mapped AST node requires a new profiler.
+/// Check whether a mapped AST node is valid for profiling.
 static bool canCreateProfilerForAST(ASTNode N, SILDeclRef forDecl) {
   assert(hasASTBeenTypeChecked(N, forDecl) &&
          "Cannot use this AST for profiling");
 
   if (auto *D = N.dyn_cast<Decl *>()) {
-    if (isa<AbstractFunctionDecl>(D))
-      return true;
+    if (auto *AFD = dyn_cast<AbstractFunctionDecl>(D))
+      return AFD->hasBody();
 
     if (isa<TopLevelCodeDecl>(D))
       return true;
   } else if (N.get<Expr *>()) {
+    if (auto *closure = forDecl.getAbstractClosureExpr())
+      return closure->hasBody();
     if (forDecl.isStoredPropertyInitializer() ||
-        forDecl.isPropertyWrapperBackingInitializer() ||
-        forDecl.getAbstractClosureExpr())
+        forDecl.isPropertyWrapperBackingInitializer())
       return true;
   }
   return false;
 }
 
 SILProfiler *SILProfiler::create(SILModule &M, ASTNode N, SILDeclRef Ref) {
+  // If profiling isn't enabled, don't profile anything.
   const auto &Opts = M.getOptions();
-  if (!doesASTRequireProfiling(M, N, Ref))
+  if (!Opts.GenerateProfile && Opts.UseProfile.empty())
+    return nullptr;
+
+  if (!shouldProfile(N, Ref))
     return nullptr;
 
   if (!canCreateProfilerForAST(N, Ref)) {

lib/SILGen/SILGen.cpp

@@ -654,35 +654,6 @@ static SILFunction *getFunctionToInsertAfter(SILGenModule &SGM,
   return nullptr;
 }
 
-static bool haveProfiledAssociatedFunction(SILDeclRef constant) {
-  return constant.isDefaultArgGenerator() || constant.isForeign;
-}
-
-/// Set up the function for profiling instrumentation.
-static void setUpForProfiling(SILDeclRef constant, SILFunction *F,
-                              ForDefinition_t forDefinition) {
-  if (!forDefinition || F->getProfiler())
-    return;
-
-  ASTNode profiledNode;
-  if (!haveProfiledAssociatedFunction(constant)) {
-    if (constant.hasDecl()) {
-      if (auto *fd = constant.getFuncDecl()) {
-        if (fd->hasBody()) {
-          F->createProfiler(fd, constant);
-          profiledNode = fd->getBody(/*canSynthesize=*/false);
-        }
-      }
-    } else if (auto *ace = constant.getAbstractClosureExpr()) {
-      F->createProfiler(ace, constant);
-      profiledNode = ace;
-    }
-    // Set the function entry count for PGO.
-    if (SILProfiler *SP = F->getProfiler())
-      F->setEntryCount(SP->getExecutionCount(profiledNode));
-  }
-}
-
 static bool isEmittedOnDemand(SILModule &M, SILDeclRef constant) {
   if (!constant.hasDecl())
     return false;
@@ -730,12 +701,9 @@ static bool isEmittedOnDemand(SILModule &M, SILDeclRef constant) {
 
 SILFunction *SILGenModule::getFunction(SILDeclRef constant,
                                        ForDefinition_t forDefinition) {
-  // If we already emitted the function, return it (potentially preparing it
-  // for definition).
-  if (auto emitted = getEmittedFunction(constant, forDefinition)) {
-    setUpForProfiling(constant, emitted, forDefinition);
+  // If we already emitted the function, return it.
+  if (auto emitted = getEmittedFunction(constant, forDefinition))
     return emitted;
-  }
 
   auto getBestLocation = [](SILDeclRef ref) -> SILLocation {
     if (ref.hasDecl())
@@ -755,7 +723,6 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant,
       [&IGM](SILLocation loc, SILDeclRef constant) -> SILFunction * {
         return IGM.getFunction(constant, NotForDefinition);
       });
-  setUpForProfiling(constant, F, forDefinition);
 
   assert(F && "SILFunction should have been defined");
 
@@ -889,6 +856,7 @@ void SILGenModule::emitFunctionDefinition(SILDeclRef constant, SILFunction *f) {
     if (auto *ce = constant.getAbstractClosureExpr()) {
       preEmitFunction(constant, f, ce);
       PrettyStackTraceSILFunction X("silgen closureexpr", f);
+      f->createProfiler(ce, constant);
       SILGenFunction(*this, *f, ce).emitClosure(ce);
       postEmitFunction(constant, f);
       break;
@@ -898,6 +866,7 @@ void SILGenModule::emitFunctionDefinition(SILDeclRef constant, SILFunction *f) {
 
     preEmitFunction(constant, f, fd);
     PrettyStackTraceSILFunction X("silgen emitFunction", f);
+    f->createProfiler(fd, constant);
     SILGenFunction(*this, *f, fd).emitFunction(fd);
     postEmitFunction(constant, f);
     break;
@@ -1173,24 +1142,20 @@ void SILGenModule::emitFunctionDefinition(SILDeclRef constant, SILFunction *f) {
 
 /// Emit a function now, if it's externally usable or has been referenced in
 /// the current TU, or remember how to emit it later if not.
-static void emitOrDelayFunction(SILGenModule &SGM,
-                                SILDeclRef constant,
-                                bool forceEmission = false) {
+static void emitOrDelayFunction(SILGenModule &SGM, SILDeclRef constant) {
   assert(!constant.isThunk());
   assert(!constant.isClangImported());
 
   auto emitAfter = SGM.lastEmittedFunction;
 
-  SILFunction *f = nullptr;
-
   // Implicit decls may be delayed if they can't be used externally.
   auto linkage = constant.getLinkage(ForDefinition);
-  bool mayDelay = !forceEmission &&
-             (!constant.hasUserWrittenCode() &&
-              !constant.isDynamicallyReplaceable() &&
-              !isPossiblyUsedExternally(linkage, SGM.M.isWholeModule()));
+  bool mayDelay = !constant.hasUserWrittenCode() &&
+                  !constant.isDynamicallyReplaceable() &&
+                  !isPossiblyUsedExternally(linkage, SGM.M.isWholeModule());
 
   // Avoid emitting a delayable definition if it hasn't already been referenced.
+  SILFunction *f = nullptr;
   if (mayDelay)
     f = SGM.getEmittedFunction(constant, ForDefinition);
   else
@@ -1444,12 +1409,8 @@ void SILGenModule::emitFunction(FuncDecl *fd) {
 
   emitAbstractFuncDecl(fd);
 
-  if (fd->hasBody()) {
-    SILDeclRef constant(decl);
-    bool ForCoverageMapping = doesASTRequireProfiling(M, fd, constant);
-    emitOrDelayFunction(*this, constant,
-                        /*forceEmission=*/ForCoverageMapping);
-  }
+  if (fd->hasBody())
+    emitOrDelayFunction(*this, SILDeclRef(decl));
 }
 
 void SILGenModule::addGlobalVariable(VarDecl *global) {

test/Profiler/Inputs/unmapped_secondary.swift

@@ -0,0 +1,22 @@
+// A secondary file for unmapped.swift that can contain profiled code.
+
+@propertyWrapper
+struct Wrapper<T> {
+  var wrappedValue: T
+}
+
+struct Projected<T> {
+  var value: T
+}
+
+@propertyWrapper
+struct WrapperWithProjectedValue<T> {
+  var wrappedValue: T
+  init(projectedValue: Projected<T>) {
+    self.wrappedValue = projectedValue.value
+  }
+  var projectedValue: Projected<T> {
+    Projected(value: wrappedValue)
+  }
+}
+

test/Profiler/coverage_property_wrapper_backing.swift

@@ -1,4 +1,5 @@
-// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sorted-sil -emit-sil -module-name coverage_property_wrapper_backing %s | %FileCheck %s
+// This uses '-primary-file' to ensure we're conservative with lazy SIL emission.
+// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sorted-sil -emit-sil -module-name coverage_property_wrapper_backing -primary-file %s | %FileCheck %s
 // RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -emit-ir %s
 
 @propertyWrapper
@@ -9,6 +10,11 @@ struct Wrapper<T> {
   }
 }
 
+@propertyWrapper
+struct PassThroughWrapper<T> {
+  var wrappedValue: T
+}
+
 // rdar://99931619 – Make sure we emit the profiler increment for the backing
 // initializer.
 
@@ -32,6 +38,20 @@ struct S {
   // CHECK-NEXT: }
 }
 
-// FIXME(rdar://99962285): This is currently needed to SILGen the property
-// initializer for 'i'.
-_ = S().i
+struct T {
+  // The backing initializer for j has no user-written code, so no coverage map needed.
+  // CHECK-NOT: sil_coverage_map {{.*}} "s33coverage_property_wrapper_backing1TV1jSivpfP"
+  @PassThroughWrapper
+  var j = 3
+
+  // FIXME: Ideally the region here would only span the length of the @Wrapper
+  // argument list.
+  // CHECK-LABEL: sil_coverage_map {{.*}} "$s33coverage_property_wrapper_backing1TV1kSivpfP" {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.T.k
+  // CHECK-NEXT:  [[@LINE+4]]:4 -> [[@LINE+5]]:30 : 0
+  // CHECK-NEXT:  [[@LINE+4]]:24 -> [[@LINE+4]]:25 : 1
+  // CHECK-NEXT:  [[@LINE+3]]:28 -> [[@LINE+3]]:29 : (0 - 1)
+  // CHECK-NEXT:  }
+  @PassThroughWrapper
+  @Wrapper(.random() ? 1 : 2)
+  var k = 3
+}