[PATCH] Always emit function declaration when generating profile instrumentation

[Eric Christopher]

... I'll bite.

Why do you want to know "this function wasn't instrumented" versus
"this had no calls" for coverage? If it's not instrumented it's
definitely not called. Otherwise you need to do this for all functions
(and who knows what chaos with special member functions that you
didn't have to create... :)

-eric

On Wed, May 28, 2014 at 3:21 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>
>> On 2014-May-28, at 14:15, Reid Kleckner <rnk at google.com> wrote:
>>
>> On Wed, May 28, 2014 at 1:56 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> > On 2014-May-28, at 13:43, Reid Kleckner <rnk at google.com> wrote:
>> >
>> > On Wed, May 28, 2014 at 1:06 PM, Alex L <arphaman at gmail.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits