Merge pull request #62418 from hamishknight/qwoops

hamishknight · web-flow · commit 67909c0c5805 · 2022-12-07T15:59:33.000Z
diff --git a/include/swift/SIL/SILCoverageMap.h b/include/swift/SIL/SILCoverageMap.h
@@ -55,6 +55,15 @@ class SILCoverageMap : public llvm::ilist_node<SILCoverageMap>,
   };
 
 private:
+  /// The parent source file containing the coverage map.
+  /// 
+  /// NOTE: `ParentSourceFile->getFilename()` is not necessarily equivalent to
+  /// `Filename`. `Filename` could be a .swift file, and `ParentSourceFile`
+  /// could be a parsed .sil file. As such, this should only be used for
+  /// determining which object file to emit the coverage map into.
+  /// `Filename` should be used for coverage data.
+  SourceFile *ParentSourceFile;
+
   // The name of the source file where this mapping is found.
   StringRef Filename;
 
@@ -78,19 +87,28 @@ class SILCoverageMap : public llvm::ilist_node<SILCoverageMap>,
   SILCoverageMap &operator=(const SILCoverageMap &) = delete;
 
   /// Private constructor. Create these using SILCoverageMap::create.
-  SILCoverageMap(uint64_t Hash);
+  SILCoverageMap(SourceFile *ParentSourceFile, uint64_t Hash);
 
 public:
   ~SILCoverageMap();
 
   static SILCoverageMap *
