[PATCH] D57929: [InstrProf] Implement static profdata registration
Reid Kleckner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 7 16:15:52 PST 2019
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
More information about the llvm-commits
mailing list