[memprof] Introduce IndexedCallstackIdConveter (NFC) (#120540)

This patch introduces IndexedCallstackIdConveter as a convenience
wrapper around FrameIdConverter and CallStackIdConverter just for
t

[memprof] Introduce IndexedCallstackIdConveter (NFC) (#120540)

This patch introduces IndexedCallstackIdConveter as a convenience
wrapper around FrameIdConverter and CallStackIdConverter just for
tests.

With the new wrapper, we get to replace idioms like:

FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
MemProfData.Frames);
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
MemProfData.CallStacks, FrameIdConv);

with:

IndexedCallstackIdConveter CSIdConv(MemProfData);

Unfortunately, this exact pattern occurs in tests only; the
combinations of the frame ID converter and call stack ID converter are
diverse in production code.

show more ...

[memprof] Drop memprof:: in unit tests (NFC) (#119113)

This patch replaces memprof::Foo with Foo if we have corresponding:

using llvm::memprof::Foo;

[memprof] Add getMemProfDataForTest for unit tests (#119061)

We always call getFrameMapping and getCallStackMapping together in
InstrProfTest.cpp. This patch combines the two functions into new
f

[memprof] Add getMemProfDataForTest for unit tests (#119061)

We always call getFrameMapping and getCallStackMapping together in
InstrProfTest.cpp. This patch combines the two functions into new
function getMemProfDataForTest.

show more ...

[memprof] Remove a stale comment in a unit test (#119060)

We've removed IndexedAllocationInfo::CallStack, so we don't need to
mention it.

[memprof] Use gtest matchers at more places (#119050)

These gtest matchers reduce the number of times we mention the
variables under examined.

[memprof] Remove MemProf format Version 1 (#117357)

This patch removes MemProf format Version 1 now that Version 2 and 3
are working well.

[memprof] Upgrade a unit test to MemProf Version 3 (#117063)

This patch upgrades a unit test to MemProf Version 3 while removing
those bits that cannot be upgraded to Version 3.

The bits being r

[memprof] Upgrade a unit test to MemProf Version 3 (#117063)

This patch upgrades a unit test to MemProf Version 3 while removing
those bits that cannot be upgraded to Version 3.

The bits being removed expect instrprof_error::hash_mismatch from a
broken MemProf profile that references a frame that doesn't actually
exist. Now, Version 3 no longer issues
instrprof_error::hash_mismatch. Even if it still issued
instrprof_error::hash_mismatch, we would have a couple of hurdles:

- InstrProfWriter::addMemProfData will soon require all (or none) of
the fields (frames, call stacks, and records) be populated. That
is, it won't accept an instance of IndexedMemProfData with frames
missing.

- writeMemProfV3 asserts that every frame occurs at least once:

assert(MemProfData.Frames.size() == FrameHistogram.size());

This patch gives up on instrprof_error::hash_mismatch and tries to
trigger instrprof_error::unknown_function with the empty profile.

show more ...

[memprof] Use LineLocation in a unit test (NFC) (#117031)

We've switched to LineLocation from FieldsAre in MemProfUseTest.cpp.
This patch does the same thing in InstrProfTest.cpp.

llvm/unittests

[memprof] Use LineLocation in a unit test (NFC) (#117031)

We've switched to LineLocation from FieldsAre in MemProfUseTest.cpp.
This patch does the same thing in InstrProfTest.cpp.

llvm/unittests/Transforms/Instrumentation/MemProfUseTest.cpp

show more ...

[memprof] Add another constructor to IndexedAllocationInfo (NFC) (#116684)

This patch adds another constructor to IndexedAllocationInfo that is
identical to the existing constructor except that the

[memprof] Add another constructor to IndexedAllocationInfo (NFC) (#116684)

This patch adds another constructor to IndexedAllocationInfo that is
identical to the existing constructor except that the new one leaves
the CallStack field empty.

I'm planning to remove MemProf format Version 1. Then we will migrate
the users of the existing constructor to the new one as nobody will be
using the CallStack field anymore.

Adding the new constructor now allows us to migrate a few existing
users of the old constructor even before we remove the CallStack
field. In turn, that simplifies the patch to actually remove the
field.

show more ...

[memprof] Add InstrProfWriter::addMemProfData (#116528)

This patch adds InstrProfWriter::addMemProfData, which adds the
complete MemProf profile (frames, call stacks, and records) to the
writer co

[memprof] Add InstrProfWriter::addMemProfData (#116528)

This patch adds InstrProfWriter::addMemProfData, which adds the
complete MemProf profile (frames, call stacks, and records) to the
writer context.

Without this function, functions like loadInput in llvm-profdata.cpp
and InstrProfWriter::mergeRecordsFromWriter must add one item (frame,
call stack, or record) at a time. The new function std::moves the
entire MemProf profile to the writer context if the destination is
empty, which is the common use case. Otherwise, we fall back to
adding one item at a time behind the scene.

Here are a couple of reasons why we should add this function:

- We've had a bug where we forgot to add one of the three data
structures (frames, call stacks, and records) to the writer context,
resulting in a nearly empty indexed profile. We should always
package the three data structures together, especially on API
boundaries.

- We expose a little too much of the MemProf detail to
InstrProfWriter. I'd like to gradually transform
InstrProfReader/Writer to entities managing buffers (sequences of
bytes), with actual serialization/deserialization left to external
classes. We already do some of this in InstrProfReader, where
InstrProfReader "contracts out" to IndexedMemProfReader to handle
MemProf details.

I am not changing loadInput or InstrProfWriter::mergeRecordsFromWriter
for now because MemProfReader uses DenseMap for frames and call
stacks, whereas MemProfData uses MapVector. I'll resolve these
mismatches in subsequent patches.

show more ...

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

I'm planning to remove MemProf Version 1, which is a maintenance
burden because it uses a different set of struct fields in
IndexedAllocationIn

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

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.

show more ...

[memprof] Update a comment (NFC) (#116500)

Note that Version0 has been removed in #116442.

[memprof] Add IndexedMemProfReader::getMemProfCallerCalleePairs (#115807)

Undrifting the MemProf profile requires two sets of information:

- caller-callee pairs from the profile
- callee-callee

[memprof] Add IndexedMemProfReader::getMemProfCallerCalleePairs (#115807)

Undrifting the MemProf profile requires two sets of information:

- caller-callee pairs from the profile
- callee-callee pairs from the IR

This patch adds a function to do the former. The latter has been
addressed by extractCallsFromIR.

Unfortunately, the current MemProf format does not directly give us
the caller-callee pairs from the profile. "struct Frame" just tells
us where the call site is -- Caller GUID and line/column numbers; it
doesn't tell us what function a given Frame is calling. To extract
caller-callee pairs, we need to scan each call stack, look at two
adjacent Frames, and extract a caller-callee pair.

Conceptually, we would extract caller-callee pairs with:

for each MemProfRecord in the profile:
for each call stack in AllocSites:
extract caller-callee pairs from adjacent pairs of Frames

However, this is highly inefficient. Obtaining MemProfRecord involves
looking up the OnDiskHashTable, allocating several vectors on the
heap, and populating fields that are irrelevant to us, such as MIB and
CallSites.

This patch adds an efficient way of doing the above. Specifically, we

- go though all IndexedMemProfRecords,
- look at each linear call stack ID
- extract caller-callee pairs from each call stack

The extraction is done by a new class CallerCalleePairExtractor,
modified from LinearCallStackIdConverter, which reconstructs a call
stack from the radix tree array. For our purposes, we skip the
reconstruction and immediately populates the data structure for
caller-callee pairs.

The resulting caller-callee-pairs is of the type:

DenseMap<uint64_t, SmallVector<CallEdgeTy, 0>> CallerCalleePairs;

which can be passed directly to longestCommonSequence just like the
result of extractCallsFromIR.

Further performance optimizations are possible for the new functions
in this patch. I'll address those in follow-up patches.

show more ...

[unittests] Use {} instead of std::nullopt to initialize empty ArrayRef (#109388)

Follow up to #109133.

[llvm][unittests] Don't call raw_string_ostream::flush() (NFC)

raw_string_ostream::flush() is essentially a no-op (also specified in docs).
Don't call it in tests that aren't meant to test 'raw_stri

[llvm][unittests] Don't call raw_string_ostream::flush() (NFC)

raw_string_ostream::flush() is essentially a no-op (also specified in docs).
Don't call it in tests that aren't meant to test 'raw_string_ostream' itself.

p.s. remove a few redundant calls to raw_string_ostream::str()

show more ...

[ProfileData] Take ArrayRef<InstrProfValueData> in addValueData (NFC) (#97363)

This patch fixes another place in ProfileData where we have a pointer
to an array of InstrProfValueData and its length

[ProfileData] Take ArrayRef<InstrProfValueData> in addValueData (NFC) (#97363)

This patch fixes another place in ProfileData where we have a pointer
to an array of InstrProfValueData and its length separately.

addValueData is a bit unique in that it remaps incoming values in
place before adding them to ValueSites. AFAICT, no caller of
addValueData uses updated incoming values. With this patch, we add
value data to ValueSites first and then remaps values there. This
way, we can take ArrayRef<InstrProfValueData> as a parameter.

show more ...

[ProfileData] Migrate to a new version of getValueProfDataFromInst (#96381)

[ProfileData] Teach addValueData to honor parameter Site (#96233)

This patch teaches addValueData to honor Site for verification
purposes. It does not affect the profile data in any manner.

[ProfileData] Sink the length checks (#95604)

The new API getValueArrayForSite returns ArrayRef<InstrProfValueData>,
packaging the array length and contents together.

This patch sinks the array

[ProfileData] Sink the length checks (#95604)

The new API getValueArrayForSite returns ArrayRef<InstrProfValueData>,
packaging the array length and contents together.

This patch sinks the array length checks just before we check the
contents. This way, we check both the array length and contents
immediately after calling getValueArrayForSite.

show more ...

[ProfileData] Migrate to a new version of getValueProfDataFromInst (#95568)

Note that the version of getValueProfDataFromInst that returns bool
has been "deprecated" since:

commit 1e15371dd884

[ProfileData] Migrate to a new version of getValueProfDataFromInst (#95568)

Note that the version of getValueProfDataFromInst that returns bool
has been "deprecated" since:

commit 1e15371dd8843dfc52b9435afaa133997c1773d8
Author: Mingming Liu <mingmingl@google.com>
Date: Mon Apr 1 15:14:49 2024 -0700

show more ...

[ProfileData] Sink the length checks (#95559)

The new API getValueArrayForSite returns ArrayRef<InstrProfValueData>,
packaging the array length and contents together.

This patch sinks the array

[ProfileData] Sink the length checks (#95559)

The new API getValueArrayForSite returns ArrayRef<InstrProfValueData>,
packaging the array length and contents together.

This patch sinks the array length checks just before we check the
contents. This way, we check both the array length and contents
immediately after calling getValueArrayForSite.

show more ...

[ProfileData] Fix the order of tests (#95549)

Without this patch, we call getValueForSite before veryfing that we
have an expected number of value sites with getNumValueSites.

This patch fixes t

[ProfileData] Fix the order of tests (#95549)

Without this patch, we call getValueForSite before veryfing that we
have an expected number of value sites with getNumValueSites.

This patch fixes the order by "sinking" the call to getValueForSite.

While I am at it, this patch migrates the use of getValueForSite to
getValueArrayForSite.

show more ...

[ProfileData] Migrate to getValueArrayForSite (#95493)

This patch migrates uses of getValueForSite to getValueArrayForSite.
Each hunk is self-contained, meaning that each one can be applied
indepe

[ProfileData] Migrate to getValueArrayForSite (#95493)

This patch migrates uses of getValueForSite to getValueArrayForSite.
Each hunk is self-contained, meaning that each one can be applied
independently of the others.

In the unit test, there are cases where the array length check is
performed a lot earlier than the array content check. For now, I'm
leaving the length checks where they are. I'll consider moving them
when I migrate uses of getNumValueDataForSite to getValueArrayForSite
in a follow-up patch.

show more ...

[ProfileData] Migrate to getValueArrayForSite (#95457)

This patch is a collection of one-liner migrations to
getValueArrayForSite.

[ProfileData] Add getValueArrayForSite (#95335)

Without this patch, a typical traversal over the value data looks
like:

uint32_t NV = Func.getNumValueDataForSite(VK, S);
std::unique_ptr<Instr

[ProfileData] Add getValueArrayForSite (#95335)

Without this patch, a typical traversal over the value data looks
like:

uint32_t NV = Func.getNumValueDataForSite(VK, S);
std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
for (uint32_t V = 0; V < NV; V++)
Do something with VD[V].Value and/or VD[V].Count;

This patch adds getValueArrayForSite, which returns
ArrayRef<InstrProfValueData>, so we can do:

for (const auto &V : Func.getValueArrayForSite(VK, S))
Do something with V.Value and/or V.Count;

I'm planning to migrate the existing uses of getValueForSite to
getValueArrayForSite in follow-up patches and remove getValueForSite
and getNumValueDataForSite.

show more ...