[PATCH] D20314: Move ProfileSummary to IR

Easwaran Raman via llvm-commits llvm-commits at lists.llvm.org
Wed May 18 15:04:32 PDT 2016


eraman marked an inline comment as done.
eraman added a comment.

In http://reviews.llvm.org/D20314#432500, @silvas wrote:

> In http://reviews.llvm.org/D20314#432459, @eraman wrote:
>
> > In http://reviews.llvm.org/D20314#432438, @silvas wrote:
> >
> > > 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?
> >
> >
> > 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.


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.


http://reviews.llvm.org/D20314





More information about the llvm-commits mailing list