[Support] Extended llvm-profdata's merge functionality to exclude profiles from functions matching configurable patterns

[nilanjana_basu]

Summary

For debugging PGO performance, it is useful to have a facility to filter out profiles at a function-level granularity while merging. This will help in checking the impact of a set of function profiles on performance.

[nilanjana_basu]

Added error reporting for options that are not yet supported with -exclude-function option

[nilanjana_basu]

minor cosmetic changes

[paquette]

llvm/test/tools/llvm-profdata/merge-filtering.test

58

I'd expect the regular expression here to be something like foo.*, not foo?

llvm/tools/llvm-profdata/llvm-profdata.cpp

338

To me, this looks like it's matching a substring, not a regex.

/// Return true if the given string is a substring of *this, and false
/// otherwise.
[[nodiscard]] bool contains(StringRef Other) const {
  return find(Other) != npos;
}

I'd expect this to use the utilities from llvm/include/llvm/Support/Regex.h

345

clang-format should remove these braces

1280

I think you can de Morgan this:

"Exclude functions whose names match any regex in a semicolon-separated list"

[nilanjana_basu]

Updated function pattern to use Regex, and added corresponding tests.

llvm/test/tools/llvm-profdata/merge-filtering.test

58

Added new cases for regex.

llvm/tools/llvm-profdata/llvm-profdata.cpp

345

It didn't, but I removed it manually.

[nilanjana_basu]

ping!

[paquette]

Do we need to keep the comma-separated list now that we're using a regex?

[nilanjana_basu]

Removed redundant feature of semicolon-separated exclude patterns, since regex can handle that

[nilanjana_basu]

In D147285#4316538, @paquette wrote:

Do we need to keep the comma-separated list now that we're using a regex?

It was left over by mistake. Thanks for spotting it. Removed it.

[nilanjana_basu]

ping!

[nilanjana_basu]

ping!

[Gerolf]

llvm/tools/llvm-profdata/llvm-profdata.cpp

1314

The check above is for !SuppInstrWithSample, so this code could issue a wrong error message.

[nilanjana_basu]

llvm/tools/llvm-profdata/llvm-profdata.cpp

1314

The check above is for !SupplInstrWithSample.empty(). I don't see how this is a wrong error message, since exclude-function functionality is not supported with the -supplement-instr-with-sample option.

[nilanjana_basu]

llvm/tools/llvm-profdata/llvm-profdata.cpp

1314

@Gerolf : If I misunderstood the issue, can you elaborate on this?

llvm/test/tools/llvm-profdata/merge-filtering.test

