[PATCH] D132515: [memprof] Correct max size and access count computations

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added reviewers: davidxl, snehasish.
Herald added a subscriber: Enna1.
Herald added a project: All.
tejohnson requested review of this revision.
Herald added projects: Sanitizers, LLVM.
Herald added a subscriber: Sanitizers.

The existing code resulted in the max size and access counts being equal
to the min. Compute the max instead (max lifetime was already correct).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132515

Files:
  compiler-rt/include/profile/MemProfData.inc
  compiler-rt/test/memprof/TestCases/test_merge_mib.cpp
  llvm/include/llvm/ProfileData/MemProfData.inc


Index: llvm/include/llvm/ProfileData/MemProfData.inc
===================================================================
--- llvm/include/llvm/ProfileData/MemProfData.inc
+++ llvm/include/llvm/ProfileData/MemProfData.inc
@@ -137,11 +137,11 @@
 
   TotalAccessCount += newMIB.TotalAccessCount;
   MinAccessCount = newMIB.MinAccessCount < MinAccessCount ? newMIB.MinAccessCount : MinAccessCount;
-  MaxAccessCount = newMIB.MaxAccessCount < MaxAccessCount ? newMIB.MaxAccessCount : MaxAccessCount;
+  MaxAccessCount = newMIB.MaxAccessCount > MaxAccessCount ? newMIB.MaxAccessCount : MaxAccessCount;
 
   TotalSize += newMIB.TotalSize;
   MinSize = newMIB.MinSize < MinSize ? newMIB.MinSize : MinSize;
-  MaxSize = newMIB.MaxSize < MaxSize ? newMIB.MaxSize : MaxSize;
+  MaxSize = newMIB.MaxSize > MaxSize ? newMIB.MaxSize : MaxSize;
 
   TotalLifetime += newMIB.TotalLifetime;
   MinLifetime = newMIB.MinLifetime < MinLifetime ? newMIB.MinLifetime : MinLifetime;
Index: compiler-rt/test/memprof/TestCases/test_merge_mib.cpp
===================================================================
--- /dev/null
+++ compiler-rt/test/memprof/TestCases/test_merge_mib.cpp
@@ -0,0 +1,35 @@
+// Check that merging of MIB info (min/max size and access counts specifically)
+// is done correctly.
+
+// RUN: %clangxx_memprof -O0 %s -o %t
+// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s
+
+// This is actually:
+//  Memory allocation stack id = STACKID
+//   alloc_count 2, size (ave/min/max) 60.00 / 40 / 80
+// but we need to look for them in the same CHECK to get the correct STACKID.
+// CHECK:  Memory allocation stack id = [[STACKID:[0-9]+]]{{[[:space:]].*}}alloc_count 2, size (ave/min/max) 60.00 / 40 / 80
+// CHECK-NEXT:  access_count (ave/min/max): 30.00 / 20 / 40
+// Unfortunately there is not a reliable way to check the ave/min/max lifetime.
+// CHECK-NEXT:  lifetime (ave/min/max):
+// CHECK-NEXT:  num migrated: {{[0-1]}}, num lifetime overlaps: 0, num same alloc cpu: 1, num same dealloc_cpu: 1
+// CHECK: Stack for id [[STACKID]]:
+// CHECK-NEXT: #0 {{.*}} in operator new
+// CHECK-NEXT: #1 {{.*}} in main {{.*}}:[[@LINE+7]]
+
+#include <stdio.h>
+#include <stdlib.h>
+
+int main() {
+  for (int j = 1; j < 3; j++) {
+    int *p = new int[10 * j];
+    for (int i = 0; i < 10 * j; i++)
+      p[i] = i;
+    int a = 0;
+    for (int i = 0; i < 10 * j; i++)
+      a += p[i];
+    delete[] p;
+  }
+
+  return 0;
+}
Index: compiler-rt/include/profile/MemProfData.inc
===================================================================
--- compiler-rt/include/profile/MemProfData.inc
+++ compiler-rt/include/profile/MemProfData.inc
@@ -137,11 +137,11 @@
 
   TotalAccessCount += newMIB.TotalAccessCount;
   MinAccessCount = newMIB.MinAccessCount < MinAccessCount ? newMIB.MinAccessCount : MinAccessCount;
-  MaxAccessCount = newMIB.MaxAccessCount < MaxAccessCount ? newMIB.MaxAccessCount : MaxAccessCount;
+  MaxAccessCount = newMIB.MaxAccessCount > MaxAccessCount ? newMIB.MaxAccessCount : MaxAccessCount;
 
   TotalSize += newMIB.TotalSize;
   MinSize = newMIB.MinSize < MinSize ? newMIB.MinSize : MinSize;
-  MaxSize = newMIB.MaxSize < MaxSize ? newMIB.MaxSize : MaxSize;
+  MaxSize = newMIB.MaxSize > MaxSize ? newMIB.MaxSize : MaxSize;
 
   TotalLifetime += newMIB.TotalLifetime;
   MinLifetime = newMIB.MinLifetime < MinLifetime ? newMIB.MinLifetime : MinLifetime;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132515.455003.patch
Type: text/x-patch
Size: 3411 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220823/072b78a9/attachment.bin>