[InstrProf][Correlate] Verify debug info with llvm-profdata show

[ellis]

Summary

Use the llvm-profdata show command to verify debug info for profile correlation using the --debug-info option.

[ellis]

Working

[ellis]

Display counters section size

[ellis]

Add comment

[kyulee]

compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c

2 ↗

(On Diff #403073)

It appears there are many similar names, which is confusing to remember.
The compilation uses -debug-info-correlate.
For llvm-profdata merge, use -debug-info=<debug file>.
Now, llvm-profdata show use -debug-info-correlation. This usage actually specifies <debug file> in the place of <profile file> which is a required argument to llvm-profdata show.
One thing I can think about is to use llvm-profdata show -debug-info=<debug file> while allowing to omit the required <profile file>, which becomes optional. But this doesn't look ideal, either.
I don't have a strong suggestion, but I wish some more consistent naming.

[phosek]

compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c

2 ↗

(On Diff #403073)

I agree and in particular I'd prefer the last suggestion, that is making <profile file> optional when -debug-info=<debug file> is specified. I think that changing the meaning of the positional argument (that is either a profile or a binary) depending on other arguments is more confusing.

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

2567

Nit: spelling.

[ellis]

compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c

2 ↗

(On Diff #403073)

I'm happy with using -debug-info=<debug file>. Originally I wanted to avoid removing the cl::Required in the Filename option, but this really isn't a big deal.

[ellis]

Change option name and add another test file.

[kyulee]

Thanks for making the flag more consistent! Could you quickly update the summary in this patch to reflect the current code?
LGTM

compiler-rt/test/profile/Linux/instrprof-d.c

+// REQUIRES: zlib
+
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
+// RUN: llvm-profdata show --verify-correlation %t | FileCheck %s
+
+// RUN: %clang_pgogen -o %t.no.dbg -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
+// RUN: not llvm-profdata show --verify-correlation %t.no.dbg 2>&1 | FileCheck %s --check-prefix NO-DBG
+// NO-DBG: unable to correlate profile: could not find any profile metadata in debug info
+
+// CHECK: Found 3 profiled functions.
+// CHECK: a
+// CHECK: b
+// CHECK: main
+void a() {}
+void b() {}
+int main() { return 0; }

llvm/include/llvm/ProfileData/InstrProfCorrelator.h

 public:
   static llvm::Expected<std::unique_ptr<InstrProfCorrelator>>
   get(StringRef DebugInfoFilename);
 
   /// Construct a ProfileData vector used to correlate raw instrumentation data
   /// to their functions.
   virtual Error correlateProfileData() = 0;
 
+  /// Return the number of ProfileData elements.
+  llvm::Optional<size_t> getDataSize() const;
+
+  /// Return a pointer to the names string that this class constructs.
+  const char *getNamesPointer() const { return Names.c_str(); }
+
+  /// Return the number of bytes in the names string.
+  size_t getNamesSize() const { return Names.size(); }
+
   static const char *FunctionNameAttributeName;
   static const char *CFGHashAttributeName;
   static const char *NumCountersAttributeName;
 
   enum InstrProfCorrelatorKind { CK_32Bit, CK_64Bit };
   InstrProfCorrelatorKind getKind() const { return Kind; }
   virtual ~InstrProfCorrelator() {}
 
 protected:
   struct Context {
     static llvm::Expected<std::unique_ptr<Context>>
     get(std::unique_ptr<MemoryBuffer> Buffer, const object::ObjectFile &Obj);
     std::unique_ptr<MemoryBuffer> Buffer;
     /// The address range of the __llvm_prf_cnts section.
     uint64_t CountersSectionStart;
     uint64_t CountersSectionEnd;
     /// True if target and host have different endian orders.
     bool ShouldSwapBytes;
   };
   const std::unique_ptr<InstrProfCorrelator::Context> Ctx;
 
   InstrProfCorrelator(InstrProfCorrelatorKind K, std::unique_ptr<Context> Ctx)
       : Ctx(std::move(Ctx)), Kind(K) {}
 
+  std::string Names;
+  std::vector<std::string> NamesVec;
+
 private:
   static llvm::Expected<std::unique_ptr<InstrProfCorrelator>>
   get(std::unique_ptr<MemoryBuffer> Buffer);
 
   const InstrProfCorrelatorKind Kind;
 };
 
 /// InstrProfCorrelatorImpl - A child of InstrProfCorrelator with a template
 /// pointer type so that the ProfileData vector can be materialized.
 template <class IntPtrT>
 class InstrProfCorrelatorImpl : public InstrProfCorrelator {
 public:
   InstrProfCorrelatorImpl(std::unique_ptr<InstrProfCorrelator::Context> Ctx);
   static bool classof(const InstrProfCorrelator *C);
 
   /// Return a pointer to the underlying ProfileData vector that this class
   /// constructs.
   const RawInstrProf::ProfileData<IntPtrT> *getDataPointer() const {
     return Data.empty() ? nullptr : Data.data();
   }
 
   /// Return the number of ProfileData elements.
   size_t getDataSize() const { return Data.size(); }
 
-  /// Return a pointer to the names string that this class constructs.
-  const char *getNamesPointer() const { return Names.c_str(); }
-
-  /// Return the number of bytes in the names string.
-  size_t getNamesSize() const { return Names.size(); }
-
   static llvm::Expected<std::unique_ptr<InstrProfCorrelatorImpl<IntPtrT>>>
   get(std::unique_ptr<InstrProfCorrelator::Context> Ctx,
       const object::ObjectFile &Obj);
 
 protected:
   std::vector<RawInstrProf::ProfileData<IntPtrT>> Data;
-  std::string Names;
 
   Error correlateProfileData() override;
   virtual void correlateProfileDataImpl() = 0;
 
   void addProbe(StringRef FunctionName, uint64_t CFGHash, IntPtrT CounterOffset,
                 IntPtrT FunctionPtr, uint32_t NumCounters);
 
 private:
   InstrProfCorrelatorImpl(InstrProfCorrelatorKind Kind,
                           std::unique_ptr<InstrProfCorrelator::Context> Ctx)
       : InstrProfCorrelator(Kind, std::move(Ctx)){};
-  std::vector<std::string> NamesVec;
   llvm::DenseSet<IntPtrT> CounterOffsets;
 
   // Byte-swap the value if necessary.
   template <class T> T maybeSwap(T Value) const {
     return Ctx->ShouldSwapBytes ? sys::getSwappedBytes(Value) : Value;
   }
 };
 

llvm/lib/ProfileData/InstrProfCorrelator.cpp

if (T.isArch64Bit())
       return InstrProfCorrelatorImpl<uint64_t>::get(std::move(*CtxOrErr), *Obj);
     if (T.isArch32Bit())
       return InstrProfCorrelatorImpl<uint32_t>::get(std::move(*CtxOrErr), *Obj);
   }
   return make_error<InstrProfError>(
       instrprof_error::unable_to_correlate_profile, "not an object file");
 }
 
