This is an archive of the discontinued LLVM Phabricator instance.

Add a profile summary class specific to instrumented profiles.
ClosedPublic

Authored by eraman on Feb 16 2016, 4:22 PM.

Details

Summary

Modify ProfileSummary class to make it not instrumented profile specific. Add a new InstrumentedProfileSummary class that inherits from ProfileSummary. This would avoid the ugliness of having to bases sample profile specific summary information in instrumented profiling terms.

I have also moved MaxFunctionCount to the derived class even though the notion of max of weights at function entry is relevant to SampleProfile. I want to keep the base class only about a set of weights, their count, sum, max and detailed summary.

Diff Detail

Event Timeline

eraman updated this revision to Diff 48123.Feb 16 2016, 4:22 PM
eraman retitled this revision from to Add a profile summary class specific to instrumented profiles..
eraman updated this object.
eraman added a reviewer: davidxl.
eraman added a subscriber: dnovillo.
davidxl added inline comments.Feb 16 2016, 4:37 PM
include/llvm/ProfileData/InstrProfReader.h
356

s/InstrumentedProfileSummary/InstrProfSummary/

include/llvm/ProfileData/ProfileCommon.h
36

I am not particularly fond of the term 'weight' here. # of samples in SampleFDO is basically sample count, so it seems perfectly fine to keep using 'Count'

37

Why not NumCounts (which is also better for Instr Profile than NumBlocks)?

66

Why is MaxFunctionCount specific to InstrProfile?

eraman marked 3 inline comments as done.Feb 16 2016, 5:02 PM
eraman added inline comments.
include/llvm/ProfileData/ProfileCommon.h
66

It is not. As I said in the summary, the base class computes and maintains useful statistics about a set of value (counts or samples). Since function count is a "special" value - a value that satisfies a particular property - I moved it to derived class.

eraman updated this revision to Diff 48135.Feb 16 2016, 5:04 PM

Address David's comments

davidxl accepted this revision.Feb 16 2016, 9:04 PM
davidxl edited edge metadata.

lgtm.

This revision is now accepted and ready to land.Feb 16 2016, 9:04 PM
This revision was automatically updated to reflect the committed changes.