[llvm-profgen] Fix bug of populating profile symbol list

[wlei]

Summary

Previous implementation of populating profile symbol list is wrong, it only included the profiled symbols. Actually it should use all symbols, here this switches to use the symbols from debug info. Also turned the flag off by default.

[hoy]

Thanks for the fix. The switch flipping makes sense to me. It enables the consistency between CS and non-CS profile.

[wenlei]

llvm/tools/llvm-profgen/ProfiledBinary.cpp

557

How do we deal with symbols without address range (getAddressRanges)? The old tool skips those, and this seems different. I'm not sure when we would have a function without address range though.

[wlei]

rebase on https://reviews.llvm.org/D112282

[wlei]

llvm/tools/llvm-profgen/ProfiledBinary.cpp

557

Good catch, thanks. After rebased on https://reviews.llvm.org/D112282, this should align with our internal tool

[wenlei]

lgtm, thanks.

[wlei]

rebase

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-SYM-LIST: main
 
 ; 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/artificial-branch.perfscript --binary=%S/Inputs/inline-noprobe2.perfbin --output=%t  --skip-symbolization --use-offset=0
 ; RUN: FileCheck %s --input-file %t --check-prefix=CHECK-ARTIFICIAL-BRANCH
 ; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/inline-noprobe2.perfscript --binary=%S/Inputs/inline-noprobe2.perfbin --output=%t  --skip-symbolization --use-offset=0
 ; RUN: FileCheck %s --input-file %t --check-prefix=CHECK-RAW-PROFILE
 ; RUN: llvm-profgen --format=text --unsymbolized-profile=%t --binary=%S/Inputs/inline-noprobe2.perfbin --output=%t1 --use-offset=0
 ; RUN: FileCheck %s --input-file %t1 --check-prefix=CHECK
 
-; RUN: llvm-profgen --format=extbinary --perfscript=%S/Inputs/inline-noprobe2.perfscript --binary=%S/Inputs/inline-noprobe2.perfbin --output=%t
+; RUN: llvm-profgen --format=extbinary --perfscript=%S/Inputs/inline-noprobe2.perfscript --binary=%S/Inputs/inline-noprobe2.perfbin --output=%t --populate-profile-symbol-list=1
 ; RUN: llvm-profdata show -show-prof-sym-list -sample %t | FileCheck %s --check-prefix=CHECK-SYM-LIST
 
 ; CHECK-ARTIFICIAL-BRANCH: 3
 ; CHECK-ARTIFICIAL-BRANCH: 400540-400540:1
 ; CHECK-ARTIFICIAL-BRANCH: 400870-400870:2
 ; CHECK-ARTIFICIAL-BRANCH: 400875-4008bf:1
 ; CHECK-ARTIFICIAL-BRANCH: 2
 ; CHECK-ARTIFICIAL-BRANCH: 400870->400540:2
 ; CHECK-ARTIFICIAL-BRANCH: 4008bf->400870:2
 
 ; 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-SYM-LIST: swap
 
 
 ;CHECK-RAW-PROFILE-NOT: 7f7448e889e4
 ;CHECK-RAW-PROFILE-NOT: 7f7448e88826
 
 ;CHECK:       partition_pivot_first:1045:5
 ;CHECK-NEXT:  0: 5
 ;CHECK-NEXT:  1: 5

llvm/tools/llvm-profgen/ProfileGenerator.cpp

cl::values(
                    "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,
+    "populate-profile-symbol-list", cl::init(false), 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));
ProfileGeneratorBase::create(ProfiledBinary *Binary,
   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);
-    }
+    Binary->populateSymbolListFromDWARF(SymbolList);
     Writer->setProfileSymbolList(&SymbolList);
   }
 
   if (std::error_code EC = Writer->write(ProfileMap))
     exitWithError(std::move(EC));
 }
 
 void ProfileGeneratorBase::write() {

llvm/tools/llvm-profgen/ProfiledBinary.h

RangesTy getRangesForOffset(uint64_t Offset) {
 
     return FRange->Func->Ranges;
   }
 
   uint32_t getFuncSizeForContext(SampleContext &Context) {
     return FuncSizeTracker.getFuncSizeForContext(Context);
   }
 
+  // Load the symbols from debug table and populate into symbol list.
+  void populateSymbolListFromDWARF(ProfileSymbolList &SymbolList);
+
   const SampleContextFrameVector &
   getFrameLocationStack(uint64_t Offset, bool UseProbeDiscriminator = false) {
     auto I = Offset2LocStackMap.emplace(Offset, SampleContextFrameVector());
     if (I.second) {
       InstructionPointer IP(this, Offset);
       I.first->second = symbolize(IP, true, UseProbeDiscriminator);
     }
     return I.first->second;

llvm/tools/llvm-profgen/ProfiledBinary.cpp

 //===-- ProfiledBinary.cpp - Binary decoder ---------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ProfiledBinary.h"
 #include "ErrorHandling.h"
 #include "ProfileGenerator.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/TargetSelect.h"
 
 #define DEBUG_TYPE "load-binary"
for (const auto &DieInfo : CompilationUnit->dies()) {
       auto Name = Die.getName(llvm::DINameKind::LinkageName);
       if (!Name)
         Name = Die.getName(llvm::DINameKind::ShortName);
       if (!Name)
         continue;
 
       auto RangesOrError = Die.getAddressRanges();
       if (!RangesOrError)
         continue;
       const DWARFAddressRangesVector &Ranges = RangesOrError.get();
 
       if (Ranges.empty())
         continue;
 
       // Different DWARF symbols can have same function name, search or create
       // BinaryFunction indexed by the name.
       auto Ret = BinaryFunctions.emplace(Name, BinaryFunction());
for (const auto &DieInfo : CompilationUnit->dies()) {
               << "\n";
         }
       }
     }
   }
   assert(!StartOffset2FuncRangeMap.empty() && "Misssing debug info.");
 }
 
+void ProfiledBinary::populateSymbolListFromDWARF(
+    ProfileSymbolList &SymbolList) {
+  for (auto &I : StartOffset2FuncRangeMap)
+    SymbolList.add(I.second.getFuncName());
+}
+
 void ProfiledBinary::setupSymbolizer() {
   symbolize::LLVMSymbolizer::Options SymbolizerOpts;
   SymbolizerOpts.PrintFunctions =
       DILineInfoSpecifier::FunctionNameKind::LinkageName;
   SymbolizerOpts.Demangle = false;
   SymbolizerOpts.DefaultArch = TheTriple.getArchName().str();
   SymbolizerOpts.UseSymbolTable = false;
   SymbolizerOpts.RelativeAddresses = false;