-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[MemProf] Change histogram storage from uint64_t to uint8_t #147854
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.
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
base: main
Are you sure you want to change the base?
[MemProf] Change histogram storage from uint64_t to uint8_t #147854
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
38dfa22
to
3371ae2
Compare
@llvm/pr-subscribers-pgo Author: Snehasish Kumar (snehasish) ChangesPatch is 20.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147854.diff 26 Files Affected:
diff --git a/compiler-rt/include/profile/MemProfData.inc b/compiler-rt/include/profile/MemProfData.inc
index 3f785bd23fce3..4a43f72c2405d 100644
--- a/compiler-rt/include/profile/MemProfData.inc
+++ b/compiler-rt/include/profile/MemProfData.inc
@@ -33,11 +33,11 @@
(uint64_t)'o' << 24 | (uint64_t)'f' << 16 | (uint64_t)'r' << 8 | (uint64_t)129)
// The version number of the raw binary format.
-#define MEMPROF_RAW_VERSION 4ULL
+#define MEMPROF_RAW_VERSION 5ULL
// Currently supported versions.
#define MEMPROF_RAW_SUPPORTED_VERSIONS \
- { 3ULL, 4ULL }
+ { 3ULL, 4ULL, 5ULL }
#define MEMPROF_V3_MIB_SIZE 132ULL;
@@ -219,7 +219,11 @@ void Merge(const MemInfoBlock &newMIB) {
ShorterHistogramSize = newMIB.AccessHistogramSize;
}
for (size_t i = 0; i < ShorterHistogramSize; ++i) {
- ((uint64_t *)AccessHistogram)[i] += ((uint64_t *)ShorterHistogram)[i];
+ // Cast to uint8_t* and cap the sum at 255 to prevent overflow
+ uint8_t *CurrentHistPtr = (uint8_t *)AccessHistogram;
+ uint8_t *ShorterHistPtr = (uint8_t *)ShorterHistogram;
+ uint32_t sum = CurrentHistPtr[i] + ShorterHistPtr[i];
+ CurrentHistPtr[i] = (sum > 255) ? 255 : (uint8_t)sum;
}
}
diff --git a/compiler-rt/lib/memprof/memprof_allocator.cpp b/compiler-rt/lib/memprof/memprof_allocator.cpp
index 60f5c853f9d76..c5cef5cde9466 100644
--- a/compiler-rt/lib/memprof/memprof_allocator.cpp
+++ b/compiler-rt/lib/memprof/memprof_allocator.cpp
@@ -77,7 +77,7 @@ void Print(const MemInfoBlock &M, const u64 id, bool print_terse) {
? MAX_HISTOGRAM_PRINT_SIZE
: M.AccessHistogramSize;
for (size_t i = 0; i < PrintSize; ++i) {
- Printf("%llu ", ((uint64_t *)M.AccessHistogram)[i]);
+ Printf("%u ", ((uint8_t *)M.AccessHistogram)[i]);
}
Printf("\n");
}
@@ -327,12 +327,13 @@ struct Allocator {
uint32_t HistogramSize =
RoundUpTo(user_size, HISTOGRAM_GRANULARITY) / HISTOGRAM_GRANULARITY;
uintptr_t Histogram =
- (uintptr_t)InternalAlloc(HistogramSize * sizeof(uint64_t));
- memset((void *)Histogram, 0, HistogramSize * sizeof(uint64_t));
+ (uintptr_t)InternalAlloc(HistogramSize * sizeof(uint8_t));
+ memset((void *)Histogram, 0, HistogramSize * sizeof(uint8_t));
for (size_t i = 0; i < HistogramSize; ++i) {
u8 Counter =
*((u8 *)HISTOGRAM_MEM_TO_SHADOW(p + HISTOGRAM_GRANULARITY * i));
- ((uint64_t *)Histogram)[i] = (uint64_t)Counter;
+ // Cap the counter at HISTOGRAM_MAX_COUNTER (255) to prevent overflow
+ ((uint8_t *)Histogram)[i] = (Counter > HISTOGRAM_MAX_COUNTER) ? HISTOGRAM_MAX_COUNTER : Counter;
}
MemInfoBlock newMIB(user_size, c, m->timestamp_ms, curtime, m->cpu_id,
GetCpuId(), Histogram, HistogramSize);
diff --git a/compiler-rt/lib/memprof/memprof_rawprofile.cpp b/compiler-rt/lib/memprof/memprof_rawprofile.cpp
index a897648584828..95f22ffc5c42a 100644
--- a/compiler-rt/lib/memprof/memprof_rawprofile.cpp
+++ b/compiler-rt/lib/memprof/memprof_rawprofile.cpp
@@ -171,7 +171,8 @@ void SerializeMIBInfoToBuffer(MIBMapTy &MIBMap, const Vector<u64> &StackIds,
// deserialization.
Ptr = WriteBytes((*h)->mib, Ptr);
for (u64 j = 0; j < (*h)->mib.AccessHistogramSize; ++j) {
- u64 HistogramEntry = ((u64 *)((*h)->mib.AccessHistogram))[j];
+ // Read as uint8_t and write as uint8_t
+ uint8_t HistogramEntry = ((uint8_t *)((*h)->mib.AccessHistogram))[j];
Ptr = WriteBytes(HistogramEntry, Ptr);
}
if ((*h)->mib.AccessHistogramSize > 0) {
@@ -249,7 +250,7 @@ u64 SerializeToRawProfile(MIBMapTy &MIBMap, ArrayRef<LoadedModule> Modules,
},
reinterpret_cast<void *>(&TotalAccessHistogramEntries));
const u64 NumHistogramBytes =
- RoundUpTo(TotalAccessHistogramEntries * sizeof(uint64_t), 8);
+ RoundUpTo(TotalAccessHistogramEntries * sizeof(uint8_t), 8);
const u64 NumStackBytes = RoundUpTo(StackSizeBytes(StackIds), 8);
diff --git a/compiler-rt/test/memprof/TestCases/memprof_histogram_uint8.cpp b/compiler-rt/test/memprof/TestCases/memprof_histogram_uint8.cpp
new file mode 100644
index 0000000000000..ec3e4494feaca
--- /dev/null
+++ b/compiler-rt/test/memprof/TestCases/memprof_histogram_uint8.cpp
@@ -0,0 +1,32 @@
+// RUN: %clangxx_memprof -O0 -mllvm -memprof-histogram -mllvm -memprof-use-callbacks=true %s -o %t && %env_memprof_opts=print_text=1:histogram=1:log_path=stdout %run %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+#include <stdlib.h>
+
+int main() {
+ // Allocate memory that will create a histogram
+ char *buffer = (char *)malloc(1024);
+ if (!buffer) return 1;
+
+ for (int i = 0; i < 10; ++i) {
+ // Access every 8th byte (since shadow granularity is 8b.
+ buffer[i * 8] = 'A';
+ }
+
+ for (int j = 0; j < 200; ++j) {
+ buffer[8] = 'B'; // Count = previous count + 200
+ }
+
+ for (int j = 0; j < 400; ++j) {
+ buffer[16] = 'B'; // Count is saturated at 255
+ }
+
+ // Free the memory to trigger MIB creation with histogram
+ free(buffer);
+
+ printf("Test completed successfully\n");
+ return 0;
+}
+
+// CHECK: AccessCountHistogram[128]: 1 201 255 1 1 1 1 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
+// CHECK: Test completed successfully
diff --git a/llvm/include/llvm/ProfileData/MemProfData.inc b/llvm/include/llvm/ProfileData/MemProfData.inc
index 3f785bd23fce3..4a43f72c2405d 100644
--- a/llvm/include/llvm/ProfileData/MemProfData.inc
+++ b/llvm/include/llvm/ProfileData/MemProfData.inc
@@ -33,11 +33,11 @@
(uint64_t)'o' << 24 | (uint64_t)'f' << 16 | (uint64_t)'r' << 8 | (uint64_t)129)
// The version number of the raw binary format.
-#define MEMPROF_RAW_VERSION 4ULL
+#define MEMPROF_RAW_VERSION 5ULL
// Currently supported versions.
#define MEMPROF_RAW_SUPPORTED_VERSIONS \
- { 3ULL, 4ULL }
+ { 3ULL, 4ULL, 5ULL }
#define MEMPROF_V3_MIB_SIZE 132ULL;
@@ -219,7 +219,11 @@ void Merge(const MemInfoBlock &newMIB) {
ShorterHistogramSize = newMIB.AccessHistogramSize;
}
for (size_t i = 0; i < ShorterHistogramSize; ++i) {
- ((uint64_t *)AccessHistogram)[i] += ((uint64_t *)ShorterHistogram)[i];
+ // Cast to uint8_t* and cap the sum at 255 to prevent overflow
+ uint8_t *CurrentHistPtr = (uint8_t *)AccessHistogram;
+ uint8_t *ShorterHistPtr = (uint8_t *)ShorterHistogram;
+ uint32_t sum = CurrentHistPtr[i] + ShorterHistPtr[i];
+ CurrentHistPtr[i] = (sum > 255) ? 255 : (uint8_t)sum;
}
}
diff --git a/llvm/lib/ProfileData/MemProfReader.cpp b/llvm/lib/ProfileData/MemProfReader.cpp
index 235b1347e0077..28e1cc2a4b665 100644
--- a/llvm/lib/ProfileData/MemProfReader.cpp
+++ b/llvm/lib/ProfileData/MemProfReader.cpp
@@ -166,6 +166,90 @@ readMemInfoBlocksV4(const char *Ptr) {
return Items;
}
+llvm::SmallVector<std::pair<uint64_t, MemInfoBlock>>
+readMemInfoBlocksV5(const char *Ptr) {
+ using namespace support;
+
+ const uint64_t NumItemsToRead =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+
+ llvm::SmallVector<std::pair<uint64_t, MemInfoBlock>> Items;
+ for (uint64_t I = 0; I < NumItemsToRead; I++) {
+ const uint64_t Id =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+
+ MemInfoBlock MIB;
+ MIB.AllocCount =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.TotalAccessCount =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MinAccessCount =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MaxAccessCount =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.TotalSize =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MinSize =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MaxSize =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.AllocTimestamp =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.DeallocTimestamp =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.TotalLifetime =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MinLifetime =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MaxLifetime =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.AllocCpuId =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.DeallocCpuId =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.NumMigratedCpu =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.NumLifetimeOverlaps =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.NumSameAllocCpu =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.NumSameDeallocCpu =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.DataTypeId =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.TotalAccessDensity =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MinAccessDensity =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MaxAccessDensity =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.TotalLifetimeAccessDensity =
+ endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MinLifetimeAccessDensity =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.MaxLifetimeAccessDensity =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.AccessHistogramSize =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(Ptr);
+ MIB.AccessHistogram =
+ endian::readNext<uintptr_t, llvm::endianness::little, unaligned>(Ptr);
+
+ if (MIB.AccessHistogramSize > 0) {
+ // The in-memory representation uses uint64_t for histogram entries.
+ MIB.AccessHistogram =
+ (uintptr_t)malloc(MIB.AccessHistogramSize * sizeof(uint64_t));
+ for (uint64_t J = 0; J < MIB.AccessHistogramSize; J++) {
+ // The on-disk format for V5 uses uint8_t.
+ const uint8_t Val =
+ endian::readNext<uint8_t, llvm::endianness::little, unaligned>(Ptr);
+ ((uint64_t *)MIB.AccessHistogram)[J] = Val;
+ }
+ }
+ Items.push_back({Id, MIB});
+ }
+ return Items;
+}
+
CallStackMap readStackInfo(const char *Ptr) {
using namespace support;
@@ -658,6 +742,8 @@ RawMemProfReader::readMemInfoBlocks(const char *Ptr) {
return readMemInfoBlocksV3(Ptr);
if (MemprofRawVersion == 4ULL)
return readMemInfoBlocksV4(Ptr);
+ if (MemprofRawVersion == 5ULL)
+ return readMemInfoBlocksV5(Ptr);
llvm_unreachable(
"Panic: Unsupported version number when reading MemInfoBlocks");
}
diff --git a/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofexe b/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofexe
index f69c0b12a89eb..1c612f70bf940 100755
Binary files a/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofexe and b/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofexe differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofraw b/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofraw
index ed679dc49c53b..09fb96ace7e1e 100644
Binary files a/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofraw and b/llvm/test/tools/llvm-profdata/Inputs/basic-histogram.memprofraw differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/basic.memprofexe b/llvm/test/tools/llvm-profdata/Inputs/basic.memprofexe
index 14cbfeb88eaf8..8810ee1090869 100755
Binary files a/llvm/test/tools/llvm-profdata/Inputs/basic.memprofexe and b/llvm/test/tools/llvm-profdata/Inputs/basic.memprofexe differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/basic.memprofraw b/llvm/test/tools/llvm-profdata/Inputs/basic.memprofraw
index c3ac49e8079e9..6943c18c74792 100644
Binary files a/llvm/test/tools/llvm-profdata/Inputs/basic.memprofraw and b/llvm/test/tools/llvm-profdata/Inputs/basic.memprofraw differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofexe b/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofexe
index 1b4db88d8186d..4ab80401496fe 100755
Binary files a/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofexe and b/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofexe differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofraw b/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofraw
index e959e7679f56c..c6aec8d0b59e1 100644
Binary files a/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofraw and b/llvm/test/tools/llvm-profdata/Inputs/buildid.memprofraw differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/inline.memprofexe b/llvm/test/tools/llvm-profdata/Inputs/inline.memprofexe
index 2822f2fa20434..5af6c81f07fad 100755
Binary files a/llvm/test/tools/llvm-profdata/Inputs/inline.memprofexe and b/llvm/test/tools/llvm-profdata/Inputs/inline.memprofexe differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/inline.memprofraw b/llvm/test/tools/llvm-profdata/Inputs/inline.memprofraw
index 05deb2e963a27..8958af941c59d 100644
Binary files a/llvm/test/tools/llvm-profdata/Inputs/inline.memprofraw and b/llvm/test/tools/llvm-profdata/Inputs/inline.memprofraw differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/multi.memprofexe b/llvm/test/tools/llvm-profdata/Inputs/multi.memprofexe
index 22c6136f3dda8..e9ec22cc96708 100755
Binary files a/llvm/test/tools/llvm-profdata/Inputs/multi.memprofexe and b/llvm/test/tools/llvm-profdata/Inputs/multi.memprofexe differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/multi.memprofraw b/llvm/test/tools/llvm-profdata/Inputs/multi.memprofraw
index 364aa1cefdd73..3952768d44c68 100644
Binary files a/llvm/test/tools/llvm-profdata/Inputs/multi.memprofraw and b/llvm/test/tools/llvm-profdata/Inputs/multi.memprofraw differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofexe b/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofexe
index 34db7e784208c..e558efc7cfbc9 100755
Binary files a/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofexe and b/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofexe differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofraw b/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofraw
index 7a7d3a6460aed..ed2e7757ac8f2 100644
Binary files a/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofraw and b/llvm/test/tools/llvm-profdata/Inputs/padding-histogram.memprofraw differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/pic.memprofexe b/llvm/test/tools/llvm-profdata/Inputs/pic.memprofexe
index f7d172314de6d..63eea4438dad8 100755
Binary files a/llvm/test/tools/llvm-profdata/Inputs/pic.memprofexe and b/llvm/test/tools/llvm-profdata/Inputs/pic.memprofexe differ
diff --git a/llvm/test/tools/llvm-profdata/Inputs/pic.memprofraw b/llvm/test/tools/llvm-profdata/Inputs/pic.memprofraw
index 0920028b55840..b6a733af50f5d 100644
Binary files a/llvm/test/tools/llvm-profdata/Inputs/pic.memprofraw and b/llvm/test/tools/llvm-profdata/Inputs/pic.memprofraw differ
diff --git a/llvm/test/tools/llvm-profdata/memprof-basic-histogram.test b/llvm/test/tools/llvm-profdata/memprof-basic-histogram.test
index 3d30a627bdd79..ce534db77f4f4 100644
--- a/llvm/test/tools/llvm-profdata/memprof-basic-histogram.test
+++ b/llvm/test/tools/llvm-profdata/memprof-basic-histogram.test
@@ -7,7 +7,7 @@ We expect 5 MIBs, each with different AccessHistogramValues.
CHECK: MemprofProfile:
CHECK-NEXT: Summary:
-CHECK-NEXT: Version: 4
+CHECK-NEXT: Version: 5
CHECK-NEXT: NumSegments: {{[0-9]+}}
CHECK-NEXT: NumMibInfo: 5
CHECK-NEXT: NumAllocFunctions: 3
@@ -241,4 +241,4 @@ CHECK-NEXT: MinLifetimeAccessDensity: 56000
CHECK-NEXT: MaxLifetimeAccessDensity: 56000
CHECK-NEXT: AccessHistogramSize: 8
CHECK-NEXT: AccessHistogram: {{[0-9]+}}
-CHECK-NEXT: AccessHistogramValues: 168 147 126 105 84 63 42 21
\ No newline at end of file
+CHECK-NEXT: AccessHistogramValues: 168 147 126 105 84 63 42 21
diff --git a/llvm/test/tools/llvm-profdata/memprof-basic.test b/llvm/test/tools/llvm-profdata/memprof-basic.test
index e15df50bc1657..81550ebce40d3 100644
--- a/llvm/test/tools/llvm-profdata/memprof-basic.test
+++ b/llvm/test/tools/llvm-profdata/memprof-basic.test
@@ -8,7 +8,7 @@ additional allocations which do not originate from the main binary are pruned.
CHECK: MemprofProfile:
CHECK-NEXT: Summary:
-CHECK-NEXT: Version: 4
+CHECK-NEXT: Version: 5
CHECK-NEXT: NumSegments: {{[0-9]+}}
CHECK-NEXT: NumMibInfo: 2
CHECK-NEXT: NumAllocFunctions: 1
@@ -96,4 +96,4 @@ CHECK-NEXT: TotalLifetimeAccessDensity: 20000
CHECK-NEXT: MinLifetimeAccessDensity: 20000
CHECK-NEXT: MaxLifetimeAccessDensity: 20000
CHECK-NEXT: AccessHistogramSize: 0
-CHECK-NEXT: AccessHistogram: 0
\ No newline at end of file
+CHECK-NEXT: AccessHistogram: 0
diff --git a/llvm/test/tools/llvm-profdata/memprof-inline.test b/llvm/test/tools/llvm-profdata/memprof-inline.test
index 79ce2ad838482..4a3f6201f0a35 100644
--- a/llvm/test/tools/llvm-profdata/memprof-inline.test
+++ b/llvm/test/tools/llvm-profdata/memprof-inline.test
@@ -5,7 +5,7 @@ RUN: llvm-profdata show --memory %p/Inputs/inline.memprofraw --profiled-binary %
CHECK: MemprofProfile:
CHECK-NEXT: Summary:
-CHECK-NEXT: Version: 4
+CHECK-NEXT: Version: 5
CHECK-NEXT: NumSegments: {{[0-9]+}}
CHECK-NEXT: NumMibInfo: 2
CHECK-NEXT: NumAllocFunctions: 2
diff --git a/llvm/test/tools/llvm-profdata/memprof-multi.test b/llvm/test/tools/llvm-profdata/memprof-multi.test
index 62439823defd0..35f94dfe2c096 100644
--- a/llvm/test/tools/llvm-profdata/memprof-multi.test
+++ b/llvm/test/tools/llvm-profdata/memprof-multi.test
@@ -7,7 +7,7 @@ We expect 2 MIB entries, 1 each for the malloc calls in the program.
CHECK: MemprofProfile:
CHECK-NEXT: Summary:
-CHECK-NEXT: Version: 4
+CHECK-NEXT: Version: 5
CHECK-NEXT: NumSegments: {{[0-9]+}}
CHECK-NEXT: NumMibInfo: 2
CHECK-NEXT: NumAllocFunctions: 1
diff --git a/llvm/test/tools/llvm-profdata/memprof-padding-histogram.test b/llvm/test/tools/llvm-profdata/memprof-padding-histogram.test
index 4ba58e3c870d5..79521f3aceb6d 100644
--- a/llvm/test/tools/llvm-profdata/memprof-padding-histogram.test
+++ b/llvm/test/tools/llvm-profdata/memprof-padding-histogram.test
@@ -7,7 +7,7 @@ We expect 2 different MIBs with histogram values. This test is to make sure we p
CHECK: MemprofProfile:
CHECK-NEXT: Summary:
-CHECK-NEXT: Version: 4
+CHECK-NEXT: Version: 5
CHECK-NEXT: NumSegments: {{[0-9]+}}
CHECK-NEXT: NumMibInfo: 2
CHECK-NEXT: NumAllocFunctions: 1
@@ -96,4 +96,4 @@ CHEC-NEXT MinLifetimeAccessDensity: 8000
CHEC-NEXT MaxLifetimeAccessDensity: 8000
CHEC-NEXT AccessHistogramSize: 6
CHEC-NEXT AccessHistogram: {{[0-9]+}}
-CHEC-NEXT AccessHistogramValues: -2 -0 -0 -0 -1 -1
\ No newline at end of file
+CHEC-NEXT AccessHistogramValues: -2 -0 -0 -0 -1 -1
diff --git a/llvm/test/tools/llvm-profdata/memprof-pic.test b/llvm/test/tools/llvm-profdata/memprof-pic.test
index 78d2c5c54feb1..66203ef9248ff 100644
--- a/llvm/test/tools/llvm-profdata/memprof-pic.test
+++ b/llvm/test/tools/llvm-profdata/memprof-pic.test
@@ -11,7 +11,7 @@ RUN: llvm-profdata show --memory %p/Inputs/pic.memprofraw --profiled-binary %p/I
CHECK: MemprofProfile:
CHECK-NEXT: Summary:
-CHECK-NEXT: Version: 4
+CHECK-NEXT: Version: 5
CHECK-NEXT: NumSegments: {{[0-9]+}}
CHECK-NEXT: NumMibInfo: 2
CHECK-NEXT: NumAllocFunctions: 1
@@ -100,4 +100,4 @@ CHECK-NEXT: TotalLifetimeAccessDensity: 20000
CHECK-NEXT: MinLifetimeAccessDensity: 20000
CHECK-NEXT: MaxLifetimeAccessDensity: 20000
CHECK-NEXT: AccessHistogramSize: 0
-CHECK-NEXT: AccessHistogram: 0
\ No newline at end o...
[truncated]
|
4135d07
to
1ec7c01
Compare
Also @mattweingarten can you take a look? |
1ec7c01
to
f9fb66f
Compare
uint8_t *CurrentHistPtr = (uint8_t *)AccessHistogram; | ||
uint8_t *ShorterHistPtr = (uint8_t *)ShorterHistogram; | ||
uint32_t sum = CurrentHistPtr[i] + ShorterHistPtr[i]; | ||
CurrentHistPtr[i] = (sum > 255) ? 255 : (uint8_t)sum; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should/can this use HISTOGRAM_MAX_COUNTER?
Does this mean that we were getting histogram values >255 before in the profile, and if so, do we know how much this matters in practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if a change in behavior do we need a test for that?
CHECK-NEXT: AccessHistogramValues: 168 147 126 105 84 63 42 21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spurious difference here and at the end of some of the other changed tests due to different line ending at EOF?
|
||
MemInfoBlock MIB; | ||
#define READ_MIB_FIELD(FIELD) \ | ||
MIB.FIELD = endian::readNext<decltype(MIB.FIELD), llvm::endianness::little, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a functional change from what we were doing before? Is it required for the new version? If not, should we read things the same way in v4?
Change memory access histogram storage from uint64_t to uint8_t to reduce profile size on disk. This change updates the raw profile format to v5. When reading v5 raw profiles also update the reader to be endian-aware (don't directly interpret the bytes in the native format). Also add a histogram test in compiler-rt since we didn't have one before. With this change the histogram memprof raw for the basic test reduces from 75KB -> 11KB.