[compiler-rt] 0d8bdc1 - [MemProf] Record accesses for all words touched in mem intrinsic

[Teresa Johnson via llvm-commits]

Author: Teresa Johnson
Date: 2021-09-30T15:07:55-07:00
New Revision: 0d8bdc17862e1c80edf1223282af3be786da4882

URL: https://github.com/llvm/llvm-project/commit/0d8bdc17862e1c80edf1223282af3be786da4882
DIFF: https://github.com/llvm/llvm-project/commit/0d8bdc17862e1c80edf1223282af3be786da4882.diff

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

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.

Differential Revision: https://reviews.llvm.org/D110799

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/memprof/memprof_rtl.cpp b/compiler-rt/lib/memprof/memprof_rtl.cpp
index fee2912d64d49..fb2ef37e51a25 100644
--- a/compiler-rt/lib/memprof/memprof_rtl.cpp
+++ b/compiler-rt/lib/memprof/memprof_rtl.cpp
@@ -264,14 +264,9 @@ void __memprof_record_access(void const volatile *addr) {
   __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

diff  --git a/compiler-rt/test/memprof/TestCases/test_memintrin.cpp b/compiler-rt/test/memprof/TestCases/test_memintrin.cpp
index 06c61db6d26ee..bc3f66fbc5b6f 100644
--- a/compiler-rt/test/memprof/TestCases/test_memintrin.cpp
+++ b/compiler-rt/test/memprof/TestCases/test_memintrin.cpp
@@ -5,16 +5,16 @@
 // This is actually:
 //  Memory allocation stack id = STACKIDP
 //   alloc_count 1, size (ave/min/max) 40.00 / 40 / 40
-//   access_count (ave/min/max): 3.00 / 3 / 3
+//   access_count (ave/min/max): 11.00 / 11 / 11
 // 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
+//   access_count (ave/min/max): 6.00 / 6 / 6
 // 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 @@ int main() {
   // 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;

diff  --git a/compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp b/compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
index c007b43716087..58aecdf61e598 100644
--- a/compiler-rt/test/memprof/TestCases/unaligned_loads_and_stores.cpp
+++ b/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>