[NFC][MemProf] Move getGUID out of IndexedMemProfRecord #140502

snehasish · May 19, 2025

Part of a larger refactoring with the following goals

Reduce the size of MemProf.h
Avoid including ModuleSummaryIndex just for a couple of types

snehasish · May 19, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · May 19, 2025

@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-transforms

Author: Snehasish Kumar (snehasish)

Changes

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

7 Files Affected:

(modified) llvm/include/llvm/ProfileData/MemProf.h (+5-5)
(modified) llvm/include/llvm/ProfileData/MemProfYAML.h (+1-1)
(modified) llvm/lib/ProfileData/MemProf.cpp (+1-1)
(modified) llvm/lib/ProfileData/MemProfReader.cpp (+1-1)
(modified) llvm/lib/Transforms/Instrumentation/MemProfiler.cpp (+2-2)
(modified) llvm/unittests/ProfileData/MemProfTest.cpp (+10-10)
(modified) llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp (+24-24)

diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 0bc1432f7d198..215102c131fff 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -472,13 +472,13 @@ struct IndexedMemProfRecord {
   // translate CallStackId to call stacks with frames inline.
   MemProfRecord toMemProfRecord(
       llvm::function_ref<std::vector<Frame>(const CallStackId)> Callback) const;
-
-  // Returns the GUID for the function name after canonicalization. For
-  // memprof, we remove any .llvm suffix added by LTO. MemProfRecords are
-  // mapped to functions using this GUID.
-  static GlobalValue::GUID getGUID(const StringRef FunctionName);
 };
 
+// Returns the GUID for the function name after canonicalization. For
+// memprof, we remove any .llvm suffix added by LTO. MemProfRecords are
+// mapped to functions using this GUID.
+GlobalValue::GUID getGUID(const StringRef FunctionName);
+
 // Holds call site information with frame contents inline.
 struct CallSiteInfo {
   // The frames in the call stack
diff --git a/llvm/include/llvm/ProfileData/MemProfYAML.h b/llvm/include/llvm/ProfileData/MemProfYAML.h
index 08dee253f615a..b642e3098aa0e 100644
--- a/llvm/include/llvm/ProfileData/MemProfYAML.h
+++ b/llvm/include/llvm/ProfileData/MemProfYAML.h
@@ -46,7 +46,7 @@ template <> struct ScalarTraits<memprof::GUIDHex64> {
       Val = Num;
     } else {
       // Otherwise, treat the input as a string containing a function name.
-      Val = memprof::IndexedMemProfRecord::getGUID(Scalar);
+      Val = memprof::getGUID(Scalar);
     }
     return StringRef();
   }
diff --git a/llvm/lib/ProfileData/MemProf.cpp b/llvm/lib/ProfileData/MemProf.cpp
index a9c5ee09a6daf..795e97bee38f5 100644
--- a/llvm/lib/ProfileData/MemProf.cpp
+++ b/llvm/lib/ProfileData/MemProf.cpp
@@ -343,7 +343,7 @@ MemProfRecord IndexedMemProfRecord::toMemProfRecord(
   return Record;
 }
 
