[PGO]: Simplify coverage data lowering code

[davidxl]

Summary

The names referenced by the coverage data may be associated with functions that are never emitted by Clang. That means those PGO names won't be materialized into the __llvm_prf_names section during instr-prof lowering. To make sure those names are emitted, the lowering pass will need to go through the coverage map global variable and check the referenced names.

The lowering code makes assumption about the layout of the coverage map and function record data which is error prone. Besides, in the near future, when name compression is implemented, the name references won't be available from the coverage data anymore.

In this patch, the referenced names are explicitly recorded in an internal global var and passed to the middle end. This simplifies the lowering code and also make it possible to do name compression. This is part-2 of the patch. Part-1 is http://reviews.llvm.org/D15852

[davidxl]

Update patch to reduce overhead: Only record names for unused functions.

[vsk]

Fwiw the previous version of this patch looked good to me.

In this version 'getCoverageNamesVarName()' has a new meaning. It should probably be renamed (maybe 'getUnusedPGONamesVarName()'), and its comment should be updated.

I see that lowerCoverageData() sets the section information and alignment for names. With this change, it no longer handles records for which addFunctionMappingRecord(isUsed = true). It looks like these names will no longer be materialized into __llvm_prf_names. This doesn't seem right to me. Could you explain why it works? This code is a bit new to me...

lib/CodeGen/CoverageMappingGen.h

75

Nit: Parameter names should be capitalized.

[davidxl]

I missed the review comments earlier.

The local var name is fixed. The getCoverageNamesVarName API name will be fixed later.

Regarding to your question: the patch does not not change the behavior of the name handling. For all used functions, their names are handled in regular lowering procedure (getOrCreateRegionCounter) so their names will be skipped by lowerCoverageData actually.

[vsk]

Done in clang/r257092

[vsk]

Closing old review

lib/CodeGen/CodeGenPGO.cpp

CodeGenPGO::emitEmptyCounterMapping(const Decl *D, StringRef Name,
   MappingGen.emitEmptyMapping(D, OS);
   OS.flush();
 
   if (CoverageMapping.empty())
     return;
 
   setFuncName(Name, Linkage);
   CGM.getCoverageMapping()->addFunctionMappingRecord(
-      FuncNameVar, FuncName, FunctionHash, CoverageMapping);
+      FuncNameVar, FuncName, FunctionHash, CoverageMapping, false);
 }
 
 void CodeGenPGO::computeRegionCounts(const Decl *D) {
   StmtCountMap.reset(new llvm::DenseMap<const Stmt *, uint64_t>);
   ComputeRegionCounts Walker(*StmtCountMap, *this);
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
     Walker.VisitFunctionDecl(FD);
   else if (const ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(D))

lib/CodeGen/CoverageMappingGen.h

 
 /// \brief Organizes the cross-function state that is used while generating
 /// code coverage mapping data.
 class CoverageMappingModuleGen {
   CodeGenModule &CGM;
   CoverageSourceInfo &SourceInfo;
   llvm::SmallDenseMap<const FileEntry *, unsigned, 8> FileEntries;
   std::vector<llvm::Constant *> FunctionRecords;
+  std::vector<llvm::Constant *> FunctionNames;
   llvm::StructType *FunctionRecordTy;
   std::string CoverageMappings;
 
 public:
   CoverageMappingModuleGen(CodeGenModule &CGM, CoverageSourceInfo &SourceInfo)
       : CGM(CGM), SourceInfo(SourceInfo), FunctionRecordTy(nullptr) {}
 
   CoverageSourceInfo &getSourceInfo() const {
     return SourceInfo;
   }
 
   /// \brief Add a function's coverage mapping record to the collection of the
   /// function mapping records.
   void addFunctionMappingRecord(llvm::GlobalVariable *FunctionName,
                                 StringRef FunctionNameValue,
                                 uint64_t FunctionHash,
-                                const std::string &CoverageMapping);
+                                const std::string &CoverageMapping,
+                                bool isUsed = true);
 
   /// \brief Emit the coverage mapping data for a translation unit.
   void emit();
 
   /// \brief Return the coverage mapping translation unit file id
   /// for the given file.
   unsigned getFileID(const FileEntry *File);
 };

lib/CodeGen/CoverageMappingGen.cpp

for (const auto &R : Regions) {
     Ctx.dump(R.Count, OS);
     if (R.Kind == CounterMappingRegion::ExpansionRegion)
       OS << " (Expanded file = " << R.ExpandedFileID << ")";
     OS << "\n";
   }
 }
 
 void CoverageMappingModuleGen::addFunctionMappingRecord(
-    llvm::GlobalVariable *NamePtr, StringRef NameValue,
-    uint64_t FuncHash, const std::string &CoverageMapping) {
+    llvm::GlobalVariable *NamePtr, StringRef NameValue, uint64_t FuncHash,
+    const std::string &CoverageMapping, bool isUsed) {
   llvm::LLVMContext &Ctx = CGM.getLLVMContext();
   if (!FunctionRecordTy) {
-    #define COVMAP_FUNC_RECORD(Type, LLVMType, Name, Init) LLVMType,
     llvm::Type *FunctionRecordTypes[] = {
       #include "llvm/ProfileData/InstrProfData.inc"
     };
     FunctionRecordTy =
         llvm::StructType::get(Ctx, makeArrayRef(FunctionRecordTypes),
                               /*isPacked=*/true);
   }
 
   #define COVMAP_FUNC_RECORD(Type, LLVMType, Name, Init) Init,
   llvm::Constant *FunctionRecordVals[] = {
       #include "llvm/ProfileData/InstrProfData.inc"
   };
   FunctionRecords.push_back(llvm::ConstantStruct::get(
       FunctionRecordTy, makeArrayRef(FunctionRecordVals)));
+  if (!isUsed)
+    FunctionNames.push_back(
+        llvm::ConstantExpr::getBitCast(NamePtr, llvm::Type::getInt8PtrTy(Ctx)));
   CoverageMappings += CoverageMapping;
 
   if (CGM.getCodeGenOpts().DumpCoverageMapping) {
     // Dump the coverage mapping data for this function by decoding the
     // encoded data. This allows us to dump the mapping regions which were
     // also processed by the CoverageMappingWriter which performs
     // additional minimization operations such as reducing the number of
     // expressions.
auto CovData = new llvm::GlobalVariable(
       CGM.getModule(), CovDataTy, true, llvm::GlobalValue::InternalLinkage,
       CovDataVal, llvm::getCoverageMappingVarName());
 
   CovData->setSection(getCoverageSection(CGM));
   CovData->setAlignment(8);
 
   // Make sure the data doesn't get deleted.
   CGM.addUsedGlobal(CovData);
+  // Create the deferred function records array
+  if (!FunctionNames.empty()) {
+    auto NamesArrTy = llvm::ArrayType::get(llvm::Type::getInt8PtrTy(Ctx),
+                                           FunctionNames.size());
+    auto NamesArrVal = llvm::ConstantArray::get(NamesArrTy, FunctionNames);
+    // This variable will *NOT* be emitted to the object file. It is used
+    // to pass the list of names referenced to codegen.
+    new llvm::GlobalVariable(CGM.getModule(), NamesArrTy, true,
+                             llvm::GlobalValue::InternalLinkage, NamesArrVal,
+                             llvm::getCoverageNamesVarName());
+  }
 }
 
 unsigned CoverageMappingModuleGen::getFileID(const FileEntry *File) {
   auto It = FileEntries.find(File);
   if (It != FileEntries.end())
     return It->second;
   unsigned FileID = FileEntries.size();
   FileEntries.insert(std::make_pair(File, FileID));