[llvm] [memprof] Upgrade a unit test to Version 3 (PR #116516)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

<details>
<summary>Changes</summary>

I'm planning to remove MemProf Version 1, which is a maintenance
burden because it uses a different set of struct fields in
IndexedAllocationInfo and IndexedMemProfRecord compared to Version 2
and 3.  (FWIW, Version 2 and 3 are much closer to each other.)

Before we remove the old version, we need to upgrade
test_memprof_merge to Version 3.  Here are some remarks:

- Without this patch, we call Writer.addMemProfFrame, which I don't
  think is correct.  This way, we are adding IndexedMemProfRecord to
  Writer2 without any frame.  I'm changing that to
  Writer2.addMemProfFrame.

- This patch adds a call to getCallStackMapping.  Version 2 and 3
  require a map from call stack IDs to call stacks.

- I'm calling makeRecordV2 instead of makeRecord to populate the
  struct fields used by Version 2 and 3.

- Version 1 uses MemProfRecord::MemProfRecord (that is, a constructor)
  to convert IndexedMemProfRecord to MemProfRecord.  Version 2 and 3
  use MemProfRecord::toMemProfRecord, a member function, to do the
  same task.


---
Full diff: https://github.com/llvm/llvm-project/pull/116516.diff


1 Files Affected:

- (modified) llvm/unittests/ProfileData/InstrProfTest.cpp (+16-20) 


``````````diff
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index 75b47a18daf49a..582efad531bf74 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -677,29 +677,29 @@ TEST_F(InstrProfTest, test_memprof_merge) {
   Writer.addRecord({"func1", 0x1234, {42}}, Err);
 
   InstrProfWriter Writer2;
+  Writer2.setMemProfVersionRequested(memprof::Version3);
   ASSERT_THAT_ERROR(Writer2.mergeProfileKind(InstrProfKind::MemProf),
                     Succeeded());
 
-  const IndexedMemProfRecord IndexedMR = makeRecord(
-      /*AllocFrames=*/
-      {
-          {0, 1},
-          {2, 3},
-      },
-      /*CallSiteFrames=*/{
-          {4, 5},
-      });
-
   const FrameIdMapTy IdToFrameMap = getFrameMapping();
   for (const auto &I : IdToFrameMap) {
-    Writer.addMemProfFrame(I.first, I.getSecond(), Err);
+    Writer2.addMemProfFrame(I.first, I.getSecond(), Err);
   }
+
+  const auto CSIdToCallStackMap = getCallStackMapping();
+  for (const auto &[CSId, CallStack] : CSIdToCallStackMap)
+    Writer2.addMemProfCallStack(CSId, CallStack, Err);
+
+  const IndexedMemProfRecord IndexedMR = makeRecordV2(
+      /*AllocFrames=*/{0x111, 0x222},
+      /*CallSiteFrames=*/{}, makePartialMIB(), memprof::getHotColdSchema());
   Writer2.addMemProfRecord(/*Id=*/0x9999, IndexedMR);
 
   ASSERT_THAT_ERROR(Writer.mergeProfileKind(Writer2.getProfileKind()),
                     Succeeded());
   Writer.mergeRecordsFromWriter(std::move(Writer2), Err);
 
+  Writer.setMemProfVersionRequested(memprof::Version3);
   auto Profile = Writer.writeBuffer();
   readProfile(std::move(Profile));
 
@@ -714,16 +714,12 @@ TEST_F(InstrProfTest, test_memprof_merge) {
 
   std::optional<memprof::FrameId> LastUnmappedFrameId;
 
-  auto IdToFrameCallback = [&](const memprof::FrameId Id) {
-    auto Iter = IdToFrameMap.find(Id);
-    if (Iter == IdToFrameMap.end()) {
-      LastUnmappedFrameId = Id;
-      return memprof::Frame(0, 0, 0, false);
-    }
-    return Iter->second;
-  };
+  memprof::FrameIdConverter<decltype(IdToFrameMap)> FrameIdConv(IdToFrameMap);
+  memprof::CallStackIdConverter<decltype(CSIdToCallStackMap)> CSIdConv(
+      CSIdToCallStackMap, FrameIdConv);
 
-  const memprof::MemProfRecord WantRecord(IndexedMR, IdToFrameCallback);
+  const ::llvm::memprof::MemProfRecord WantRecord =
+      IndexedMR.toMemProfRecord(CSIdConv);
   ASSERT_EQ(LastUnmappedFrameId, std::nullopt)
       << "could not map frame id: " << *LastUnmappedFrameId;
   EXPECT_THAT(WantRecord, EqualsRecord(Record));

``````````

</details>


https://github.com/llvm/llvm-project/pull/116516