This is an archive of the discontinued LLVM Phabricator instance.

[Profile] Add off-by-default -Wprofile-instr-missing warning
ClosedPublic

Authored by vsk on Jan 18 2017, 11:01 AM.

Details

Summary

Clang warns that a profile is out-of-date if it can't find a profile
record for any function in a TU. This warning is now noisy because llvm
can dead-strip functions with profiling instrumentation.

To fix this, this patch changes the existing profile out-of-date warning
(-Wprofile-instr-out-of-date) so that it only complains about mismatched
data. Further, it introduces a new, off-by-default warning about missing
function data (-Wprofile-instr-missing).

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Jan 18 2017, 11:01 AM
xur edited edge metadata.Jan 18 2017, 11:47 AM

This change gonna hide the missing profile for a function permanently if there is no mismatch. Like, if a user adds a new function without changing existing functions, he will never get a warning if using the old profiles.

In LLVM IR level PGO, we also find the missing profile warning sometime too verbose. We turn missing profile off by default, but not depending on mismatch. I think that is a better method.

vsk added a comment.Jan 18 2017, 12:10 PM

Ah ok, so it sounds like a better approach would be to split the missing record message into a separate off-by-default warning. I don't have the time to update this diff this week, but will shoot for the next.

In D28867#649760, @vsk wrote:

Ah ok, so it sounds like a better approach would be to split the missing record message into a separate off-by-default warning.

I'm in favor of this.

vsk updated this revision to Diff 96847.Apr 26 2017, 4:24 PM
vsk retitled this revision from [Profile] Warn about out-of-date profiles only when there are mismatches to [Profile] Add off-by-default -Wprofile-instr-missing warning.
vsk edited the summary of this revision. (Show Details)
vsk added a reviewer: davidxl.

Apologies for the long delay. I've updated this patch so that -Wprofile-instr-out-of-date and -Wprofile-instr-missing are split up, as suggested by Rong and Mehdi.

xur added a comment.Apr 27 2017, 9:51 AM

Looks good to me.

This revision was automatically updated to reflect the committed changes.