Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 19, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b570e3a
[NFC][MemProf] Move getGUID out of IndexedMemProfRecord
snehasish May 17, 2025
6ec85a1
Fix formatting.
snehasish May 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions llvm/include/llvm/ProfileData/MemProf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/ProfileData/MemProfYAML.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/ProfileData/MemProf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/ProfileData/MemProfReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
IndexedMemProfRecord::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);
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
20 changes: 10 additions & 10 deletions llvm/unittests/ProfileData/MemProfTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
57 changes: 26 additions & 31 deletions llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,15 @@ 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"))));
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) {
Expand Down Expand Up @@ -200,41 +199,37 @@ 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"))));
ElementsAre(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_THAT(F2CallSites,
ElementsAre(Pair(LineLocation(2, 3),
IndexedMemProfRecord::getGUID("_Z2f1v"))));
EXPECT_EQ(F2CallerGUID, memprof::getGUID("_ZL2f2v"));
EXPECT_THAT(F2CallSites, ElementsAre(Pair(LineLocation(2, 3),
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_THAT(F3CallSites,
ElementsAre(Pair(LineLocation(1, 10),
IndexedMemProfRecord::getGUID("_ZL2f2v"))));
EXPECT_EQ(F3CallerGUID, memprof::getGUID("_ZL2f3v"));
EXPECT_THAT(F3CallSites, ElementsAre(Pair(LineLocation(1, 10),
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"))));
ElementsAre(Pair(LineLocation(1, 8), memprof::getGUID("_Z2g1v")),
Pair(LineLocation(2, 3), memprof::getGUID("_Z2g2v"))));
}

TEST(MemProf, ExtractDirectCallsFromIRCallingNew) {
Expand Down Expand Up @@ -295,10 +290,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)));
}

Expand Down Expand Up @@ -406,10 +401,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,
Expand Down