-GlobalValue::GUID IndexedMemProfRecord::getGUID(const StringRef FunctionName) {
+GlobalValue::GUID getGUID(const StringRef FunctionName) {
   // Canonicalize the function name to drop suffixes such as ".llvm.". Note
   // we do not drop any ".__uniq." suffixes, as getCanonicalFnName does not drop
   // those by default. This is by design to differentiate internal linkage
diff --git a/llvm/lib/ProfileData/MemProfReader.cpp b/llvm/lib/ProfileData/MemProfReader.cpp
index e0f280b9eb2f6..aca534b0a4c98 100644
--- a/llvm/lib/ProfileData/MemProfReader.cpp
+++ b/llvm/lib/ProfileData/MemProfReader.cpp
@@ -570,7 +570,7 @@ Error RawMemProfReader::symbolizeAndFilterStackFrames(
            I++) {
         const auto &DIFrame = DI.getFrame(I);
         const uint64_t Guid =
-            IndexedMemProfRecord::getGUID(DIFrame.FunctionName);
+            memprof::getGUID(DIFrame.FunctionName);
         const Frame F(Guid, DIFrame.Line - DIFrame.StartLine, DIFrame.Column,
                       // Only the last entry is not an inlined location.
                       I != NumFrames - 1);
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 375ff84f82ed2..5982476f3994e 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -865,8 +865,8 @@ memprof::extractCallsFromIR(Module &M, const TargetLibraryInfo &TLI,
           StringRef CallerName = DIL->getSubprogramLinkageName();
           assert(!CallerName.empty() &&
                  "Be sure to enable -fdebug-info-for-profiling");
-          uint64_t CallerGUID = IndexedMemProfRecord::getGUID(CallerName);
-          uint64_t CalleeGUID = IndexedMemProfRecord::getGUID(CalleeName);
+          uint64_t CallerGUID = memprof::getGUID(CallerName);
+          uint64_t CalleeGUID = memprof::getGUID(CalleeName);
           // Pretend that we are calling a function with GUID == 0 if we are
           // in the inline stack leading to a heap allocation function.
           if (IsAlloc) {
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index a072dee26d9a0..2ae9cd96f0197 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -107,7 +107,7 @@ const DILineInfoSpecifier specifier() {
 MATCHER_P4(FrameContains, FunctionName, LineOffset, Column, Inline, "") {
   const Frame &F = arg;
 
-  const uint64_t ExpectedHash = IndexedMemProfRecord::getGUID(FunctionName);
+  const uint64_t ExpectedHash = memprof::getGUID(FunctionName);
   if (F.Function != ExpectedHash) {
     *result_listener << "Hash mismatch";
     return false;
@@ -180,7 +180,7 @@ TEST(MemProf, FillsValue) {
   ASSERT_THAT(Records, SizeIs(4));
 
   // Check the memprof record for foo.
-  const llvm::GlobalValue::GUID FooId = IndexedMemProfRecord::getGUID("foo");
+  const llvm::GlobalValue::GUID FooId = memprof::getGUID("foo");
   ASSERT_TRUE(Records.contains(FooId));
   const MemProfRecord &Foo = Records[FooId];
   ASSERT_THAT(Foo.AllocSites, SizeIs(1));
@@ -196,7 +196,7 @@ TEST(MemProf, FillsValue) {
   EXPECT_TRUE(Foo.CallSites.empty());
 
   // Check the memprof record for bar.
-  const llvm::GlobalValue::GUID BarId = IndexedMemProfRecord::getGUID("bar");
+  const llvm::GlobalValue::GUID BarId = memprof::getGUID("bar");
   ASSERT_TRUE(Records.contains(BarId));
   const MemProfRecord &Bar = Records[BarId];
   ASSERT_THAT(Bar.AllocSites, SizeIs(1));
@@ -217,7 +217,7 @@ TEST(MemProf, FillsValue) {
                               FrameContains("bar", 51U, 20U, false)))));
 
   // Check the memprof record for xyz.
-  const llvm::GlobalValue::GUID XyzId = IndexedMemProfRecord::getGUID("xyz");
+  const llvm::GlobalValue::GUID XyzId = memprof::getGUID("xyz");
   ASSERT_TRUE(Records.contains(XyzId));
   const MemProfRecord &Xyz = Records[XyzId];
   // Expect the entire frame even though in practice we only need the first
@@ -229,7 +229,7 @@ TEST(MemProf, FillsValue) {
                               FrameContains("abc", 5U, 30U, false)))));
 
   // Check the memprof record for abc.
-  const llvm::GlobalValue::GUID AbcId = IndexedMemProfRecord::getGUID("abc");
+  const llvm::GlobalValue::GUID AbcId = memprof::getGUID("abc");
   ASSERT_TRUE(Records.contains(AbcId));
   const MemProfRecord &Abc = Records[AbcId];
   EXPECT_TRUE(Abc.AllocSites.empty());
@@ -461,9 +461,9 @@ TEST(MemProf, SymbolizationFilter) {
 
 TEST(MemProf, BaseMemProfReader) {
   IndexedMemProfData MemProfData;
-  Frame F1(/*Hash=*/IndexedMemProfRecord::getGUID("foo"), /*LineOffset=*/20,
+  Frame F1(/*Hash=*/memprof::getGUID("foo"), /*LineOffset=*/20,
            /*Column=*/5, /*IsInlineFrame=*/true);
-  Frame F2(/*Hash=*/IndexedMemProfRecord::getGUID("bar"), /*LineOffset=*/10,
+  Frame F2(/*Hash=*/memprof::getGUID("bar"), /*LineOffset=*/10,
            /*Column=*/2, /*IsInlineFrame=*/false);
   auto F1Id = MemProfData.addFrame(F1);
   auto F2Id = MemProfData.addFrame(F2);
@@ -493,9 +493,9 @@ TEST(MemProf, BaseMemProfReader) {
 
 TEST(MemProf, BaseMemProfReaderWithCSIdMap) {
   IndexedMemProfData MemProfData;
-  Frame F1(/*Hash=*/IndexedMemProfRecord::getGUID("foo"), /*LineOffset=*/20,
+  Frame F1(/*Hash=*/memprof::getGUID("foo"), /*LineOffset=*/20,
            /*Column=*/5, /*IsInlineFrame=*/true);
-  Frame F2(/*Hash=*/IndexedMemProfRecord::getGUID("bar"), /*LineOffset=*/10,
+  Frame F2(/*Hash=*/memprof::getGUID("bar"), /*LineOffset=*/10,
            /*Column=*/2, /*IsInlineFrame=*/false);
   auto F1Id = MemProfData.addFrame(F1);
   auto F2Id = MemProfData.addFrame(F2);
@@ -813,7 +813,7 @@ TEST(MemProf, YAMLParserGUID) {
   // Verify the entire contents of MemProfData.Records.
   ASSERT_THAT(MemProfData.Records, SizeIs(1));
   const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
-  EXPECT_EQ(GUID, IndexedMemProfRecord::getGUID("_Z3fooi"));
+  EXPECT_EQ(GUID, memprof::getGUID("_Z3fooi"));
 
   IndexedCallstackIdConverter CSIdConv(MemProfData);
   MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
diff --git a/llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp b/llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp
index e4926f87dd6dc..95828356b490b 100644
--- a/llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp
+++ b/llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp
@@ -101,16 +101,16 @@ declare !dbg !19 void @_Z2f3v()
   ASSERT_NE(It, Calls.end());
 
   const auto &[CallerGUID, CallSites] = *It;
-  EXPECT_EQ(CallerGUID, IndexedMemProfRecord::getGUID("_Z3foov"));
+  EXPECT_EQ(CallerGUID, memprof::getGUID("_Z3foov"));
 
   // Verify that call sites show up in the ascending order of their source
   // locations.
   EXPECT_THAT(
       CallSites,
       ElementsAre(
-          Pair(LineLocation(1, 3), IndexedMemProfRecord::getGUID("_Z2f1v")),
-          Pair(LineLocation(2, 3), IndexedMemProfRecord::getGUID("_Z2f2v")),
-          Pair(LineLocation(2, 9), IndexedMemProfRecord::getGUID("_Z2f3v"))));
+          Pair(LineLocation(1, 3), memprof::getGUID("_Z2f1v")),
+          Pair(LineLocation(2, 3), memprof::getGUID("_Z2f2v")),
+          Pair(LineLocation(2, 9), memprof::getGUID("_Z2f3v"))));
 }
 
 TEST(MemProf, ExtractDirectCallsFromIRInline) {
@@ -200,41 +200,41 @@ declare !dbg !25 void @_Z2g2v() local_unnamed_addr
 
   // Verify each key-value pair.
 
-  auto FooIt = Calls.find(IndexedMemProfRecord::getGUID("_Z3foov"));
+  auto FooIt = Calls.find(memprof::getGUID("_Z3foov"));
   ASSERT_NE(FooIt, Calls.end());
   const auto &[FooCallerGUID, FooCallSites] = *FooIt;
-  EXPECT_EQ(FooCallerGUID, IndexedMemProfRecord::getGUID("_Z3foov"));
+  EXPECT_EQ(FooCallerGUID, memprof::getGUID("_Z3foov"));
   EXPECT_THAT(
       FooCallSites,
       ElementsAre(
-          Pair(LineLocation(1, 3), IndexedMemProfRecord::getGUID("_ZL2f3v")),
-          Pair(LineLocation(2, 9), IndexedMemProfRecord::getGUID("_ZL2g3v"))));
+          Pair(LineLocation(1, 3), memprof::getGUID("_ZL2f3v")),
+          Pair(LineLocation(2, 9), memprof::getGUID("_ZL2g3v"))));
 
-  auto F2It = Calls.find(IndexedMemProfRecord::getGUID("_ZL2f2v"));
+  auto F2It = Calls.find(memprof::getGUID("_ZL2f2v"));
   ASSERT_NE(F2It, Calls.end());
   const auto &[F2CallerGUID, F2CallSites] = *F2It;
-  EXPECT_EQ(F2CallerGUID, IndexedMemProfRecord::getGUID("_ZL2f2v"));
+  EXPECT_EQ(F2CallerGUID, memprof::getGUID("_ZL2f2v"));
   EXPECT_THAT(F2CallSites,
               ElementsAre(Pair(LineLocation(2, 3),
-                               IndexedMemProfRecord::getGUID("_Z2f1v"))));
+                               memprof::getGUID("_Z2f1v"))));
 
-  auto F3It = Calls.find(IndexedMemProfRecord::getGUID("_ZL2f3v"));
+  auto F3It = Calls.find(memprof::getGUID("_ZL2f3v"));
   ASSERT_NE(F3It, Calls.end());
   const auto &[F3CallerGUID, F3CallSites] = *F3It;
-  EXPECT_EQ(F3CallerGUID, IndexedMemProfRecord::getGUID("_ZL2f3v"));
+  EXPECT_EQ(F3CallerGUID, memprof::getGUID("_ZL2f3v"));
   EXPECT_THAT(F3CallSites,
               ElementsAre(Pair(LineLocation(1, 10),
-                               IndexedMemProfRecord::getGUID("_ZL2f2v"))));
+                               memprof::getGUID("_ZL2f2v"))));
 
-  auto G3It = Calls.find(IndexedMemProfRecord::getGUID("_ZL2g3v"));
+  auto G3It = Calls.find(memprof::getGUID("_ZL2g3v"));
   ASSERT_NE(G3It, Calls.end());
   const auto &[G3CallerGUID, G3CallSites] = *G3It;
-  EXPECT_EQ(G3CallerGUID, IndexedMemProfRecord::getGUID("_ZL2g3v"));
+  EXPECT_EQ(G3CallerGUID, memprof::getGUID("_ZL2g3v"));
   EXPECT_THAT(
       G3CallSites,
       ElementsAre(
-          Pair(LineLocation(1, 8), IndexedMemProfRecord::getGUID("_Z2g1v")),
-          Pair(LineLocation(2, 3), IndexedMemProfRecord::getGUID("_Z2g2v"))));
+          Pair(LineLocation(1, 8), memprof::getGUID("_Z2g1v")),
+          Pair(LineLocation(2, 3), memprof::getGUID("_Z2g2v"))));
 }
 
 TEST(MemProf, ExtractDirectCallsFromIRCallingNew) {
@@ -295,10 +295,10 @@ attributes #2 = { builtin allocsize(0) }
 
   // Verify each key-value pair.
 
-  auto FooIt = Calls.find(IndexedMemProfRecord::getGUID("_Z3foov"));
+  auto FooIt = Calls.find(memprof::getGUID("_Z3foov"));
   ASSERT_NE(FooIt, Calls.end());
   const auto &[FooCallerGUID, FooCallSites] = *FooIt;
-  EXPECT_EQ(FooCallerGUID, IndexedMemProfRecord::getGUID("_Z3foov"));
+  EXPECT_EQ(FooCallerGUID, memprof::getGUID("_Z3foov"));
   EXPECT_THAT(FooCallSites, ElementsAre(Pair(LineLocation(1, 10), 0)));
 }
 
