vsk added a comment. In D98061#2615250 <https://reviews.llvm.org/D98061#2615250>, @phosek wrote:
> In D98061#2615239 <https://reviews.llvm.org/D98061#2615239>, @vsk wrote: > >> @ributzka may have stronger thoughts about when -fprofile-instr-generate >> must imply that a known set of symbols appear with external visibility. Up >> until now, the answer has been "always", and this is what tapi enforces for >> MachO. It's awkward to have this be inconsistent between MachO/ELF, but if >> there's a compelling performance reason then I think it's fine. > > From the perspective of Fuchsia, we've seen a noticeable impact of this > change when using `-fprofile-instr-generate` together `-fprofile-list` to > apply instrumentation selectively only to modified files. What kind of impact do you see? If #counters > 0, is it mostly binary size cost? If #counters == 0, what's the avg. overhead of writing out the full profile? Can it be fixed by doing an early-exit in the runtime initializer, writing out an empty .profraw? That raises a question about tooling support: some workflows (like the Xcode coverage plugin) might assume that a program compiled with -fprofile-instr-generate always creates a .profraw. If there's no profile written at all for the #counters == 0 case, that could be a breaking change. ================ Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:1082-1084 auto *User = Function::Create(FunctionType::get(Int32Ty, false), GlobalValue::LinkOnceODRLinkage, getInstrProfRuntimeHookVarUseFuncName(), M); ---------------- phosek wrote: > vsk wrote: > > phosek wrote: > > > @vsk do you know why we need this function instead of just using > > > `llvm.compiler.used`/`llvm.used` for the symbol? I used that approach for > > > ELF and it seems to be working fine. > > I don't have the context for this, since this code is from before I started > > working on llvm. I'm guessing, but maybe it's possible that > > llvm(.compiler)?.used didn't exist or work well when this code was written. > Would it be OK with you if I sent out a separate change to remove this? Thanks, yes, that would be great. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98061/new/ https://reviews.llvm.org/D98061 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
