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

Tue Mar 9 15:28:12 PST 2021

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