This is an archive of the discontinued LLVM Phabricator instance.

[PGO] verify BFI counts after loading profile data
ClosedPublic

Authored by xur on Nov 19 2020, 11:57 AM.

Details

Summary

Compare BFI counts with the raw profile counts and print messages.

Split from https://reviews.llvm.org/D61540

Diff Detail

Event Timeline

xur created this revision.Nov 19 2020, 11:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2020, 11:57 AM
davidxl added inline comments.Nov 19 2020, 12:24 PM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1698

Probably use opt-remark-analysis -- it can provide line number which is more informative. The function header print can remain as debug print.

1705

opt-remark seems better.

1707

nit: Num_of_mismatch_BB

xur added inline comments.Nov 19 2020, 9:57 PM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1698

Agreed. Remarks fit well here. I will change to use remarks.

1705

Ditto.

1707

Will change.

xur updated this revision to Diff 306744.Nov 20 2020, 11:12 AM

Using remarks to print the message.
Note that the message printing is still under the internal option.
(This seems OK to me as other options, like EmitBranchProbability, are doing the same.)

davidxl added inline comments.Nov 20 2020, 11:32 AM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1704

Remark or RemarkAnalysis?

davidxl added inline comments.Nov 20 2020, 11:36 AM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
262

Add a description here that -Rpass-analysis=PGO is needed for messages.

xur added inline comments.Nov 20 2020, 12:20 PM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
262

Sure.

1704

I thought you suggested Remark. I'm OK either way.
I will change to RemarkAnalysis.

xur updated this revision to Diff 306763.Nov 20 2020, 12:36 PM

Integrated David's review comments.

davidxl accepted this revision.Nov 20 2020, 2:05 PM

lgtm

This revision is now accepted and ready to land.Nov 20 2020, 2:05 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptDec 14 2020, 4:16 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
dmajor added a subscriber: dmajor.Dec 14 2020, 4:50 PM
dmajor added inline comments.
clang/lib/CodeGen/CGCall.cpp
1948 ↗(On Diff #311740)

This looks like something for temporary local debugging, was it intentional to commit it?

Thanks for catching that.
I messed up the commit from another patch.
Fixed in commit c36f31c.

-Rong