[PATCH] D98061: [InstrProfiling] Generate runtime hook for ELF platforms

Tue Mar 9 15:07:27 PST 2021

phosek marked an inline comment as done.
phosek added a comment.

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.

================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:1082-1084
   auto *User = Function::Create(FunctionType::get(Int32Ty, false),
                                 GlobalValue::LinkOnceODRLinkage,
                                 getInstrProfRuntimeHookVarUseFuncName(), M);
----------------
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?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98061