[llvm-profgen] Support symbol list for accurate profile

[wlei]

Summary

Support symbol list in profile generation along with extended binary format.

[hoy]

Thanks for bringing this up. Can you make this optional under a switch? Should be on by default. Currently without a switch we have to go a round trip (ext->txt->ext) to get rid of the symbol list in an extbinary profile.

llvm/tools/llvm-profgen/ProfileGenerator.cpp

128

Move this into the if block above?

[wlei]

add a switch

[wlei]

In D110859#3037131, @hoy wrote:

Thanks for bringing this up. Can you make this optional under a switch? Should be on by default. Currently without a switch we have to go a round trip (ext->txt->ext) to get rid of the symbol list in an extbinary profile.

I see, thanks for pointing out, switch added.

llvm/tools/llvm-profgen/ProfileGenerator.cpp

128

Fixed!

[hoy]

LGTM. We probably need this for CS profile too.

llvm/tools/llvm-profgen/ProfileGenerator.cpp

121

This can be moved into the if block too.

[wlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

121

SymbolList is used inside Writer.write(ProfileMap) as a pointer, moving it into if block will causing the dangling pointer issue.

[hoy]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

121

I see. Good point.

[wenlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

117

Looking at the implementation, seems it should work for CS profile too? Profile.getName() only gets the leaf name instead of full context name, we just need to deduplicate the leaf name for CS profile. So I'm wondering if we can move it to ProfileGeneratorBase::write so it covers both CS and non-CS profile?

[wlei]

add support for CS profile generation

[wlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

117

Sounds good, added to support CS profile.

[hoy]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

35

On the second thought, do we want the default value of PopulateProfileSymbolList to be true or false? We probably do not want the list for either CS or non-CS in practice for now.

101

Use const FunctionSamples& ?

[wlei]

addressing feedback

[wlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

35

I checked that our internal tools make it on by default for non-CS. But we haven't tested for CS profile.

Or we can make it off for CS util we tested it and then we can turn on?

101

Good catch, thanks!

[wenlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

35

Leave it default on, but turn it off for CS profile unless explicitly specified sounds good to me.

[wlei]

made it off for CS profile.

[wenlei]

lgtm, thanks.

[hoy]

lgtm.

llvm/test/tools/llvm-profgen/inline-cs-noprobe.test

 ; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/inline-cs-noprobe.perfscript --binary=%S/Inputs/inline-cs-noprobe.perfbin --output=%t --skip-symbolization --profile-summary-cold-count=0
 ; RUN: FileCheck %s --input-file %t --check-prefix=CHECK-UNWINDER
 ; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/inline-cs-noprobe.perfscript --binary=%S/Inputs/inline-cs-noprobe.perfbin --output=%t --profile-summary-cold-count=0
 ; RUN: FileCheck %s --input-file %t
 ; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/inline-cs-noprobe.perfscript --binary=%S/Inputs/inline-cs-noprobe.perfbin --output=%t --profile-summary-cold-count=0 --ignore-stack-samples
 ; RUN: FileCheck %s --input-file %t --check-prefix=CHECK-STRIP-CTX
 
+; RUN: llvm-profgen --format=extbinary --perfscript=%S/Inputs/inline-cs-noprobe.perfscript --binary=%S/Inputs/inline-cs-noprobe.perfbin --output=%t --profile-summary-cold-count=0 --populate-profile-symbol-list=1
+; RUN: llvm-profdata show -show-prof-sym-list -sample %t | FileCheck %s --check-prefix=CHECK-SYM-LIST
+
+; CHECK-SYM-LIST: Dump profile symbol list
+; CHECK-SYM-LIST: bar
+; CHECK-SYM-LIST: foo
+
 ; CHECK:[main:1 @ foo]:225:0
 ; CHECK: 2.1: 14
 ; CHECK: 3: 15
 ; CHECK: 3.1: 14 bar:14
 ; CHECK: 3.2: 1
 ; CHECK:[main:1 @ foo:3.1 @ bar]:84:0
 ; CHECK: 1: 14
 

llvm/test/tools/llvm-profgen/inline-noprobe2.test

 
 ; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/inline-noprobe2.perfscript --binary=%S/Inputs/inline-noprobe2.perfbin --output=%t
 ; RUN: FileCheck %s --input-file %t --check-prefix=CHECK
 
+; RUN: llvm-profgen --format=extbinary --perfscript=%S/Inputs/inline-noprobe2.perfscript --binary=%S/Inputs/inline-noprobe2.perfbin --output=%t
+; RUN: llvm-profdata show -show-prof-sym-list -sample %t | FileCheck %s --check-prefix=CHECK-SYM-LIST
+
+; CHECK-SYM-LIST: Dump profile symbol list
+; CHECK-SYM-LIST: main
+; CHECK-SYM-LIST: partition_pivot_first
+; CHECK-SYM-LIST: partition_pivot_last
+; CHECK-SYM-LIST: quick_sort
+
 ;CHECK:       partition_pivot_first:1045:5
 ;CHECK-NEXT:  0: 5
 ;CHECK-NEXT:  1: 5
 ;CHECK-NEXT:  2: 5
 ;CHECK-NEXT:  3: 5
 ;CHECK-NEXT:  3.1: 83
 ;CHECK-NEXT:  4: 82
 ;CHECK-NEXT:  4.1: 26

llvm/tools/llvm-profgen/ProfileGenerator.cpp

cl::values(
         clEnumValN(SPF_GCC, "gcc",
                    "GCC encoding (only meaningful for -sample)")));
 
 cl::opt<bool> UseMD5(
     "use-md5", cl::init(false), cl::Hidden,
     cl::desc("Use md5 to represent function names in the output profile (only "
              "meaningful for -extbinary)"));
 
+static cl::opt<bool> PopulateProfileSymbolList(
+    "populate-profile-symbol-list", cl::init(true), cl::Hidden,
+    cl::desc("Populate profile symbol list (only meaningful for -extbinary)"));
+
 static cl::opt<int32_t, true> RecursionCompression(
     "compress-recursion",
     cl::desc("Compressing recursion by deduplicating adjacent frame "
              "sequences up to the specified size. -1 means no size limit."),
     cl::Hidden,
     cl::location(llvm::sampleprof::CSProfileGenerator::MaxCompressionSize));
 
 static cl::opt<bool> CSProfMergeColdContext(
if (ProfileIsCS) {
     Generator.reset(new ProfileGenerator(Binary, SampleCounters));
   }
 
   return Generator;
 }
 
 void ProfileGeneratorBase::write(std::unique_ptr<SampleProfileWriter> Writer,
                                  SampleProfileMap &ProfileMap) {
+  // Populate profile symbol list if extended binary format is used.
+  ProfileSymbolList SymbolList;
+
+  // Turn it off temporarily for CS profile.
+  if (FunctionSamples::ProfileIsCS &&
+      !PopulateProfileSymbolList.getNumOccurrences())
+    PopulateProfileSymbolList = false;
+
+  if (PopulateProfileSymbolList && OutputFormat == SPF_Ext_Binary) {
+    for (const auto &Item : ProfileMap) {
+      auto &Profile = Item.second;
+      SymbolList.add(Profile.getName(), true);
+    }
+    Writer->setProfileSymbolList(&SymbolList);
+  }
+
   if (std::error_code EC = Writer->write(ProfileMap))
     exitWithError(std::move(EC));
 }
 
 void ProfileGeneratorBase::write() {
   auto WriterOrErr = SampleProfileWriter::create(OutputFilename, OutputFormat);
   if (std::error_code EC = WriterOrErr.getError())
     exitWithError(EC, OutputFilename);
 
   if (UseMD5) {
     if (OutputFormat != SPF_Ext_Binary)
       WithColor::warning() << "-use-md5 is ignored. Specify "
                               "--format=extbinary to enable it\n";
     else
       WriterOrErr.get()->setUseMD5();
   }
 
   write(std::move(WriterOrErr.get()), ProfileMap);
 }
 
 void ProfileGeneratorBase::findDisjointRanges(RangeSample &DisjointRanges,
                                               const RangeSample &Ranges) {
 
   /*