This splits ProfileSummary into two classes: a ProfileSummary class that has methods to convert from/to metadata and a ProfileSummaryBuilder class that computes the profiles summary which is in ProfileData. In subsequent patches, I will add an analysis pass to access the profile summary information.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
include/llvm/IR/ProfileSummary.h | ||
---|---|---|
2 ↗ | (On Diff #57425) | Why is this not suitable to be in Support? |
Sorry if this is a stupid question, but why do we need separate InstrProfSummary and SampleProfileSummary? Do you expect most client passes to do different things depending on which one it is?
include/llvm/IR/ProfileSummary.h | ||
---|---|---|
68 ↗ | (On Diff #57425) | No need to make this virtual. Just switch on Kind. |
I believe it is useful to differentiate the source of profile in some cases. For example, sample profile has peculiarities where the number of samples at function entry is 0, but the function itself has non-zero samples. But I agree that we don't need separate ProfileSummary classes to differentiate this and it was a mistake on my part to have this class hierarchy. I thought I will first do the minimal change to migrate to IR and then fix this, but if you prefer I'll get rid of the SampleProfileSummary and InstrProfSummary classes (and have an enum for the profile source) in this patch itself.
Feel free to do the refactoring either before or after the move to IR (unless the refactoring is needed to avoid layering issues; I don't think that is the case here). I agree that we should keep the move as mechanical as possible.
lib/ProfileData/ProfileSummaryBuilder.cpp | ||
---|---|---|
28 ↗ | (On Diff #57425) | Nit: in a separate patch it would be nice to make this a constant array to avoid the static constructor. |
Yes, the refactoring is not needed to avoid the layering issues. I'll get the other patch reviewed and submit this patch followed by the other one back-to-back.