Support for remapping profile data, for sample-based profiling.

[rsmith]

Summary

Split out from https://reviews.llvm.org/D51240, see that patch for full context.

[davidxl]

include/llvm/ProfileData/SampleProfReader.h

518 ↗

(On Diff #162529)

Can add some documentation on the class.

Also why put Itanium in the class name? The user provided remapping file should be independent of any C++ ABIs.

lib/ProfileData/SampleProfReader.cpp

833 ↗

(On Diff #162529)

Ideally, there should be a tool to convert the compact format file into a non-compact file. The nocompat file is then fed into the profile remapping tool which converts the profile to the remapped namespace.

[wmi]

We do not support remapping indirect branch target information, but all other profile data should be remapped appropriately.

Any particular reason for that? Without support of call target, the performance could still be hurt badly after the renaming.

lib/ProfileData/SampleProfReader.cpp

833 ↗

(On Diff #162529)

Most of the time, we can regenerate the non-compact profile and use the remapping tool to convert the non-compact profile directly. Currently I don't see I have the need to convert non-compact profile directly. We may leave the request there for the moment until we need it.

[rsmith]

Added documentation comment.

[rsmith]

In D51248#1214761, @wmi wrote:

We do not support remapping indirect branch target information, but all other profile data should be remapped appropriately.

Any particular reason for that? Without support of call target, the performance could still be hurt badly after the renaming.

Yes, the idea here is that the user doesn't need to specify which mappings have already happened (there is no "before" / "after", and no "remangling" step, just two name components that are to be treated as "the same"). That works well for most profile data: we can intercept queries for a call to foo(std::__1::string) and look up any names that are "similar" to that. But it doesn't work so easily for indirect call targets, where we would need to check (demangle and attempt to match) all functions within the module in order to determine what a particular indirect call target is transformed into. If it's important to be able to handle this, I think the cleanest way to do it would be for the profile data consumer to pass a list of all "known" function names into the reader to support such remapping. (Another approach would be to add a remangling step and require the equivalences to be specified as before -> after transformations, but that would add substantially more complexity and puts a maintenance burden on all future extensions to the demangler, so it wouldn't be my preference.)

I think this is something that we should incrementally add later, even if we already know for sure that we'll need it.

include/llvm/ProfileData/SampleProfReader.h

518 ↗

(On Diff #162529)

(Comment added.)

Remapping is supported only for Itanium manglings for now, and that's all this class supports as a consequence. While I think it should be feasible to support remapping MS ABI manglings with largely the same interface, it seems reasonable to me to give this class the more specific name until we generalize it.

But I don't feel at all strongly about this, and I'm happy change the name if you prefer. If we drop the Itanium here, I think it'd make sense to also drop it from the ProfileRemappingReader with a view to it (eventually) hiding the difference between the mangling schemes there.

[davidxl]

unittests/ProfileData/SampleProfTest.cpp

120 ↗

(On Diff #162726)

directly using raw name will break compact format testing.

[wmi]

LGTM.

lib/ProfileData/SampleProfReader.cpp

830 ↗

(On Diff #162726)

Nit: SampleProfileReaderItaniumRemapper assumes the underlying reader has already read in the profile. The override function read() here is a little misleading that people may think SampleProfileReaderItaniumRemapper can both read in the profile and do the remapping. May be just rename it as remap()?

[rsmith]

Rebase and extend testing to cover raw binary format.

[rsmith]

unittests/ProfileData/SampleProfTest.cpp

120 ↗

(On Diff #162726)

getSamplesFor converts the name to MD5 if the profile is in compact format.

[rsmith]

@davidxl Do you have any more thoughts on this before I submit?

[davidxl]

looks fine to me.

llvm/trunk/include/llvm/ProfileData/SampleProfReader.h

 #include "llvm/IR/Function.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/ProfileSummary.h"
 #include "llvm/ProfileData/GCOV.h"
 #include "llvm/ProfileData/SampleProf.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/SymbolRemappingReader.h"
 #include <algorithm>
 #include <cstdint>
 #include <memory>
 #include <string>
 #include <system_error>
 #include <vector>
 
 namespace llvm {
public:
   /// Print all the profiles on stream \p OS.
   void dump(raw_ostream &OS = dbgs());
 
   /// Return the samples collected for function \p F.
   FunctionSamples *getSamplesFor(const Function &F) {
     // The function name may have been updated by adding suffix. In sample
     // profile, the function names are all stripped, so we need to strip
     // the function name suffix before matching with profile.
-    StringRef Fname = F.getName().split('.').first;
+    return getSamplesFor(F.getName().split('.').first);
+  }
+
+  /// Return the samples collected for function \p F.
+  virtual FunctionSamples *getSamplesFor(StringRef Fname) {
     std::string FGUID;
     Fname = getRepInFormat(Fname, getFormat(), FGUID);
-    if (Profiles.count(Fname))
-      return &Profiles[Fname];
+    auto It = Profiles.find(Fname);
+    if (It != Profiles.end())
+      return &It->second;
     return nullptr;
   }
 
   /// Return all the profiles.
   StringMap<FunctionSamples> &getProfiles() { return Profiles; }
 
   /// Report a parse error message.
   void reportError(int64_t LineNumber, Twine Msg) const {
protected:
   LLVMContext &Ctx;
 
   /// Memory buffer holding the profile file.
   std::unique_ptr<MemoryBuffer> Buffer;
 
   /// Profile summary information.
   std::unique_ptr<ProfileSummary> Summary;
 
+  /// Take ownership of the summary of this reader.
+  static std::unique_ptr<ProfileSummary>
+  takeSummary(SampleProfileReader &Reader) {
+    return std::move(Reader.Summary);
+  }
+
   /// Compute summary for this profile.
   void computeSummary();
 
   /// \brief The format of sample.
   SampleProfileFormat Format = SPF_None;
 };
 
 class SampleProfileReaderText : public SampleProfileReader {
protected:
   /// Function names in this profile.
   std::vector<std::string> Names;
 
   /// GCOV tags used to separate sections in the profile file.
   static const uint32_t GCOVTagAFDOFileNames = 0xaa000000;
   static const uint32_t GCOVTagAFDOFunction = 0xac000000;
 };
 
+/// A profile data reader proxy that remaps the profile data from another
+/// sample profile data reader, by applying a provided set of equivalences
+/// between components of the symbol names in the profile.
+class SampleProfileReaderItaniumRemapper : public SampleProfileReader {
+public:
+  SampleProfileReaderItaniumRemapper(
+      std::unique_ptr<MemoryBuffer> B, LLVMContext &C,
+      std::unique_ptr<SampleProfileReader> Underlying)
+      : SampleProfileReader(std::move(B), C, Underlying->getFormat()) {
+    Profiles = std::move(Underlying->getProfiles());
+    Summary = takeSummary(*Underlying);
+  }
+
+  /// Create a remapped sample profile from the given remapping file and
+  /// underlying samples.
+  static ErrorOr<std::unique_ptr<SampleProfileReader>>
+  create(const Twine &Filename, LLVMContext &C,
+         std::unique_ptr<SampleProfileReader> Underlying);
+
+  /// Read and validate the file header.
+  std::error_code readHeader() override { return sampleprof_error::success; }
+
+  /// Read remapping file and apply it to the sample profile.
+  std::error_code read() override;
+
+  /// Return the samples collected for function \p F.
+  FunctionSamples *getSamplesFor(StringRef FunctionName) override;
+  using SampleProfileReader::getSamplesFor;
+
+private:
+  SymbolRemappingReader Remappings;
+  DenseMap<SymbolRemappingReader::Key, FunctionSamples*> SampleMap;
+};
+
 } // end namespace sampleprof
 
 } // end namespace llvm
 
 #endif // LLVM_PROFILEDATA_SAMPLEPROFREADER_H

llvm/trunk/lib/ProfileData/SampleProfReader.cpp

std::error_code SampleProfileReaderGCC::read() {
   return sampleprof_error::success;
 }
 
 bool SampleProfileReaderGCC::hasFormat(const MemoryBuffer &Buffer) {
   StringRef Magic(reinterpret_cast<const char *>(Buffer.getBufferStart()));
   return Magic == "adcg*704";
 }
 
+std::error_code SampleProfileReaderItaniumRemapper::read() {
+  // If the underlying data is in compact format, we can't remap it because
+  // we don't know what the original function names were.
+  if (getFormat() == SPF_Compact_Binary) {
+    Ctx.diagnose(DiagnosticInfoSampleProfile(
+        Buffer->getBufferIdentifier(),
+        "Profile data remapping cannot be applied to profile data "
+        "in compact format (original mangled names are not available).",
+        DS_Warning));
+    return sampleprof_error::success;
+  }
+
+  if (Error E = Remappings.read(*Buffer)) {
+    handleAllErrors(
+        std::move(E), [&](const SymbolRemappingParseError &ParseError) {
+          reportError(ParseError.getLineNum(), ParseError.getMessage());
+        });
+    return sampleprof_error::malformed;
+  }
+
+  for (auto &Sample : getProfiles())
+    if (auto Key = Remappings.insert(Sample.first()))
+      SampleMap.insert({Key, &Sample.second});
+
+  return sampleprof_error::success;
+}
+
+FunctionSamples *
+SampleProfileReaderItaniumRemapper::getSamplesFor(StringRef Fname) {
+  if (auto Key = Remappings.lookup(Fname))
+    return SampleMap.lookup(Key);
+  return SampleProfileReader::getSamplesFor(Fname);
+}
+
 /// Prepare a memory buffer for the contents of \p Filename.
 ///
 /// \returns an error code indicating the status of the buffer.
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 setupMemoryBuffer(const Twine &Filename) {
   auto BufferOrErr = MemoryBuffer::getFileOrSTDIN(Filename);
   if (std::error_code EC = BufferOrErr.getError())
     return EC;

 ErrorOr<std::unique_ptr<SampleProfileReader>>
 SampleProfileReader::create(const Twine &Filename, LLVMContext &C) {
   auto BufferOrError = setupMemoryBuffer(Filename);
   if (std::error_code EC = BufferOrError.getError())
     return EC;
   return create(BufferOrError.get(), C);
 }
 
+/// Create a sample profile remapper from the given input, to remap the
+/// function names in the given profile data.
+///
+/// \param Filename The file to open.
+///
+/// \param C The LLVM context to use to emit diagnostics.
+///
+/// \param Underlying The underlying profile data reader to remap.
+///
+/// \returns an error code indicating the status of the created reader.
+ErrorOr<std::unique_ptr<SampleProfileReader>>
+SampleProfileReaderItaniumRemapper::create(
+    const Twine &Filename, LLVMContext &C,
+    std::unique_ptr<SampleProfileReader> Underlying) {
+  auto BufferOrError = setupMemoryBuffer(Filename);
+  if (std::error_code EC = BufferOrError.getError())
+    return EC;
+  return llvm::make_unique<SampleProfileReaderItaniumRemapper>(
+      std::move(BufferOrError.get()), C, std::move(Underlying));
+}
+
 /// Create a sample profile reader based on the format of the input data.
 ///
 /// \param B The memory buffer to create the reader from (assumes ownership).
 ///
 /// \param C The LLVM context to use to emit diagnostics.
 ///
 /// \returns an error code indicating the status of the created reader.
 ErrorOr<std::unique_ptr<SampleProfileReader>>

llvm/trunk/unittests/ProfileData/SampleProfTest.cpp

struct SampleProfTest : ::testing::Test {
 
   void readProfile(const Module &M, StringRef Profile) {
     auto ReaderOrErr = SampleProfileReader::create(Profile, Context);
     ASSERT_TRUE(NoError(ReaderOrErr.getError()));
     Reader = std::move(ReaderOrErr.get());
     Reader->collectFuncsToUse(M);
   }
 
-  void testRoundTrip(SampleProfileFormat Format) {
+  void testRoundTrip(SampleProfileFormat Format, bool Remap) {
     SmallVector<char, 128> ProfilePath;
     ASSERT_TRUE(NoError(llvm::sys::fs::createTemporaryFile("profile", "", ProfilePath)));
     StringRef Profile(ProfilePath.data(), ProfilePath.size());
     createWriter(Format, Profile);
 
     StringRef FooName("_Z3fooi");
     FunctionSamples FooSamples;
     FooSamples.setName(FooName);
void testRoundTrip(SampleProfileFormat Format, bool Remap) {
 
     Writer->getOutputStream().flush();
 
     readProfile(M, Profile);
 
     EC = Reader->read();
     ASSERT_TRUE(NoError(EC));
 
-    StringMap<FunctionSamples> &ReadProfiles = Reader->getProfiles();
-    ASSERT_EQ(2u, ReadProfiles.size());
+    if (Remap) {
+      auto MemBuffer = llvm::MemoryBuffer::getMemBuffer(R"(
+        # Types 'int' and 'long' are equivalent
+        type i l
+        # Function names 'foo' and 'faux' are equivalent
+        name 3foo 4faux
+      )");
+      Reader.reset(new SampleProfileReaderItaniumRemapper(
+          std::move(MemBuffer), Context, std::move(Reader)));
+      FooName = "_Z4fauxi";
+      BarName = "_Z3barl";
 
-    std::string FooGUID;
-    StringRef FooRep = getRepInFormat(FooName, Format, FooGUID);
-    FunctionSamples &ReadFooSamples = ReadProfiles[FooRep];
-    ASSERT_EQ(7711u, ReadFooSamples.getTotalSamples());
-    ASSERT_EQ(610u, ReadFooSamples.getHeadSamples());
-
-    std::string BarGUID;
-    StringRef BarRep = getRepInFormat(BarName, Format, BarGUID);
-    FunctionSamples &ReadBarSamples = ReadProfiles[BarRep];
-    ASSERT_EQ(20301u, ReadBarSamples.getTotalSamples());
-    ASSERT_EQ(1437u, ReadBarSamples.getHeadSamples());
+      EC = Reader->read();
+      ASSERT_TRUE(NoError(EC));
+    }
+
+    ASSERT_EQ(2u, Reader->getProfiles().size());
+
+    FunctionSamples *ReadFooSamples = Reader->getSamplesFor(FooName);
+    ASSERT_TRUE(ReadFooSamples != nullptr);
+    ASSERT_EQ(7711u, ReadFooSamples->getTotalSamples());
+    ASSERT_EQ(610u, ReadFooSamples->getHeadSamples());
+
+    FunctionSamples *ReadBarSamples = Reader->getSamplesFor(BarName);
+    ASSERT_TRUE(ReadBarSamples != nullptr);
+    ASSERT_EQ(20301u, ReadBarSamples->getTotalSamples());
+    ASSERT_EQ(1437u, ReadBarSamples->getHeadSamples());
     ErrorOr<SampleRecord::CallTargetMap> CTMap =
-        ReadBarSamples.findCallTargetMapAt(1, 0);
+        ReadBarSamples->findCallTargetMapAt(1, 0);
     ASSERT_FALSE(CTMap.getError());
 
     std::string MconstructGUID;
     StringRef MconstructRep =
         getRepInFormat(MconstructName, Format, MconstructGUID);
     std::string StringviewGUID;
     StringRef StringviewRep =
         getRepInFormat(StringviewName, Format, StringviewGUID);
void testRoundTrip(SampleProfileFormat Format, bool Remap) {
     PS = ProfileSummary::getFromMD(MD);
     ASSERT_TRUE(PS);
     VerifySummary(*PS);
     delete PS;
   }
 };
 
 TEST_F(SampleProfTest, roundtrip_text_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Text);
+  testRoundTrip(SampleProfileFormat::SPF_Text, false);
 }
 
 TEST_F(SampleProfTest, roundtrip_raw_binary_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Binary);
+  testRoundTrip(SampleProfileFormat::SPF_Binary, false);
 }
 
 TEST_F(SampleProfTest, roundtrip_compact_binary_profile) {
-  testRoundTrip(SampleProfileFormat::SPF_Compact_Binary);
+  testRoundTrip(SampleProfileFormat::SPF_Compact_Binary, false);
+}
+
+TEST_F(SampleProfTest, remap_text_profile) {
+  testRoundTrip(SampleProfileFormat::SPF_Text, true);
+}
+
+TEST_F(SampleProfTest, remap_raw_binary_profile) {
+  testRoundTrip(SampleProfileFormat::SPF_Binary, true);
 }
 
 TEST_F(SampleProfTest, sample_overflow_saturation) {
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
   sampleprof_error Result;
 
   StringRef FooName("_Z3fooi");
   FunctionSamples FooSamples;