[PATCH] D110799: [MemProf] Record accesses for all words touched in mem intrinsic

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added a reviewer: snehasish.
tejohnson requested review of this revision.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

Previously for mem* intrinsics we only incremented the access count for
the first word in the range. However, after thinking it through I think
it makes more sense to record an access for every word in the range.
This better matches the behavior of inlined memory intrinsics, and also
allows better analysis of utilization at a future date.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110799

Files:
  compiler-rt/lib/memprof/memprof_rtl.cpp
  compiler-rt/test/memprof/TestCases/test_memintrin.cpp
  compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp


Index: compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
===================================================================
--- compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
+++ compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
@@ -5,7 +5,7 @@
 //    alloc_count 1, size (ave/min/max) 128.00 / 128 / 128
 // 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 1, size (ave/min/max) 128.00 / 128 / 128
-// CHECK-NEXT:   access_count (ave/min/max): 7.00 / 7 / 7
+// CHECK-NEXT:   access_count (ave/min/max): 22.00 / 22 / 22
 
 #include <sanitizer/memprof_interface.h>
 
Index: compiler-rt/test/memprof/TestCases/test_memintrin.cpp
===================================================================
--- compiler-rt/test/memprof/TestCases/test_memintrin.cpp
+++ compiler-rt/test/memprof/TestCases/test_memintrin.cpp
@@ -7,14 +7,14 @@
 //   alloc_count 1, size (ave/min/max) 40.00 / 40 / 40
 //   access_count (ave/min/max): 3.00 / 3 / 3
 // but we need to look for them in the same CHECK to get the correct STACKIDP.
-// CHECK-DAG:  Memory allocation stack id = [[STACKIDP:[0-9]+]]{{[[:space:]].*}} alloc_count 1, size (ave/min/max) 40.00 / 40 / 40{{[[:space:]].*}} access_count (ave/min/max): 3.00 / 3 / 3
+// CHECK-DAG:  Memory allocation stack id = [[STACKIDP:[0-9]+]]{{[[:space:]].*}} alloc_count 1, size (ave/min/max) 40.00 / 40 / 40{{[[:space:]].*}} access_count (ave/min/max): 11.00 / 11 / 11
 //
 // This is actually:
 //  Memory allocation stack id = STACKIDQ
 //   alloc_count 1, size (ave/min/max) 20.00 / 20 / 20
 //   access_count (ave/min/max): 2.00 / 2 / 2
 // but we need to look for them in the same CHECK to get the correct STACKIDQ.
-// CHECK-DAG:  Memory allocation stack id = [[STACKIDQ:[0-9]+]]{{[[:space:]].*}} alloc_count 1, size (ave/min/max) 20.00 / 20 / 20{{[[:space:]].*}} access_count (ave/min/max): 2.00 / 2 / 2
+// CHECK-DAG:  Memory allocation stack id = [[STACKIDQ:[0-9]+]]{{[[:space:]].*}} alloc_count 1, size (ave/min/max) 20.00 / 20 / 20{{[[:space:]].*}} access_count (ave/min/max): 6.00 / 6 / 6
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -37,9 +37,9 @@
   // CHECK-DAG: Stack for id [[STACKIDQ]]:{{[[:space:]].*}} #0 {{.*}} in operator new{{.*[[:space:]].*}} #1 {{.*}} in main {{.*}}:[[@LINE+1]]
   int *q = new int[5];
 
-  memset(p, 1, 10);
-  memcpy(q, p, 5);
-  int x = memcmp(p, q, 5);
+  memset(p, 1, 10 * sizeof(int));
+  memcpy(q, p, 5 * sizeof(int));
+  int x = memcmp(p, q, 5 * sizeof(int));
 
   delete[] p;
   delete[] q;
Index: compiler-rt/lib/memprof/memprof_rtl.cpp
===================================================================
--- compiler-rt/lib/memprof/memprof_rtl.cpp
+++ compiler-rt/lib/memprof/memprof_rtl.cpp
@@ -264,14 +264,9 @@
   __memprof::RecordAccess((uptr)addr);
 }
 
-// We only record the access on the first location in the range,
-// since we will later accumulate the access counts across the
-// full allocation, and we don't want to inflate the hotness from
-// a memory intrinsic on a large range of memory.
-// TODO: Should we do something else so we can better track utilization?
-void __memprof_record_access_range(void const volatile *addr,
-                                   UNUSED uptr size) {
-  __memprof::RecordAccess((uptr)addr);
+void __memprof_record_access_range(void const volatile *addr, uptr size) {
+  for (uptr a = (uptr)addr; a < (uptr)addr + size; a += kWordSize)
+    __memprof::RecordAccess(a);
 }
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE u16


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110799.376094.patch
Type: text/x-patch
Size: 3617 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210930/94aa7f87/attachment.bin>