[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

[Petr Hosek via Phabricator via llvm-commits]

phosek added a comment.

In D76802#2061718 <https://reviews.llvm.org/D76802#2061718>, @davidxl wrote:

> What is the instrumented size improvement for Clang? How about profile size reduction?


I tried building Clang using `-fprofile-instr-generate` with this change and without it and the size went from 1.7G to 820M, so roughly 50% reduction. I also tried compiling a trivial C file and the generated profile was 155M with this change and 978M without it.



================
Comment at: compiler-rt/lib/profile/InstrProfilingPlatformLinux.c:37
+extern ValueProfNode PROF_VNODES_START COMPILER_RT_VISIBILITY COMPILER_RT_WEAK;
+extern ValueProfNode PROF_VNODES_STOP COMPILER_RT_VISIBILITY COMPILER_RT_WEAK;
 
----------------
davidxl wrote:
> why adding the weak attribute ?
See on the left.


================
Comment at: compiler-rt/lib/profile/InstrProfilingPlatformLinux.c:40
-/* Add dummy data to ensure the section is always created. */
-__llvm_profile_data
-    __prof_data_sect_data[0] COMPILER_RT_SECTION(INSTR_PROF_DATA_SECT_NAME);
----------------
davidxl wrote:
> what is this change for?
`!associated` metadata is lowered to `SHF_LINK_ORDER` and linker requires that all sections that are merged together have the same flags. Since these symbols are placed into the same section as the compiler generated symbols, and don't set the `SHF_LINK_ORDER` flag, we get a link-time error:
```
/usr/bin/ld: __llvm_prf_cnts has both ordered [`__llvm_prf_cnts' in /tmp/instrprof-value-prof-real-2c3254.o] and unordered [`__llvm_prf_cnts' in /src/clang-llvm/llvm-build/debug/lib/clang/11.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfilingPlatformLinux.c.o)] sections
```
One possible way around this is to define these symbols using inline assembly (since there's no way to set `SHF_LINK_ORDER` in pure C/C++):
```
asm(
  ".pushsection " INSTR_PROF_DATA_SECT_NAME ",\"ao\", at progbits,.text\n\t"
  ".popsection\n"
)
```
Another option is to avoid these symbols altogether, and instead make the `__start`/`__stop` symbols weak which means that they'll end up as `NULL` if those sections are empty, and any loop that iterates over those section contents using those `__start`/`__stop` will be no-op. This seemed like a cleaner solution to me, but I'm happy to change if you prefer. One alternative I considered would be to introduce a new attribute which would allow setting the `SHF_LINK_ORDER` flag from C/C++, but that's going to take a bit more work.


================
Comment at: compiler-rt/test/profile/instrprof-gc-sections.c:72
+// PRF_CNTS: Hex dump of section '__llvm_prf_cnts':
+// PRF_CNTS-NEXT: {{.*}} 00000000 00000000 {{.*$}}
----------------
davidxl wrote:
> Explain the expected output here?
I've already tried to do that in `Note:` comments above.


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

https://reviews.llvm.org/D76802