[llvm-branch-commits] [llvm] [NFC][MemProf] Move getGUID out of IndexedMemProfRecord (PR #140502)
Snehasish Kumar via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon May 19 15:15:54 PDT 2025
https://github.com/snehasish updated https://github.com/llvm/llvm-project/pull/140502
>From 9028dc98ac740d72c9c6ad02e4503da5e9c02a13 Mon Sep 17 00:00:00 2001
From: Snehasish Kumar <snehasishk at google.com>
Date: Fri, 16 May 2025 20:20:00 -0700
Subject: [PATCH 1/2] [NFC][MemProf] Move getGUID out of IndexedMemProfRecord
---
llvm/include/llvm/ProfileData/MemProf.h | 10 ++--
llvm/include/llvm/ProfileData/MemProfYAML.h | 2 +-
llvm/lib/ProfileData/MemProf.cpp | 2 +-
llvm/lib/ProfileData/MemProfReader.cpp | 2 +-
.../Instrumentation/MemProfiler.cpp | 4 +-
llvm/unittests/ProfileData/MemProfTest.cpp | 20 ++++----
.../Instrumentation/MemProfUseTest.cpp | 48 +++++++++----------
7 files changed, 44 insertions(+), 44 deletions(-)
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 26b09698c7ea3..201ee2d7272cf 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,
>From 6a83dcd4ce2e7952263eeea4ceb2d94ebe25a3a1 Mon Sep 17 00:00:00 2001
From: Snehasish Kumar <snehasishk at google.com>
Date: Mon, 19 May 2025 15:14:12 -0700
Subject: [PATCH 2/2] Fix formatting.
---
llvm/lib/ProfileData/MemProfReader.cpp | 3 +--
.../Instrumentation/MemProfUseTest.cpp | 27 ++++++++-----------
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/ProfileData/MemProfReader.cpp b/llvm/lib/ProfileData/MemProfReader.cpp
index aca534b0a4c98..d6bc4fdf5e448 100644
--- a/llvm/lib/ProfileData/MemProfReader.cpp
+++ b/llvm/lib/ProfileData/MemProfReader.cpp
@@ -569,8 +569,7 @@ Error RawMemProfReader::symbolizeAndFilterStackFrames(
for (size_t I = 0, NumFrames = DI.getNumberOfFrames(); I < NumFrames;
I++) {
const auto &DIFrame = DI.getFrame(I);
- const uint64_t Guid =
- memprof::getGUID(DIFrame.FunctionName);
+ const uint64_t Guid = 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/unittests/Transforms/Instrumentation/MemProfUseTest.cpp b/llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp
index 95828356b490b..2ed32c6ea0eac 100644
--- a/llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp
+++ b/llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp
@@ -107,10 +107,9 @@ declare !dbg !19 void @_Z2f3v()
// locations.
EXPECT_THAT(
CallSites,
- ElementsAre(
- Pair(LineLocation(1, 3), memprof::getGUID("_Z2f1v")),
- Pair(LineLocation(2, 3), memprof::getGUID("_Z2f2v")),
- Pair(LineLocation(2, 9), memprof::getGUID("_Z2f3v"))));
+ ElementsAre(Pair(LineLocation(1, 3), memprof::getGUID("_Z2f1v")),
+ Pair(LineLocation(2, 3), memprof::getGUID("_Z2f2v")),
+ Pair(LineLocation(2, 9), memprof::getGUID("_Z2f3v"))));
}
TEST(MemProf, ExtractDirectCallsFromIRInline) {
@@ -206,25 +205,22 @@ declare !dbg !25 void @_Z2g2v() local_unnamed_addr
EXPECT_EQ(FooCallerGUID, memprof::getGUID("_Z3foov"));
EXPECT_THAT(
FooCallSites,
- ElementsAre(
- Pair(LineLocation(1, 3), memprof::getGUID("_ZL2f3v")),
- Pair(LineLocation(2, 9), memprof::getGUID("_ZL2g3v"))));
+ ElementsAre(Pair(LineLocation(1, 3), memprof::getGUID("_ZL2f3v")),
+ Pair(LineLocation(2, 9), memprof::getGUID("_ZL2g3v"))));
auto F2It = Calls.find(memprof::getGUID("_ZL2f2v"));
ASSERT_NE(F2It, Calls.end());
const auto &[F2CallerGUID, F2CallSites] = *F2It;
EXPECT_EQ(F2CallerGUID, memprof::getGUID("_ZL2f2v"));
- EXPECT_THAT(F2CallSites,
- ElementsAre(Pair(LineLocation(2, 3),
- memprof::getGUID("_Z2f1v"))));
+ EXPECT_THAT(F2CallSites, ElementsAre(Pair(LineLocation(2, 3),
+ memprof::getGUID("_Z2f1v"))));
auto F3It = Calls.find(memprof::getGUID("_ZL2f3v"));
ASSERT_NE(F3It, Calls.end());
const auto &[F3CallerGUID, F3CallSites] = *F3It;
EXPECT_EQ(F3CallerGUID, memprof::getGUID("_ZL2f3v"));
- EXPECT_THAT(F3CallSites,
- ElementsAre(Pair(LineLocation(1, 10),
- memprof::getGUID("_ZL2f2v"))));
+ EXPECT_THAT(F3CallSites, ElementsAre(Pair(LineLocation(1, 10),
+ memprof::getGUID("_ZL2f2v"))));
auto G3It = Calls.find(memprof::getGUID("_ZL2g3v"));
ASSERT_NE(G3It, Calls.end());
@@ -232,9 +228,8 @@ declare !dbg !25 void @_Z2g2v() local_unnamed_addr
EXPECT_EQ(G3CallerGUID, memprof::getGUID("_ZL2g3v"));
EXPECT_THAT(
G3CallSites,
- ElementsAre(
- Pair(LineLocation(1, 8), memprof::getGUID("_Z2g1v")),
- Pair(LineLocation(2, 3), memprof::getGUID("_Z2g2v"))));
+ ElementsAre(Pair(LineLocation(1, 8), memprof::getGUID("_Z2g1v")),
+ Pair(LineLocation(2, 3), memprof::getGUID("_Z2g2v"))));
}
TEST(MemProf, ExtractDirectCallsFromIRCallingNew) {
More information about the llvm-branch-commits
mailing list