[PATCH] D57929: [InstrProf] Implement static profdata registration

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 5 inline comments as done.
rnk added inline comments.


================
Comment at: compiler-rt/lib/profile/InstrProfilingValue.c:34
     lprofValueProfNodes[INSTR_PROF_VNODE_POOL_SIZE] COMPILER_RT_SECTION(
-       COMPILER_RT_SEG INSTR_PROF_VNODES_SECT_NAME_STR);
+       COMPILER_RT_SEG INSTR_PROF_VNODES_SECT_NAME);
 #endif
----------------
davidxl wrote:
> What is this change for?
When I compiled my original change on Linux, I encountered warnings about this code:
  #define INSTR_PROF_DATA_COFF .lprfd$M
Clang warns that dollars in identifiers are considered to be an extension. To address that, I quoted the COFF strings, and eliminated the *_STR macros.

The only use of the section names that needs to not be quoted is the use in InstrProfilingPlatformLinux.c, which I modified to use INSTR_PROF_INSTR_PROF_DATA_COMMON to build the __start / __stop section bounds symbols. This code is already platform specific, so it seemed OK to use the underlying *_COMMON macros.

If you like, I can separate that change out. Would you like me to upload it as a separate review?


================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:682
                                                InstrProfIncrementInst *Inc,
-                                               const Triple &TT) {
+                                               Triple &TT) {
   if (!needsComdatForCounter(F, M))
----------------
davidxl wrote:
> Why is const removed?
Oops, not intended.


================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:758
+  // the counters an external linkage like linkonce_odr.
+  if (TT.isOSBinFormatCOFF() && Cmdt &&
+      Cmdt != Fn->getComdat())
----------------
davidxl wrote:
> can this code be moved closer to where COFF linkage handling above (i.e. resetting to internal)?
I'll try something. I think it might be best to re-inline the code that computes the comdat to use, because the linkage used is very related to how that is done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57929