[PATCH] D19293: [profile] LLVM support for memory-mapping profile counters

[Vedant Kumar via llvm-commits]

vsk added inline comments.

================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:390
@@ -385,1 +389,3 @@
 
+void InstrProfiling::emitCounterPadding() {
+  auto PageSize = llvm::sys::Process::getPageSize();
----------------
davidxl wrote:
> vsk wrote:
> > Hm, I think I misunderstood your question.
> > 
> > To clarify, I'm not sure whether or not other linkers have the same behavior, so I thought it would be safer to always emit the padding.
> Can you measure the size overhead (binary, .o files, and raw profile) with clang self build?
> 
> A couple of more questions:
> 
> 1) getPageSize is on host -- not the target -- this is not intended, I think
> 2) do you assume padding bytes are tail paddings ?
> 3) With comdats, this may not work -- for instance, some of the comdat function's counters may be reclaimed by the linker thus leading to counter section size not rounded up to the page size.
> 
> This is why I think you need a driver level change.
We can get the size overhead under 1 page per instrumented Module, and under 3 pages per raw profile. I'll work on getting actual measurements.

Based on your feedback on the compiler-rt patch, I think the right thing to do is to drop emitCounterPadding() and simply pass a section alignment option to the linker.

In more detail:

1. I used the host's page size to guess the target's page size. I'm not aware of a way to determine the target's page size directly, even at the driver level.
2. Only some of these padding bytes will be tail paddings (the ones in the last page of the __llvm_prf_counts__ section). This is enough to ensure that we do not mmap() in the section _after_ __llvm_prf_counts__.
3. Thanks for the catch. Instead of addressing this issue by emitting an entire host page's worth of padding at the module-level, I'll try to modify the driver. That should let me drop emitCounterPadding.


http://reviews.llvm.org/D19293