From 0d8bdc17862e1c80edf1223282af3be786da4882 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Wed, 29 Sep 2021 19:14:09 -0700 Subject: [PATCH] [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 --- compiler-rt/lib/memprof/memprof_rtl.cpp | 11 +++-------- compiler-rt/test/memprof/TestCases/test_memintrin.cpp | 14 +++++++------- .../test/memprof/TestCases/unaligned_loads_and_stores.cpp | 2 +- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/compiler-rt/lib/memprof/memprof_rtl.cpp b/compiler-rt/lib/memprof/memprof_rtl.cpp index fee2912..fb2ef37 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 06c61db..bc3f66f 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 #include @@ -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 c007b43..58aecdf 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 -- 2.7.4