[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. @davidxl, I added comments in https://github.com/llvm/llvm-project/commit/ead8586645f54de16cfc6fa26028d818efc425f2 @MaskRay, I followed the style guide for nested if statements in https://github.com/llvm/llvm-project/commit/ead8586645f54de16cfc6fa26028d818efc425f2

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

davidxl added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:569 + if (!containsProfilingIntrinsics(M)) { +if (!CoverageNamesVar || !NeedsRuntimeHook) { + return MadeChange; gulfem wrote: > MaskRay wrote: > >

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:569 + if (!containsProfilingIntrinsics(M)) { +if (!CoverageNamesVar || !NeedsRuntimeHook) { + return MadeChange; MaskRay wrote: >

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:569 + if (!containsProfilingIntrinsics(M)) { +if (!CoverageNamesVar || !NeedsRuntimeHook) { + return MadeChange;

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc7f91e227a79: [InstrProfiling] No runtime hook for unused funcs (authored by gulfem). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. In D122336#3406843 , @vsk wrote: > So long as it doesn't change behavior expected by tapi on Darwin, I think > it's OK. I doubt any other platforms are similarly affected. I don't think it should impact TAPI because it relies on

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

vsk added a comment. So long as it doesn't change behavior expected by tapi on Darwin, I think it's OK. I doubt any other platforms are similarly affected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122336/new/ https://reviews.llvm.org/D122336

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. In D122336#3406186 , @vsk wrote: > Sorry for ay delayed replies - I've switched teams at Apple and find it > difficult to keep up with llvm reviews. > >> it's my understanding is that we might be generating coverage record for

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

vsk added a subscriber: cishida. vsk added a comment. Sorry for ay delayed replies - I've switched teams at Apple and find it difficult to keep up with llvm reviews. > it's my understanding is that we might be generating coverage record for > unused functions for TAPI. Coverage function

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem added a comment. In D122336#3405835 , @davidxl wrote: > Should this be fixed in Clang (not generating coverage record)? Adding the > logic here is a little confusing. @davidxl, I originally did that (not generating coverage records) because I

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

davidxl added a comment. Should this be fixed in Clang (not generating coverage record)? Adding the logic here is a little confusing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122336/new/ https://reviews.llvm.org/D122336

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem updated this revision to Diff 417945. gulfem added a comment. Addressed phosek's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122336/new/ https://reviews.llvm.org/D122336 Files:

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:569-573 +if (!CoverageNamesVar) { + return MadeChange; +} else if (!NeedsRuntimeHook) {

[PATCH] D122336: [InstrProfiling] No runtime hook for unused funcs

gulfem created this revision. Herald added subscribers: ellis, abrachet, phosek, hiraditya. Herald added a project: All. gulfem requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. CoverageMappingModuleGen generates a coverage