+# Tests for merge of profile files with filtering capabilities
+
+# no filtering
+RUN: llvm-profdata merge --text %p/Inputs/basic.proftext | FileCheck %s
+
+CHECK: foo
+CHECK-NEXT: # Func Hash:
+CHECK-NEXT: 10
+CHECK-NEXT: # Num Counters:
+CHECK-NEXT: 2
+CHECK-NEXT: # Counter Values:
+CHECK-NEXT: 499500
+CHECK-NEXT: 179900
+CHECK: foo2
+CHECK-NEXT: # Func Hash:
+CHECK-NEXT: 10
+CHECK-NEXT: # Num Counters:
+CHECK-NEXT: 2
+CHECK-NEXT: # Counter Values:
+CHECK-NEXT: 500500
+CHECK-NEXT: 180100
+CHECK: main
+CHECK-NEXT: # Func Hash:
+CHECK-NEXT: 16650
+CHECK-NEXT: # Num Counters:
+CHECK-NEXT: 4
+CHECK-NEXT: # Counter Values:
+CHECK-NEXT: 1
+CHECK-NEXT: 1000
+CHECK-NEXT: 1000000
+CHECK-NEXT: 499500
+
+# excluding profile for one pattern matching one function
+RUN: llvm-profdata merge --exclude-function "foo2" --text %p/Inputs/basic.proftext -o - | FileCheck %s --check-prefix=MERGE-EXCL-SINGLE-FUNC
+
+MERGE-EXCL-SINGLE-FUNC-LABEL: foo
+MERGE-EXCL-SINGLE-FUNC-NEXT: # Func Hash:
+MERGE-EXCL-SINGLE-FUNC-NEXT: 10
+MERGE-EXCL-SINGLE-FUNC-NEXT: # Num Counters:
+MERGE-EXCL-SINGLE-FUNC-NEXT: 2
+MERGE-EXCL-SINGLE-FUNC-NEXT: # Counter Values:
+MERGE-EXCL-SINGLE-FUNC-NEXT: 499500
+MERGE-EXCL-SINGLE-FUNC-NEXT: 179900
+MERGE-EXCL-SINGLE-FUNC-NOT: foo2
+MERGE-EXCL-SINGLE-FUNC-LABEL: main
+MERGE-EXCL-SINGLE-FUNC-NEXT: # Func Hash:
+MERGE-EXCL-SINGLE-FUNC-NEXT: 16650
+MERGE-EXCL-SINGLE-FUNC-NEXT: # Num Counters:
+MERGE-EXCL-SINGLE-FUNC-NEXT: 4
+MERGE-EXCL-SINGLE-FUNC-NEXT: # Counter Values:
+MERGE-EXCL-SINGLE-FUNC-NEXT: 1
+MERGE-EXCL-SINGLE-FUNC-NEXT: 1000
+MERGE-EXCL-SINGLE-FUNC-NEXT: 1000000
+MERGE-EXCL-SINGLE-FUNC-NEXT: 499500
+
+# excluding profile for one pattern matching multiple functions
+RUN: llvm-profdata merge --exclude-function "foo" --text %p/Inputs/basic.proftext -o - | FileCheck %s --check-prefix=MERGE-EXCL-SINGLE-PAT
+MERGE-EXCL-SINGLE-PAT-NOT: foo
+MERGE-EXCL-SINGLE-PAT-NOT: foo2
+MERGE-EXCL-SINGLE-PAT: main
+MERGE-EXCL-SINGLE-PAT-NEXT: # Func Hash:
+MERGE-EXCL-SINGLE-PAT-NEXT: 16650
+MERGE-EXCL-SINGLE-PAT-NEXT: # Num Counters:
+MERGE-EXCL-SINGLE-PAT-NEXT: 4
+MERGE-EXCL-SINGLE-PAT-NEXT: # Counter Values:
+MERGE-EXCL-SINGLE-PAT-NEXT: 1
+MERGE-EXCL-SINGLE-PAT-NEXT: 1000
+MERGE-EXCL-SINGLE-PAT-NEXT: 1000000
+MERGE-EXCL-SINGLE-PAT-NEXT: 499500
+
+# excluding profile for multiple patterns matching multiple functions
+RUN: llvm-profdata merge --exclude-function "foo2;main" --text %p/Inputs/basic.proftext -o - | FileCheck %s --check-prefix=MERGE-EXCL-MULT-PAT
+MERGE-EXCL-MULT-PAT: foo
+MERGE-EXCL-MULT-PAT-NEXT: # Func Hash:
+MERGE-EXCL-MULT-PAT-NEXT: 10
+MERGE-EXCL-MULT-PAT-NEXT: # Num Counters:
+MERGE-EXCL-MULT-PAT-NEXT: 2
+MERGE-EXCL-MULT-PAT-NEXT: # Counter Values:
+MERGE-EXCL-MULT-PAT-NEXT: 499500
+MERGE-EXCL-MULT-PAT-NEXT: 179900
+MERGE-EXCL-MULT-PAT-NOT: foo2
+MERGE-EXCL-MULT-PAT-NOT: main
+
+# excluding profile for multiple patterns matching multiple functions
+RUN: llvm-profdata merge --exclude-function "foo2;main;" --text %p/Inputs/basic.proftext -o - | FileCheck %s --check-prefix=DANGLING-DELIMITER
+DANGLING-DELIMITER: foo
+DANGLING-DELIMITER-NEXT: # Func Hash:
+DANGLING-DELIMITER-NEXT: 10
+DANGLING-DELIMITER-NEXT: # Num Counters:
+DANGLING-DELIMITER-NEXT: 2
+DANGLING-DELIMITER-NEXT: # Counter Values:
+DANGLING-DELIMITER-NEXT: 499500
+DANGLING-DELIMITER-NEXT: 179900
+DANGLING-DELIMITER-NOT: foo2
+DANGLING-DELIMITER-NOT: main

llvm/tools/llvm-profdata/llvm-profdata.cpp