@@ -406,10 +406,10 @@ attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "t
   auto &TLI = WrapperPass.getTLI(*F);
   auto Calls = extractCallsFromIR(*M, TLI);
 
-  uint64_t GUIDFoo = IndexedMemProfRecord::getGUID("_Z3foov");
-  uint64_t GUIDBar = IndexedMemProfRecord::getGUID("_Z3barv");
-  uint64_t GUIDBaz = IndexedMemProfRecord::getGUID("_Z3bazv");
-  uint64_t GUIDZzz = IndexedMemProfRecord::getGUID("_Z3zzzv");
+  uint64_t GUIDFoo = memprof::getGUID("_Z3foov");
+  uint64_t GUIDBar = memprof::getGUID("_Z3barv");
+  uint64_t GUIDBaz = memprof::getGUID("_Z3bazv");
+  uint64_t GUIDZzz = memprof::getGUID("_Z3zzzv");
 
   // Verify that extractCallsFromIR extracts caller-callee pairs as expected.
   EXPECT_THAT(Calls,

github-actions · May 19, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

kazutakahirata

LGTM. Thanks!

kazutakahirata

LGTM. Thanks!

snehasish · May 19, 2025

Merge activity

May 19, 7:09 PM EDT: A user started a stack merge that includes this pull request via Graphite.
May 19, 7:16 PM EDT: Graphite rebased this pull request as part of a merge.
May 19, 7:18 PM EDT: @snehasish merged this pull request with Graphite.

llvmbot added PGO Profile Guided Optimizations llvm:transforms labels May 19, 2025

snehasish mentioned this pull request May 19, 2025

[NFC][MemProf] Fix typo in type name #140500

Merged

snehasish requested review from kazutakahirata, mingmingl-llvm and teresajohnson May 19, 2025 07:10

kazutakahirata approved these changes May 19, 2025

View reviewed changes

el-ev approved these changes May 19, 2025

View reviewed changes

kazutakahirata approved these changes May 19, 2025

View reviewed changes

teresajohnson approved these changes May 19, 2025

View reviewed changes

snehasish force-pushed the users/snehasish/05-16-_nfc_memprof_move_getguid_out_of_indexedmemprofrecord branch from 83ff2ba to 6a83dcd Compare May 19, 2025 22:15

snehasish force-pushed the users/snehasish/05-16-_nfc_memprof_move_radix_tree_methods_to_their_own_header_and_cpp branch from c8e520c to 1efadfb Compare May 19, 2025 22:15

snehasish force-pushed the users/snehasish/05-16-_nfc_memprof_move_radix_tree_methods_to_their_own_header_and_cpp branch from 1efadfb to 45937ca Compare May 19, 2025 23:13

Base automatically changed from users/snehasish/05-16-_nfc_memprof_move_radix_tree_methods_to_their_own_header_and_cpp to main May 19, 2025 23:16

snehasish added 2 commits May 19, 2025 23:16

[NFC][MemProf] Move getGUID out of IndexedMemProfRecord

b570e3a

Fix formatting.

6ec85a1

snehasish force-pushed the users/snehasish/05-16-_nfc_memprof_move_getguid_out_of_indexedmemprofrecord branch from 6a83dcd to 6ec85a1 Compare May 19, 2025 23:16

snehasish merged commit ad3c1d2 into main May 19, 2025
5 of 6 checks passed

snehasish deleted the users/snehasish/05-16-_nfc_memprof_move_getguid_out_of_indexedmemprofrecord branch May 19, 2025 23:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC][MemProf] Move getGUID out of IndexedMemProfRecord #140502