+Optional<size_t> InstrProfCorrelator::getDataSize() const {
+  if (const auto *C = dyn_cast<InstrProfCorrelatorImpl<uint32_t>>(this)) {
+    return C->getDataSize();
+  } else if (const auto *C =
+                 dyn_cast<InstrProfCorrelatorImpl<uint64_t>>(this)) {
+    return C->getDataSize();
+  }
+  return {};
+}
+
 namespace llvm {
 
 template <>
 InstrProfCorrelatorImpl<uint32_t>::InstrProfCorrelatorImpl(
     std::unique_ptr<InstrProfCorrelator::Context> Ctx)
     : InstrProfCorrelatorImpl(InstrProfCorrelatorKind::CK_32Bit,
                               std::move(Ctx)) {}
 template <>

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

if (Error E = ReaderOr.takeError())
     exitWithError(std::move(E), Filename);
 
   std::unique_ptr<llvm::memprof::RawMemProfReader> Reader(
       ReaderOr.get().release());
   Reader->printSummaries(OS);
   return 0;
 }
 
+static int showCorrelation(const std::string &Filename, raw_fd_ostream &OS) {
+  std::unique_ptr<InstrProfCorrelator> Correlator;
+  if (auto Err = InstrProfCorrelator::get(Filename).moveInto(Correlator))
+    exitWithError(std::move(Err), Filename);
+  if (auto Err = Correlator->correlateProfileData())
+    exitWithError(std::move(Err), Filename);
+
+  OS << "Found " << Correlator->getDataSize() << " profiled functions.\n";
+  // list names
+
+  return 0;
+}
+
 static int show_main(int argc, const char *argv[]) {
   cl::opt<std::string> Filename(cl::Positional, cl::Required,
                                 cl::desc("<profdata-file>"));
 
   cl::opt<bool> ShowCounts("counts", cl::init(false),
                            cl::desc("Show counter values for shown functions"));
   cl::opt<bool> TextFormat(
       "text", cl::init(false),
cl::opt<bool> ShowProfileSymbolList(
       cl::desc("Show profile symbol list if it exists in the profile. "));
   cl::opt<bool> ShowSectionInfoOnly(
       "show-sec-info-only", cl::init(false),
       cl::desc("Show the information of each section in the sample profile. "
                "The flag is only usable when the sample profile is in "
                "extbinary format"));
   cl::opt<bool> ShowBinaryIds("binary-ids", cl::init(false),
                               cl::desc("Show binary ids in the profile. "));
+  cl::opt<bool> VerifyCorrelation("verify-correlation", cl::init(false),
+                                  cl::desc(""));
 
   cl::ParseCommandLineOptions(argc, argv, "LLVM profile data summary\n");
 
   if (Filename == OutputFilename) {
     errs() << sys::path::filename(argv[0])
            << ": Input file name cannot be the same as the output file name!\n";
     return 1;
   }
 
   std::error_code EC;
   raw_fd_ostream OS(OutputFilename.data(), EC, sys::fs::OF_TextWithCRLF);
   if (EC)
     exitWithErrorCode(EC, OutputFilename);
 
   if (ShowAllFunctions && !ShowFunction.empty())
     WithColor::warning() << "-function argument ignored: showing all functions\n";
 
+  if (VerifyCorrelation) {
+    return showCorrelation(Filename, OS);
+  }
+
   if (ProfileKind == instr)
     return showInstrProfile(
         Filename, ShowCounts, TopNFunctions, ShowIndirectCallTargets,
         ShowMemOPSizes, ShowDetailedSummary, DetailedSummaryCutoffs,
         ShowAllFunctions, ShowCS, ValueCutoff, OnlyListBelow, ShowFunction,
         TextFormat, ShowBinaryIds, OS);
   if (ProfileKind == sample)
     return showSampleProfile(Filename, ShowCounts, TopNFunctions,