-  create(SILModule &M, StringRef Filename, StringRef Name,
-         StringRef PGOFuncName, uint64_t Hash,
+  create(SILModule &M, SourceFile *ParentSourceFile, StringRef Filename,
+         StringRef Name, StringRef PGOFuncName, uint64_t Hash,
          ArrayRef<MappedRegion> MappedRegions,
          ArrayRef<llvm::coverage::CounterExpression> Expressions);
 
+  /// The parent source file containing the coverage map.
+  ///
+  /// NOTE: `getParentSourceFile()->getFilename()` is not necessarily equivalent
+  /// to `getFilename()`. `getFilename()` could be a .swift file, and
+  /// `getParentSourceFile()` could be a parsed .sil file. As such, this should
+  /// only be used for determining which object file to emit the coverage map
+  /// in. `getFilename()` should be used for coverage data.
+  SourceFile *getParentSourceFile() const { return ParentSourceFile; }
+
   /// Return the name of the source file where this mapping is found.
-  StringRef getFile() const { return Filename; }
+  StringRef getFilename() const { return Filename; }
 
   /// Return the mangled name of the function this mapping covers.
   StringRef getName() const { return Name; }
diff --git a/lib/IRGen/GenCoverage.cpp b/lib/IRGen/GenCoverage.cpp
@@ -42,11 +42,14 @@ static std::string getInstrProfSection(IRGenModule &IGM,
   return llvm::getInstrProfSectionName(SK, IGM.Triple.getObjectFormat());
 }
 
-void IRGenModule::emitCoverageMapping() {
+void IRGenModule::emitCoverageMaps(ArrayRef<const SILCoverageMap *> Mappings) {
+  // If there aren't any coverage maps, there's nothing to emit.
+  if (Mappings.empty())
+    return;
+
   SmallVector<llvm::Constant *, 4> UnusedFuncNames;
-  std::vector<const SILCoverageMap *> Mappings;
-  for (const auto &M : getSILModule().getCoverageMaps()) {
-    auto FuncName = M.second->getPGOFuncName();
+  for (const auto *Mapping : Mappings) {
+    auto FuncName = Mapping->getPGOFuncName();
     auto VarLinkage = llvm::GlobalValue::LinkOnceAnyLinkage;
     auto FuncNameVarName = llvm::getPGOFuncNameVarName(FuncName, VarLinkage);
 
@@ -58,13 +61,8 @@ void IRGenModule::emitCoverageMapping() {
       auto *Var = llvm::createPGOFuncNameVar(Module, VarLinkage, FuncName);
       UnusedFuncNames.push_back(llvm::ConstantExpr::getBitCast(Var, Int8PtrTy));
     }
-    Mappings.push_back(M.second);
   }
 
-  // If there aren't any coverage maps, there's nothing to emit.
-  if (Mappings.empty())
-    return;
-
   // Emit the name data for any unused functions.
   if (!UnusedFuncNames.empty()) {
     auto NamePtrsTy = llvm::ArrayType::get(Int8PtrTy, UnusedFuncNames.size());
@@ -80,8 +78,8 @@ void IRGenModule::emitCoverageMapping() {
 
   std::vector<StringRef> Files;
   for (const auto &M : Mappings)
-    if (std::find(Files.begin(), Files.end(), M->getFile()) == Files.end())
-      Files.push_back(M->getFile());
+    if (std::find(Files.begin(), Files.end(), M->getFilename()) == Files.end())
+      Files.push_back(M->getFilename());
 
   auto remapper = getOptions().CoveragePrefixMap;
 
@@ -115,7 +113,7 @@ void IRGenModule::emitCoverageMapping() {
     std::string FuncRecordName = "__covrec_" + llvm::utohexstr(NameHash);
 
     unsigned FileID =
-        std::find(Files.begin(), Files.end(), M->getFile()) - Files.begin();
+        std::find(Files.begin(), Files.end(), M->getFilename()) - Files.begin();
     std::vector<CounterMappingRegion> Regions;
     for (const auto &MR : M->getMappedRegions())
       Regions.emplace_back(CounterMappingRegion::makeRegion(
@@ -194,6 +192,27 @@ void IRGenModule::emitCoverageMapping() {
 }
 
 void IRGenerator::emitCoverageMapping() {
-  for (auto &IGM : *this)
-    IGM.second->emitCoverageMapping();
+  if (SIL.getCoverageMaps().empty())
+    return;
+
+  // Shard the coverage maps across their designated IRGenModules. This is
+  // necessary to ensure we don't output N copies of a coverage map when doing
+  // parallel IRGen, where N is the number of output object files.
+  //
+  // Note we don't just dump all the coverage maps into the primary IGM as
+  // that would require creating unecessary name data entries, since the name
+  // data is likely to already be present in the IGM that contains the entity
+  // being profiled (unless it has been optimized out). Matching the coverage
+  // map to its originating SourceFile also matches the behavior of a debug
+  // build where the files are compiled separately.
+  llvm::DenseMap<IRGenModule *, std::vector<const SILCoverageMap *>> MapsToEmit;
+  for (const auto &M : SIL.getCoverageMaps()) {
+    auto &Mapping = M.second;
+    auto *SF = Mapping->getParentSourceFile();
+    MapsToEmit[getGenModule(SF)].push_back(Mapping);
+  }
+  for (auto &IGMPair : *this) {
+    auto *IGM = IGMPair.second;
+    IGM->emitCoverageMaps(MapsToEmit[IGM]);
+  }
 }
diff --git a/lib/IRGen/IRGen.cpp b/lib/IRGen/IRGen.cpp
@@ -1369,7 +1369,7 @@ GeneratedModule IRGenRequest::evaluate(Evaluator &evaluator,
     // Emit coverage mapping info. This needs to happen after we've emitted
     // any lazy definitions, as we need to know whether or not we emitted a
     // profiler increment for a given coverage map.
-    IGM.emitCoverageMapping();
+    irgen.emitCoverageMapping();
 
     // Emit symbols for eliminated dead methods.
     IGM.emitVTableStubs();
diff --git a/lib/IRGen/IRGenModule.cpp b/lib/IRGen/IRGenModule.cpp
@@ -1892,6 +1892,17 @@ void IRGenerator::addGenModule(SourceFile *SF, IRGenModule *IGM) {
   Queue.push_back(IGM);
 }
 
+IRGenModule *IRGenerator::getGenModule(SourceFile *SF) {
+  // If we're emitting for a single module, or a single file, we always use the
+  // primary IGM.
+  if (GenModules.size() == 1)
+    return getPrimaryIGM();
+
+ IRGenModule *IGM = GenModules[SF];
+ assert(IGM);
+ return IGM;
+}
+
 IRGenModule *IRGenerator::getGenModule(DeclContext *ctxt) {
   if (GenModules.size() == 1 || !ctxt) {
     return getPrimaryIGM();
diff --git a/lib/IRGen/IRGenModule.h b/lib/IRGen/IRGenModule.h
@@ -104,6 +104,7 @@ namespace swift {
   class ProtocolConformance;
   class ProtocolCompositionType;
   class RootProtocolConformance;
+  class SILCoverageMap;
   struct SILDeclRef;
   class SILDefaultWitnessTable;
   class SILDifferentiabilityWitness;
@@ -350,11 +351,7 @@ class IRGenerator {
   void addGenModule(SourceFile *SF, IRGenModule *IGM);
   
   /// Get an IRGenModule for a source file.
-  IRGenModule *getGenModule(SourceFile *SF) {
-    IRGenModule *IGM = GenModules[SF];
-    assert(IGM);
-    return IGM;
-  }
+  IRGenModule *getGenModule(SourceFile *SF);
 
   SourceFile *getSourceFile(IRGenModule *module) {
     for (auto pair : GenModules) {
@@ -1481,7 +1478,7 @@ private:                                                                       \
   void maybeEmitOpaqueTypeDecl(OpaqueTypeDecl *D);
 
   void emitSILGlobalVariable(SILGlobalVariable *gv);
-  void emitCoverageMapping();
+  void emitCoverageMaps(ArrayRef<const SILCoverageMap *> Mappings);
   void emitSILFunction(SILFunction *f);
   void emitSILWitnessTable(SILWitnessTable *wt);
   void emitSILProperty(SILProperty *prop);
diff --git a/lib/SIL/IR/SILCoverageMap.cpp b/lib/SIL/IR/SILCoverageMap.cpp
@@ -24,12 +24,13 @@ using namespace swift;
 using llvm::coverage::CounterExpression;
 
 SILCoverageMap *
-SILCoverageMap::create(SILModule &M, StringRef Filename, StringRef Name,
+SILCoverageMap::create(SILModule &M, SourceFile *ParentSourceFile,
+                       StringRef Filename, StringRef Name,
                        StringRef PGOFuncName, uint64_t Hash,
                        ArrayRef<MappedRegion> MappedRegions,
                        ArrayRef<CounterExpression> Expressions) {
   auto *Buf = M.allocate<SILCoverageMap>(1);
-  SILCoverageMap *CM = ::new (Buf) SILCoverageMap(Hash);
+  SILCoverageMap *CM = ::new (Buf) SILCoverageMap(ParentSourceFile, Hash);
 
   // Store a copy of the names so that we own the lifetime.
   CM->Filename = M.allocateCopy(Filename);
@@ -50,7 +51,8 @@ SILCoverageMap::create(SILModule &M, StringRef Filename, StringRef Name,
   return CM;
 }
 
-SILCoverageMap::SILCoverageMap(uint64_t Hash) : Hash(Hash) {}
+SILCoverageMap::SILCoverageMap(SourceFile *ParentSourceFile, uint64_t Hash)
+  : ParentSourceFile(ParentSourceFile), Hash(Hash) {}
 
 SILCoverageMap::~SILCoverageMap() {}
 
diff --git a/lib/SIL/IR/SILPrinter.cpp b/lib/SIL/IR/SILPrinter.cpp
@@ -3877,7 +3877,7 @@ void SILDifferentiabilityWitness::dump() const {
 
 void SILCoverageMap::print(SILPrintContext &PrintCtx) const {
   llvm::raw_ostream &OS = PrintCtx.OS();
-  OS << "sil_coverage_map " << QuotedString(getFile()) << " "
+  OS << "sil_coverage_map " << QuotedString(getFilename()) << " "
      << QuotedString(getName()) << " " << QuotedString(getPGOFuncName()) << " "
      << getHash() << " {\t// " << demangleSymbol(getName()) << "\n";
   if (PrintCtx.sortSIL())
diff --git a/lib/SIL/IR/SILProfiler.cpp b/lib/SIL/IR/SILProfiler.cpp
@@ -1033,7 +1033,7 @@ struct CoverageMapping : public ASTWalker {
   SILCoverageMap *emitSourceRegions(
       SILModule &M, StringRef Name, StringRef PGOFuncName, uint64_t Hash,
       llvm::DenseMap<ProfileCounterRef, unsigned> &CounterIndices,
-      StringRef Filename) {
+      SourceFile *SF, StringRef Filename) {
     if (SourceRegions.empty())
       return nullptr;
 
@@ -1051,8 +1051,8 @@ struct CoverageMapping : public ASTWalker {
       Regions.emplace_back(Start.first, Start.second, End.first, End.second,
                            Counter.expand(Builder, CounterIndices));
     }
-    return SILCoverageMap::create(M, Filename, Name, PGOFuncName, Hash, Regions,
-                                  Builder.getExpressions());
+    return SILCoverageMap::create(M, SF, Filename, Name, PGOFuncName, Hash,
+                                  Regions, Builder.getExpressions());
   }
 
   PreWalkAction walkToDeclPre(Decl *D) override {
@@ -1331,13 +1331,6 @@ getEquivalentPGOLinkage(FormalLinkage Linkage) {
   llvm_unreachable("Unhandled FormalLinkage in switch.");
 }
 
-static StringRef getCurrentFileName(SILDeclRef forDecl) {
-  auto *DC = forDecl.getInnermostDeclContext();
-  if (auto *ParentFile = DC->getParentSourceFile())
-    return ParentFile->getFilename();
-  return {};
-}
-
 static void walkNode(NodeToProfile Node, ASTWalker &Walker) {
   if (auto N = Node.getAsNode()) {
     N.walk(Walker);
@@ -1350,8 +1343,11 @@ static void walkNode(NodeToProfile Node, ASTWalker &Walker) {
 
 void SILProfiler::assignRegionCounters() {
   const auto &SM = M.getASTContext().SourceMgr;
+  auto *DC = forDecl.getInnermostDeclContext();
+  auto *SF = DC->getParentSourceFile();
+  assert(SF && "Not within a SourceFile?");
 
-  CurrentFileName = getCurrentFileName(forDecl);
+  CurrentFileName = SF->getFilename();
 
   MapRegionCounters Mapper(forDecl, RegionCounterMap);
 
@@ -1387,7 +1383,7 @@ void SILProfiler::assignRegionCounters() {
     walkNode(Root, Coverage);
     CovMap =
         Coverage.emitSourceRegions(M, CurrentFuncName, PGOFuncName, PGOFuncHash,
-                                   RegionCounterMap, CurrentFileName);
+                                   RegionCounterMap, SF, CurrentFileName);
   }
 
   if (llvm::IndexedInstrProfReader *IPR = M.getPGOReader()) {
diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp
@@ -7946,8 +7946,9 @@ bool SILParserState::parseSILCoverageMap(Parser &P) {
                        LBraceLoc);
 
   if (!BodyHasError)
-    SILCoverageMap::create(M, Filename.str(), FuncName.str(), PGOFuncName.str(),
-                           Hash, Regions, Builder.getExpressions());
+    SILCoverageMap::create(M, &P.SF, Filename.str(), FuncName.str(),
+                           PGOFuncName.str(), Hash, Regions,
+                           Builder.getExpressions());
   return false;
 }
 
diff --git a/test/Profiler/Inputs/coverage_num_threads3.swift b/test/Profiler/Inputs/coverage_num_threads3.swift
@@ -0,0 +1 @@
+public func func1() { func2() }
diff --git a/test/Profiler/Inputs/coverage_num_threads4.swift b/test/Profiler/Inputs/coverage_num_threads4.swift
@@ -0,0 +1 @@
+public func func2() {}
diff --git a/test/Profiler/coverage_num_threads.swift b/test/Profiler/coverage_num_threads.swift
@@ -1,13 +1,40 @@
-// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 0 -emit-ir %S/Inputs/coverage_num_threads1.swift | %FileCheck %s -check-prefix=SINGLE-SOURCE --implicit-check-not="llvm_coverage_mapping ="
+// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 0 -emit-ir %S/Inputs/coverage_num_threads1.swift | %FileCheck %s -check-prefix=SINGLE-SOURCE --implicit-check-not="llvm_coverage_mapping =" --implicit-check-not="@__covrec_{{[a-zA-Z0-9]+}} ="
 
+// Make sure we only emit 1 coverage record in total.
+// SINGLE-SOURCE: @__covrec_{{[a-zA-Z0-9]+}} =
 // SINGLE-SOURCE: llvm_coverage_mapping =
 
-// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 0 -emit-ir %S/Inputs/coverage_num_threads1.swift %S/Inputs/coverage_num_threads2.swift | %FileCheck %s -check-prefix=SINGLE-OBJECT --implicit-check-not="llvm_coverage_mapping ="
+// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 0 -emit-ir %S/Inputs/coverage_num_threads1.swift %S/Inputs/coverage_num_threads2.swift | %FileCheck %s -check-prefix=SINGLE-OBJECT --implicit-check-not="llvm_coverage_mapping =" --implicit-check-not="@__covrec_{{[a-zA-Z0-9]+}} ="
 
+// Make sure we only emit 2 coverage records in total.
+// SINGLE-OBJECT: @__covrec_{{[a-zA-Z0-9]+}} =
+// SINGLE-OBJECT: @__covrec_{{[a-zA-Z0-9]+}} =
 // SINGLE-OBJECT: llvm_coverage_mapping =
 
 // Using 1 goes down the multithreaded codepath but only operates with a single thread to work around an issue on Windows where the output of both IR modules is interleaved and therefore the output is invalid
-// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 1 -emit-ir %S/Inputs/coverage_num_threads1.swift %S/Inputs/coverage_num_threads2.swift | %FileCheck %s -check-prefix=MULTIPLE-OBJECTS --implicit-check-not="llvm_coverage_mapping ="
+// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 1 -emit-ir %S/Inputs/coverage_num_threads1.swift %S/Inputs/coverage_num_threads2.swift | %FileCheck %s -check-prefix=MULTIPLE-OBJECTS --implicit-check-not="llvm_coverage_mapping =" --implicit-check-not="@__covrec_{{[a-zA-Z0-9]+}} ="
 
+// Make sure we only emit 2 coverage records in total (1 per output).
+// MULTIPLE-OBJECTS: @__covrec_{{[a-zA-Z0-9]+}} =
 // MULTIPLE-OBJECTS: llvm_coverage_mapping =
+// MULTIPLE-OBJECTS: ; ModuleID =
+// MULTIPLE-OBJECTS: @__covrec_{{[a-zA-Z0-9]+}} =
 // MULTIPLE-OBJECTS: llvm_coverage_mapping =
+
+// Under -O, we inline the profiler increment of func2 into func1. The coverage
+// mapping for func2 should still however be present only in its original file.
+// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 1 -O -emit-sil %S/Inputs/coverage_num_threads3.swift %S/Inputs/coverage_num_threads4.swift | %FileCheck %s -check-prefix=MULTIPLE-OBJECTS-INLINE-SIL
+// RUN: %target-swift-frontend -profile-generate -profile-coverage-mapping -num-threads 1 -O -emit-ir %S/Inputs/coverage_num_threads3.swift %S/Inputs/coverage_num_threads4.swift | %FileCheck %s -check-prefix=MULTIPLE-OBJECTS-INLINE --implicit-check-not="llvm_coverage_mapping =" --implicit-check-not="@__covrec_{{[a-zA-Z0-9]+}} ="
+
+// MULTIPLE-OBJECTS-INLINE-SIL-LABEL: sil @$s21coverage_num_threads35func1yyF
+// MULTIPLE-OBJECTS-INLINE-SIL:       increment_profiler_counter 0, "$s21coverage_num_threads35func1yyF"
+// MULTIPLE-OBJECTS-INLINE-SIL:       increment_profiler_counter 0, "$s21coverage_num_threads35func2yyF"
+
+// MULTIPLE-OBJECTS-INLINE-SIL-LABEL: sil @$s21coverage_num_threads35func2yyF
+// MULTIPLE-OBJECTS-INLINE-SIL:       increment_profiler_counter 0, "$s21coverage_num_threads35func2yyF"
+
+// MULTIPLE-OBJECTS-INLINE: @__covrec_{{[a-zA-Z0-9]+}} =
+// MULTIPLE-OBJECTS-INLINE: llvm_coverage_mapping =
+// MULTIPLE-OBJECTS-INLINE: ; ModuleID =
+// MULTIPLE-OBJECTS-INLINE: @__covrec_{{[a-zA-Z0-9]+}} =
+// MULTIPLE-OBJECTS-INLINE: llvm_coverage_mapping =
