[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1119
+  for (const char *A : {"-sectalign", Args.MakeArgString(Segment),
+                        Args.MakeArgString(Section), "0x4000"})
+    CmdArgs.push_back(A);
----------------
davidxl wrote:
> The host tool may not know the page size of the target. Also why making the alignment 16K instead of 4K?
The maximum supported page size for iPhone 6+ is 16K, and 4K for Macs. I thought that the simplicity of using a common constant outweighed the memory savings from reduced alignment, but am happy to make this tighter if you'd prefer.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1151
+  // must also be page-aligned so that its data is not clobbered by mmap().
+  if (!ForGCOV) {
+    for (auto IPSK : {llvm::IPSK_cnts, llvm::IPSK_data}) {
----------------
davidxl wrote:
> The cost of alignment is always paid even when %c is not specified?
Yes, this is done because %c is expected to become the default in Xcode, and to remove the need for recompilation when switching %c on/off.


================
Comment at: compiler-rt/test/profile/ContinuousSyncMode/darwin-proof-of-concept.c:107
+
+  for (int i = num_cnts; i < num_cnts + 3; ++i) {
+    if (buf[i] != (i - num_cnts + 1)) {
----------------
davidxl wrote:
> What does this loop verify?
I've added some clarifying comments. This is to verify that the data written after the counters is equal to the "data[]" array, i.e. {1, 2, 3}.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68351/new/

https://reviews.llvm.org/D68351