[NFC][MemProf] Move getGUID out of IndexedMemProfRecord #140502

Uh oh!

snehasish commented May 19, 2025 •

edited

Loading

Uh oh!

snehasish commented May 19, 2025 •

edited

Loading

Uh oh!

llvmbot commented May 19, 2025 •

edited

Loading

Uh oh!

github-actions bot commented May 19, 2025 •

edited

Loading

Uh oh!

kazutakahirata left a comment

Uh oh!

kazutakahirata left a comment

Uh oh!

snehasish commented May 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Search code, repositories, users, issues, pull requests...

[NFC][MemProf] Move getGUID out of IndexedMemProfRecord #140502

[NFC][MemProf] Move getGUID out of IndexedMemProfRecord #140502

Uh oh!

Conversation

snehasish commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snehasish commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kazutakahirata left a comment

Choose a reason for hiding this comment

Uh oh!

kazutakahirata left a comment

Choose a reason for hiding this comment

Uh oh!

snehasish commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Uh oh!

snehasish commented May 19, 2025 •

edited

Loading

snehasish commented May 19, 2025 •

edited

Loading

llvmbot commented May 19, 2025 •

edited

Loading

github-actions bot commented May 19, 2025 •

edited

Loading

snehasish commented May 19, 2025 •

edited

Loading