> On 2014-May-28, at 14:15, Reid Kleckner <[email protected]> wrote: > > On Wed, May 28, 2014 at 1:56 PM, Duncan P. N. Exon Smith > <[email protected]> wrote: > > On 2014-May-28, at 13:43, Reid Kleckner <[email protected]> wrote: > > > > On Wed, May 28, 2014 at 1:06 PM, Alex L <[email protected]> wrote: > > Sure, We need to call CodeGenPGO::assignRegionCounters for all functions > > because we want to emit counter variables for all functions in a > > translation unit, even if they aren't used. > > > > Why do you want to emit counter variables for unused functions? > > Coverage. There's a difference between "no data" (this function wasn't > instrumented) and "never called" (counter is zero). This patch moves unused > functions from the former category to the latter. > > Why do you need profiling data on inline functions that were never called? > You will only need profiling data if someone adds a call to the inline > function, in which case the program changed, and the zeroed out counters > won't be very helpful. If some TU needed them, it will emit them.
I agree that this isn't useful for PGO, but the profile data is also useful for code coverage. Given profile data, consider answering the questions: "What functions were instrumented? Which of these had no coverage?" I'm not sure how to answer those questions accurately without these counters. This still leaves coverage gaps in never-used templated code, but that's a different can of worms. > Also, CodeGenerator::HandleInlineMethodDefinition() would be a better place > for this code. We recently added it for dllexport, which has basically the > semantics you want. If this ends up being the direction we want, we probably > want to merge support for -femit-all-decls, pgo, and dllexport in the same > place to control the linkage and MustBeEmitted-ness of inline functions. That sounds like a better design (assuming you agree on the direction). _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
