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

2021-03-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > This approach was already used prior to 9a041a75221ca, but we changed it to > always generate the llvm_profile_runtime due to a TAPI limitation. D43794 (rG9a041a75221ca

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

2021-03-23 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. In D98061#2623959 , @MaskRay wrote: > I am a bit concerned that whether the file is generated or not is now > dependent on the instrumentation and linker garbage collection. That's a fair concern, do you know about use cases where

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

2021-03-23 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 332739. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98061/new/ https://reviews.llvm.org/D98061 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Fuchsia.cpp clang/lib/Driver/ToolChains/Fuchsia

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

2021-03-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > There are also going to be binaries in our system build that don't use foo.h > and ideally shouldn't have any overhead since nothing inside them is > instrumented (they should behave as if -fprofile-instr-generate wasn't set > for them at all). That's not the case tod

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

2021-03-11 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG87fd09b25f88: [InstrProfiling] Generate runtime hook for ELF platforms (authored by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98061/new/ https:

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

2021-03-11 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 329875. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98061/new/ https://reviews.llvm.org/D98061 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Fuchsia.cpp clang/lib/Driver/ToolChains/Fuchsia

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

2021-03-10 Thread Vedant Kumar via Phabricator via cfe-commits
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks for the detailed explanation of the -fprofile-list workflow; given the difference constraints, this patch lgtm. Please document the divergent behavior re: no .profraw file when #counters == 0

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

2021-03-09 Thread David Li via Phabricator via cfe-commits
davidxl added a comment. thanks for the background. This patch looks good at higher level. Vedant can help detailed review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98061/new/ https://reviews.llvm.org/D98061 _

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

2021-03-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. In D98061#2615575 , @davidxl wrote: > Is the case when there is no counters very rare? And for those cases, how > much overhead the runtime hook can incur? I assume it is small compared with > actual instrumentation? I'll try t

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

2021-03-09 Thread David Li via Phabricator via cfe-commits
davidxl added a comment. Is the case when there is no counters very rare? And for those cases, how much overhead the runtime hook can incur? I assume it is small compared with actual instrumentation? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

2021-03-09 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. In D98061#2615386 , @phosek wrote: > In D98061#2615334 , @vsk wrote: > >> In D98061#2615250 , @phosek wrote: >> >>> In D98061#2615239

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

2021-03-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek marked an inline comment as done. phosek added a comment. In D98061#2615334 , @vsk wrote: > In D98061#2615250 , @phosek wrote: > >> In D98061#2615239 , @vsk wrote: >>

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

2021-03-09 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. In D98061#2615250 , @phosek wrote: > In D98061#2615239 , @vsk wrote: > >> @ributzka may have stronger thoughts about when -fprofile-instr-generate >> must imply that a known set of symbols appe

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

2021-03-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek marked an inline comment as done. phosek added a comment. In 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

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

2021-03-09 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a subscriber: ributzka. vsk added a comment. @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 awkw

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

2021-03-09 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. @vsk do you have any thoughts on this? 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

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

2021-03-05 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:1082-1084 auto *User = Function::Create(FunctionType::get(Int32Ty, false), GlobalValue::LinkOnceODRLinkage, getIns

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

2021-03-05 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision. phosek added reviewers: vsk, davidxl. Herald added a subscriber: hiraditya. phosek requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. When using -fprofile-list to selectively apply instrumentatio