Skip to content

Commit ad3c1d2

Browse files
snehasishsnehasish
authored
[NFC][MemProf] Move getGUID out of IndexedMemProfRecord (#140502)
Part of a larger refactoring with the following goals 1. Reduce the size of MemProf.h 2. Avoid including ModuleSummaryIndex just for a couple of types
1 parent a53b306 commit ad3c1d2

File tree

7 files changed

+46
-52
lines changed

7 files changed

+46
-52
lines changed

‎llvm/include/llvm/ProfileData/MemProf.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -472,13 +472,13 @@ struct IndexedMemProfRecord {
472472
// translate CallStackId to call stacks with frames inline.
473473
MemProfRecord toMemProfRecord(
474474
llvm::function_ref<std::vector<Frame>(const CallStackId)> Callback) const;
475-
476-
// Returns the GUID for the function name after canonicalization. For
477-
// memprof, we remove any .llvm suffix added by LTO. MemProfRecords are
478-
// mapped to functions using this GUID.
479-
static GlobalValue::GUID getGUID(const StringRef FunctionName);
480475
};
481476

477+
// Returns the GUID for the function name after canonicalization. For
478+
// memprof, we remove any .llvm suffix added by LTO. MemProfRecords are
479+
// mapped to functions using this GUID.
480+
GlobalValue::GUID getGUID(const StringRef FunctionName);
481+
482482
// Holds call site information with frame contents inline.
483483
struct CallSiteInfo {
484484
// The frames in the call stack

‎llvm/include/llvm/ProfileData/MemProfYAML.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ template <> struct ScalarTraits<memprof::GUIDHex64> {
4646
Val = Num;
4747
} else {
4848
// Otherwise, treat the input as a string containing a function name.
49-
Val = memprof::IndexedMemProfRecord::getGUID(Scalar);
49+
Val = memprof::getGUID(Scalar);
5050
}
5151
return StringRef();
5252
}

‎llvm/lib/ProfileData/MemProf.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ MemProfRecord IndexedMemProfRecord::toMemProfRecord(
343343
return Record;
344344
}
345345

346-
GlobalValue::GUID IndexedMemProfRecord::getGUID(const StringRef FunctionName) {
346+
GlobalValue::GUID getGUID(const StringRef FunctionName) {
347347
// Canonicalize the function name to drop suffixes such as ".llvm.". Note
348348
// we do not drop any ".__uniq." suffixes, as getCanonicalFnName does not drop
349349
// those by default. This is by design to differentiate internal linkage

‎llvm/lib/ProfileData/MemProfReader.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -569,8 +569,7 @@ Error RawMemProfReader::symbolizeAndFilterStackFrames(
569569
for (size_t I = 0, NumFrames = DI.getNumberOfFrames(); I < NumFrames;
570570
I++) {
571571
const auto &DIFrame = DI.getFrame(I);
572-
const uint64_t Guid =
573-
IndexedMemProfRecord::getGUID(DIFrame.FunctionName);
572+
const uint64_t Guid = memprof::getGUID(DIFrame.FunctionName);
574573
const Frame F(Guid, DIFrame.Line - DIFrame.StartLine, DIFrame.Column,
575574
// Only the last entry is not an inlined location.
576575
I != NumFrames - 1);

‎llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -865,8 +865,8 @@ memprof::extractCallsFromIR(Module &M, const TargetLibraryInfo &TLI,
865865
StringRef CallerName = DIL->getSubprogramLinkageName();
866866
assert(!CallerName.empty() &&
867867
"Be sure to enable -fdebug-info-for-profiling");
868-
uint64_t CallerGUID = IndexedMemProfRecord::getGUID(CallerName);
869-
uint64_t CalleeGUID = IndexedMemProfRecord::getGUID(CalleeName);
868+
uint64_t CallerGUID = memprof::getGUID(CallerName);
869+
uint64_t CalleeGUID = memprof::getGUID(CalleeName);
870870
// Pretend that we are calling a function with GUID == 0 if we are
871871
// in the inline stack leading to a heap allocation function.
872872
if (IsAlloc) {

‎llvm/unittests/ProfileData/MemProfTest.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ const DILineInfoSpecifier specifier() {
107107
MATCHER_P4(FrameContains, FunctionName, LineOffset, Column, Inline, "") {
108108
const Frame &F = arg;
109109

110-
const uint64_t ExpectedHash = IndexedMemProfRecord::getGUID(FunctionName);
110+
const uint64_t ExpectedHash = memprof::getGUID(FunctionName);
111111
if (F.Function != ExpectedHash) {
112112
*result_listener << "Hash mismatch";
113113
return false;
@@ -180,7 +180,7 @@ TEST(MemProf, FillsValue) {
180180
ASSERT_THAT(Records, SizeIs(4));
181181

182182
// Check the memprof record for foo.
183-
const llvm::GlobalValue::GUID FooId = IndexedMemProfRecord::getGUID("foo");
183+
const llvm::GlobalValue::GUID FooId = memprof::getGUID("foo");
184184
ASSERT_TRUE(Records.contains(FooId));
185185
const MemProfRecord &Foo = Records[FooId];
186186
ASSERT_THAT(Foo.AllocSites, SizeIs(1));
@@ -196,7 +196,7 @@ TEST(MemProf, FillsValue) {
196196
EXPECT_TRUE(Foo.CallSites.empty());
197197

198198
// Check the memprof record for bar.
199-
const llvm::GlobalValue::GUID BarId = IndexedMemProfRecord::getGUID("bar");
199+
const llvm::GlobalValue::GUID BarId = memprof::getGUID("bar");
200200
ASSERT_TRUE(Records.contains(BarId));
201201
const MemProfRecord &Bar = Records[BarId];
202202
ASSERT_THAT(Bar.AllocSites, SizeIs(1));
@@ -217,7 +217,7 @@ TEST(MemProf, FillsValue) {
217217
FrameContains("bar", 51U, 20U, false)))));
218218

219219
// Check the memprof record for xyz.
220-
const llvm::GlobalValue::GUID XyzId = IndexedMemProfRecord::getGUID("xyz");
220+
const llvm::GlobalValue::GUID XyzId = memprof::getGUID("xyz");
221221
ASSERT_TRUE(Records.contains(XyzId));
222222
const MemProfRecord &Xyz = Records[XyzId];
223223
// Expect the entire frame even though in practice we only need the first
@@ -229,7 +229,7 @@ TEST(MemProf, FillsValue) {
229229
FrameContains("abc", 5U, 30U, false)))));
230230

231231
// Check the memprof record for abc.
232-
const llvm::GlobalValue::GUID AbcId = IndexedMemProfRecord::getGUID("abc");
232+
const llvm::GlobalValue::GUID AbcId = memprof::getGUID("abc");
233233
ASSERT_TRUE(Records.contains(AbcId));
234234
const MemProfRecord &Abc = Records[AbcId];
235235
EXPECT_TRUE(Abc.AllocSites.empty());
@@ -461,9 +461,9 @@ TEST(MemProf, SymbolizationFilter) {
461461

462462
TEST(MemProf, BaseMemProfReader) {
463463
IndexedMemProfData MemProfData;
464-
Frame F1(/*Hash=*/IndexedMemProfRecord::getGUID("foo"), /*LineOffset=*/20,
464+
Frame F1(/*Hash=*/memprof::getGUID("foo"), /*LineOffset=*/20,
465465
/*Column=*/5, /*IsInlineFrame=*/true);
466-
Frame F2(/*Hash=*/IndexedMemProfRecord::getGUID("bar"), /*LineOffset=*/10,
466+
Frame F2(/*Hash=*/memprof::getGUID("bar"), /*LineOffset=*/10,
467467
/*Column=*/2, /*IsInlineFrame=*/false);
468468
auto F1Id = MemProfData.addFrame(F1);
469469
auto F2Id = MemProfData.addFrame(F2);
@@ -493,9 +493,9 @@ TEST(MemProf, BaseMemProfReader) {
493493

494494
TEST(MemProf, BaseMemProfReaderWithCSIdMap) {
495495
IndexedMemProfData MemProfData;
496-
Frame F1(/*Hash=*/IndexedMemProfRecord::getGUID("foo"), /*LineOffset=*/20,
496+
Frame F1(/*Hash=*/memprof::getGUID("foo"), /*LineOffset=*/20,
497497
/*Column=*/5, /*IsInlineFrame=*/true);
498-
Frame F2(/*Hash=*/IndexedMemProfRecord::getGUID("bar"), /*LineOffset=*/10,
498+
Frame F2(/*Hash=*/memprof::getGUID("bar"), /*LineOffset=*/10,
499499
/*Column=*/2, /*IsInlineFrame=*/false);
500500
auto F1Id = MemProfData.addFrame(F1);
501501
auto F2Id = MemProfData.addFrame(F2);
@@ -813,7 +813,7 @@ TEST(MemProf, YAMLParserGUID) {
813813
// Verify the entire contents of MemProfData.Records.
814814
ASSERT_THAT(MemProfData.Records, SizeIs(1));
815815
const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
816-
EXPECT_EQ(GUID, IndexedMemProfRecord::getGUID("_Z3fooi"));
816+
EXPECT_EQ(GUID, memprof::getGUID("_Z3fooi"));
817817

818818
IndexedCallstackIdConverter CSIdConv(MemProfData);
819819
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);

‎llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,15 @@ declare !dbg !19 void @_Z2f3v()
101101
ASSERT_NE(It, Calls.end());
102102

103103
const auto &[CallerGUID, CallSites] = *It;
104-
EXPECT_EQ(CallerGUID, IndexedMemProfRecord::getGUID("_Z3foov"));
104+
EXPECT_EQ(CallerGUID, memprof::getGUID("_Z3foov"));
105105

106106
// Verify that call sites show up in the ascending order of their source
107107
// locations.
108108
EXPECT_THAT(
109109
CallSites,
110-
ElementsAre(
111-
Pair(LineLocation(1, 3), IndexedMemProfRecord::getGUID("_Z2f1v")),
112-
Pair(LineLocation(2, 3), IndexedMemProfRecord::getGUID("_Z2f2v")),
113-
Pair(LineLocation(2, 9), IndexedMemProfRecord::getGUID("_Z2f3v"))));
110+
ElementsAre(Pair(LineLocation(1, 3), memprof::getGUID("_Z2f1v")),
111+
Pair(LineLocation(2, 3), memprof::getGUID("_Z2f2v")),
112+
Pair(LineLocation(2, 9), memprof::getGUID("_Z2f3v"))));
114113
}
115114

116115
TEST(MemProf, ExtractDirectCallsFromIRInline) {
@@ -200,41 +199,37 @@ declare !dbg !25 void @_Z2g2v() local_unnamed_addr
200199

201200
// Verify each key-value pair.
202201

203-
auto FooIt = Calls.find(IndexedMemProfRecord::getGUID("_Z3foov"));
202+
auto FooIt = Calls.find(memprof::getGUID("_Z3foov"));
204203
ASSERT_NE(FooIt, Calls.end());
205204
const auto &[FooCallerGUID, FooCallSites] = *FooIt;
206-
EXPECT_EQ(FooCallerGUID, IndexedMemProfRecord::getGUID("_Z3foov"));
205+
EXPECT_EQ(FooCallerGUID, memprof::getGUID("_Z3foov"));
207206
EXPECT_THAT(
208207
FooCallSites,
209-
ElementsAre(
210-
Pair(LineLocation(1, 3), IndexedMemProfRecord::getGUID("_ZL2f3v")),
211-
Pair(LineLocation(2, 9), IndexedMemProfRecord::getGUID("_ZL2g3v"))));
208+
ElementsAre(Pair(LineLocation(1, 3), memprof::getGUID("_ZL2f3v")),
209+
Pair(LineLocation(2, 9), memprof::getGUID("_ZL2g3v"))));
212210

213-
auto F2It = Calls.find(IndexedMemProfRecord::getGUID("_ZL2f2v"));
211+
auto F2It = Calls.find(memprof::getGUID("_ZL2f2v"));
214212
ASSERT_NE(F2It, Calls.end());
215213
const auto &[F2CallerGUID, F2CallSites] = *F2It;
216-
EXPECT_EQ(F2CallerGUID, IndexedMemProfRecord::getGUID("_ZL2f2v"));
217-
EXPECT_THAT(F2CallSites,
218-
ElementsAre(Pair(LineLocation(2, 3),
219-
IndexedMemProfRecord::getGUID("_Z2f1v"))));
214+
EXPECT_EQ(F2CallerGUID, memprof::getGUID("_ZL2f2v"));
215+
EXPECT_THAT(F2CallSites, ElementsAre(Pair(LineLocation(2, 3),
216+
memprof::getGUID("_Z2f1v"))));
220217

221-
auto F3It = Calls.find(IndexedMemProfRecord::getGUID("_ZL2f3v"));
218+
auto F3It = Calls.find(memprof::getGUID("_ZL2f3v"));
222219
ASSERT_NE(F3It, Calls.end());
223220
const auto &[F3CallerGUID, F3CallSites] = *F3It;
224-
EXPECT_EQ(F3CallerGUID, IndexedMemProfRecord::getGUID("_ZL2f3v"));
225-
EXPECT_THAT(F3CallSites,
226-
ElementsAre(Pair(LineLocation(1, 10),
227-
IndexedMemProfRecord::getGUID("_ZL2f2v"))));
221+
EXPECT_EQ(F3CallerGUID, memprof::getGUID("_ZL2f3v"));
222+
EXPECT_THAT(F3CallSites, ElementsAre(Pair(LineLocation(1, 10),
223+
memprof::getGUID("_ZL2f2v"))));
228224

229-
auto G3It = Calls.find(IndexedMemProfRecord::getGUID("_ZL2g3v"));
225+
auto G3It = Calls.find(memprof::getGUID("_ZL2g3v"));
230226
ASSERT_NE(G3It, Calls.end());
231227
const auto &[G3CallerGUID, G3CallSites] = *G3It;
232-
EXPECT_EQ(G3CallerGUID, IndexedMemProfRecord::getGUID("_ZL2g3v"));
228+
EXPECT_EQ(G3CallerGUID, memprof::getGUID("_ZL2g3v"));
233229
EXPECT_THAT(
234230
G3CallSites,
235-
ElementsAre(
236-
Pair(LineLocation(1, 8), IndexedMemProfRecord::getGUID("_Z2g1v")),
237-
Pair(LineLocation(2, 3), IndexedMemProfRecord::getGUID("_Z2g2v"))));
231+
ElementsAre(Pair(LineLocation(1, 8), memprof::getGUID("_Z2g1v")),
232+
Pair(LineLocation(2, 3), memprof::getGUID("_Z2g2v"))));
238233
}
239234

240235
TEST(MemProf, ExtractDirectCallsFromIRCallingNew) {
@@ -295,10 +290,10 @@ attributes #2 = { builtin allocsize(0) }
295290

296291
// Verify each key-value pair.
297292

298-
auto FooIt = Calls.find(IndexedMemProfRecord::getGUID("_Z3foov"));
293+
auto FooIt = Calls.find(memprof::getGUID("_Z3foov"));
299294
ASSERT_NE(FooIt, Calls.end());
300295
const auto &[FooCallerGUID, FooCallSites] = *FooIt;
301-
EXPECT_EQ(FooCallerGUID, IndexedMemProfRecord::getGUID("_Z3foov"));
296+
EXPECT_EQ(FooCallerGUID, memprof::getGUID("_Z3foov"));
302297
EXPECT_THAT(FooCallSites, ElementsAre(Pair(LineLocation(1, 10), 0)));
303298
}
304299

@@ -406,10 +401,10 @@ attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "t
406401
auto &TLI = WrapperPass.getTLI(*F);
407402
auto Calls = extractCallsFromIR(*M, TLI);
408403

409-
uint64_t GUIDFoo = IndexedMemProfRecord::getGUID("_Z3foov");
410-
uint64_t GUIDBar = IndexedMemProfRecord::getGUID("_Z3barv");
411-
uint64_t GUIDBaz = IndexedMemProfRecord::getGUID("_Z3bazv");
412-
uint64_t GUIDZzz = IndexedMemProfRecord::getGUID("_Z3zzzv");
404+
uint64_t GUIDFoo = memprof::getGUID("_Z3foov");
405+
uint64_t GUIDBar = memprof::getGUID("_Z3barv");
406+
uint64_t GUIDBaz = memprof::getGUID("_Z3bazv");
407+
uint64_t GUIDZzz = memprof::getGUID("_Z3zzzv");
413408

414409
// Verify that extractCallsFromIR extracts caller-callee pairs as expected.
415410
EXPECT_THAT(Calls,

0 commit comments

Comments
 (0)