for (auto &I : *Reader) {
     WC->Writer.overlapRecord(std::move(I), Overlap, FuncOverlap, FuncFilter);
     FuncOverlap.dump(OS);
   }
 }
 
 /// Load an input into a writer context.
 static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
                       const InstrProfCorrelator *Correlator,
-                      const StringRef ProfiledBinary, WriterContext *WC) {
+                      const StringRef ProfiledBinary, WriterContext *WC,
+                      const StringRef ExcludeFuncList) {
   std::unique_lock<std::mutex> CtxGuard{WC->Lock};
 
   // Copy the filename, because llvm::ThreadPool copied the input "const
   // WeightedFile &" by value, making a reference to the filename within it
   // invalid outside of this packaged task.
   std::string Filename = Input.Filename;
 
   using ::llvm::memprof::RawMemProfReader;
if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
     return;
   }
 
   for (auto &I : *Reader) {
     if (Remapper)
       I.Name = (*Remapper)(I.Name);
     const StringRef FuncName = I.Name;
     bool Reported = false;
+
+    bool ToBeExcluded = false;
+    std::string ExcludeFuncPat;
+    if (!ExcludeFuncList.empty()) {
+      std::stringstream ExcludeFuncSS(ExcludeFuncList.str());
+      while (ExcludeFuncSS.good()) {
+        getline(ExcludeFuncSS, ExcludeFuncPat, ';');
+        if (!ExcludeFuncPat.empty() && FuncName.contains(ExcludeFuncPat)) {
+          ToBeExcluded = true;
+          break;
+        }
+      }
+    }
+
+    if (ToBeExcluded) {
+      continue;
+    }
+
     WC->Writer.addRecord(std::move(I), Input.Weight, [&](Error E) {
       if (Reported) {
         consumeError(std::move(E));
         return;
       }
       Reported = true;
       // Only show hint the first time an error occurs.
       instrprof_error IPE = InstrProfError::take(std::move(E));

 }
 
 static void mergeInstrProfile(const WeightedFileVector &Inputs,
                               StringRef DebugInfoFilename,
                               SymbolRemapper *Remapper,
                               StringRef OutputFilename,
                               ProfileFormat OutputFormat, bool OutputSparse,
                               unsigned NumThreads, FailureMode FailMode,
-                              const StringRef ProfiledBinary) {
+                              const StringRef ProfiledBinary,
+                              const std::string &ExcludeFuncList) {
   if (OutputFormat != PF_Binary && OutputFormat != PF_Compact_Binary &&
       OutputFormat != PF_Ext_Binary && OutputFormat != PF_Text)
     exitWithError("unknown format is specified");
 
   std::unique_ptr<InstrProfCorrelator> Correlator;
   if (!DebugInfoFilename.empty()) {
     if (auto Err =
             InstrProfCorrelator::get(DebugInfoFilename).moveInto(Correlator))
static void mergeInstrProfile(const WeightedFileVector &Inputs,
   SmallVector<std::unique_ptr<WriterContext>, 4> Contexts;
   for (unsigned I = 0; I < NumThreads; ++I)
     Contexts.emplace_back(std::make_unique<WriterContext>(
         OutputSparse, ErrorLock, WriterErrorCodes));
 
   if (NumThreads == 1) {
     for (const auto &Input : Inputs)
       loadInput(Input, Remapper, Correlator.get(), ProfiledBinary,
-                Contexts[0].get());
+                Contexts[0].get(), ExcludeFuncList);
   } else {
     ThreadPool Pool(hardware_concurrency(NumThreads));
 
     // Load the inputs in parallel (N/NumThreads serial steps).
     unsigned Ctx = 0;
     for (const auto &Input : Inputs) {
       Pool.async(loadInput, Input, Remapper, Correlator.get(), ProfiledBinary,
-                 Contexts[Ctx].get());
+                 Contexts[Ctx].get(), ExcludeFuncList);
       Ctx = (Ctx + 1) % NumThreads;
     }
     Pool.wait();
 
     // Merge the writer contexts together (~ lg(NumThreads) serial steps).
     unsigned Mid = Contexts.size() / 2;
     unsigned End = Contexts.size();
     assert(Mid > 0 && "Expected more than one context");
static void supplementInstrProfile(
   if (std::error_code EC = Reader->read())
     exitWithErrorCode(EC, SampleFilename);
 
   // Read instr profile.
   std::mutex ErrorLock;
   SmallSet<instrprof_error, 4> WriterErrorCodes;
   auto WC = std::make_unique<WriterContext>(OutputSparse, ErrorLock,
                                             WriterErrorCodes);
-  loadInput(Inputs[0], nullptr, nullptr, /*ProfiledBinary=*/"", WC.get());
+  loadInput(Inputs[0], nullptr, nullptr, /*ProfiledBinary=*/"", WC.get(), "");
   if (WC->Errors.size() > 0)
     exitWithError(std::move(WC->Errors[0].first), InstrFilename);
 
   adjustInstrProfile(WC, Reader, SupplMinSizeThreshold, ZeroCounterThreshold,
                      InstrProfColdThreshold);
   writeInstrProfile(OutputFilename, OutputFormat, WC->Writer);
 }
 
cl::opt<std::string> DebugInfoFilename(
       cl::desc("Use the provided debug info to correlate the raw profile."));
   cl::opt<std::string> ProfiledBinary(
       "profiled-binary", cl::init(""),
       cl::desc("Path to binary from which the profile was collected."));
   cl::opt<bool> DropProfileSymbolList(
       "drop-profile-symbol-list", cl::init(false), cl::Hidden,
       cl::desc("Drop the profile symbol list when merging AutoFDO profiles "
                "(only meaningful for -sample)"));
+  cl::opt<std::string> ExcludeFuncList(
+      "exclude-function",
+      cl::desc("Include profiles from only those functions where names don't "
+               "match all the regexes separated by a semi-colon"));
 
   cl::ParseCommandLineOptions(argc, argv, "LLVM profile data merger\n");
 
   WeightedFileVector WeightedInputs;
   for (StringRef Filename : InputFilenames)
     addWeightedInput(WeightedInputs, {std::string(Filename), 1});
   for (StringRef WeightedFilename : WeightedInputFilenames)
     addWeightedInput(WeightedInputs, parseWeightedFile(WeightedFilename));
static int merge_main(int argc, const char *argv[]) {
   if (!RemappingFile.empty())
     Remapper = SymbolRemapper::create(RemappingFile);
 
   if (!SupplInstrWithSample.empty()) {
     if (ProfileKind != instr)
       exitWithError(
           "-supplement-instr-with-sample can only work with -instr. ");
 
     supplementInstrProfile(WeightedInputs, SupplInstrWithSample, OutputFilename,
                            OutputFormat, OutputSparse, SupplMinSizeThreshold,
                            ZeroCounterThreshold, InstrProfColdThreshold);
     return 0;
   }
 
   if (ProfileKind == instr)
     mergeInstrProfile(WeightedInputs, DebugInfoFilename, Remapper.get(),
                       OutputFilename, OutputFormat, OutputSparse, NumThreads,
-                      FailureMode, ProfiledBinary);
+                      FailureMode, ProfiledBinary, ExcludeFuncList);
   else
     mergeSampleProfile(
         WeightedInputs, Remapper.get(), OutputFilename, OutputFormat,
         ProfileSymbolListFile, CompressAllSections, UseMD5, GenPartialProfile,
         GenCSNestedProfile, SampleMergeColdContext, SampleTrimColdContext,
         SampleColdContextFrameDepth, FailureMode, DropProfileSymbolList,
         OutputSizeLimit);
   return 0;
static void overlapInstrProfile(const std::string &BaseFilename,
   if (Overlap.Base.CountSum < 1.0f) {
     OS << "Sum of edge counts for profile " << BaseFilename << " is 0.\n";
     exit(0);
   }
   if (Overlap.Test.CountSum < 1.0f) {
     OS << "Sum of edge counts for profile " << TestFilename << " is 0.\n";
     exit(0);
   }
-  loadInput(WeightedInput, nullptr, nullptr, /*ProfiledBinary=*/"", &Context);
+  loadInput(WeightedInput, nullptr, nullptr, /*ProfiledBinary=*/"", &Context,
+            "");
   overlapInput(BaseFilename, TestFilename, &Context, Overlap, FuncFilter, OS,
                IsCS);
   Overlap.dump(OS);
 }
 
 namespace {
 struct SampleOverlapStats {
   SampleContext BaseName;