[Memprof] Changes HISTOGRAM_GRANULARITY from 8U to 8ULL.

[mattweingarten]

This changes a bug in memprofiling with histogram where the shadow mask would be 0xFFFFFFF8 instead of 0xFFFFFFFFFFFFFFF8, essentially discarding the upper 32 bits of the address. This can cause different addresses to be mapped to the same shadow address.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Matthew Weingarten (mattweingarten)

Changes

This changes a bug in memprofiling with histogram where the shadow mask would be 0xFFFFFFF8 instead of 0xFFFFFFFFFFFFFFF8, essentially discarding the upper 32 bits of the address. This can cause different addresses to be mapped to the same shadow address.

---

Full diff: https://github.com/llvm/llvm-project/pull/100949.diff

1 Files Affected:

• (modified) compiler-rt/lib/memprof/memprof_mapping.h (+1-1)

diff --git a/compiler-rt/lib/memprof/memprof_mapping.h b/compiler-rt/lib/memprof/memprof_mapping.h
index fef8acfcfc921..6da385ab3d6e2 100644
--- a/compiler-rt/lib/memprof/memprof_mapping.h
+++ b/compiler-rt/lib/memprof/memprof_mapping.h
@@ -55,7 +55,7 @@ extern uptr kHighMemEnd; // Initialized in __memprof_init.
 // computed by summing up all individual 1 byte counters. This can incur an
 // accuracy penalty.
 
-#define HISTOGRAM_GRANULARITY 8U
+#define HISTOGRAM_GRANULARITY 8ULL
 
 #define HISTOGRAM_MAX_COUNTER 255U
 

[mattweingarten]

@teresajohnson

[teresajohnson]

Did this affect the callback handling in a way that was user visible and affected the profile results? If so, perhaps a test can be added for that case, since the IR case will be effectively tested once PR100834 goes in.

[mattweingarten]

Did this affect the callback handling in a way that was user visible and affected the profile results? If so, perhaps a test can be added for that case, since the IR case will be effectively tested once PR100834 goes in.

No there is no effect that is visible. Only the shadow adress translation is affected, so the counters have a different address inside the shadow space.

[mattweingarten]

Did this affect the callback handling in a way that was user visible and affected the profile results? If so, perhaps a test can be added for that case, since the IR case will be effectively tested once PR100834 goes in.

No there is no effect that is visible. Only the shadow adress translation is affected, so the counters have a different address inside the shadow space.

I don't think there is a way to write a test for this

[teresajohnson]

lgtm

[llvm-ci]

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building compiler-rt at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/2844

Here is the relevant piece of the build log for the reference:

Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49779.cpp (785 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/test_libc.cpp (786 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug50022.cpp (787 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/wtime.c (788 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/bug49021.cpp (789 of 795)
PASS: libomptarget :: amdgcn-amd-amdhsa :: offloading/bug49021.cpp (790 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/std_complex_arithmetic.cpp (791 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/complex_reduction.cpp (792 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49021.cpp (793 of 795)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (794 of 795)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1227.779737