[PATCH] D25482: [PPC] Allow two post RA schedulers to be in the pipeline and select one depending on the Machine Function's subtarget

[Matthias Braun via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D25482#589216, @amehsan wrote:

> In https://reviews.llvm.org/D25482#585513, @amehsan wrote:
>
> > In https://reviews.llvm.org/D25482#585510, @MatzeB wrote:
> >
> > > In https://reviews.llvm.org/D25482#585491, @amehsan wrote:
> > >
> > > > In https://reviews.llvm.org/D25482#585490, @MatzeB wrote:
> > > >
> > > > > In https://reviews.llvm.org/D25482#585479, @amehsan wrote:
> > > > >
> > > > > > > 
> > > > > > > 
> > > > > > >> 2- targetSchedulesPostRAScheduling
> > > > > > > 
> > > > > > > This seems redundant to me as soon as 1) is implemented.
> > > > > >
> > > > > > Maybe I am missing something, but this seems different from (1). Note that addPostRASched is called from addMachinePasses which is not typically overriden by targets. SystemZ has a usecase for targetSchedulesPostRAScheduling which cannot be covered by (1).
> > > > >
> > > > >
> > > > > Overriding addPostRASched() with an empty function has the same effect as returning true in targetSchedulesPostRAScheduling(), hasn't it?
> > > >
> > > >
> > > > Correct :)
> > >
> > >
> > > So we can remove targetSchedulesPostRAScheduling() and replace it with the empty function overload after adding addPostRASched().
> >
> >
> > That's a reasonable change for this patch. I'll wait to see Eric's comments well, before updating the patch though. Just to save time.
>
>
> Actually let's make this change in a follow up patch. That change will require other reviewers (from SystemZ). I will make it high priority after I commit this one, but let's finish this patch so I have https://reviews.llvm.org/P9 scheduling work checked in.


I believe this case is obvious enough that we could go without a review from a SystemZ person (of course if you have some SystemZ people in mind add them as subscribers/reviewers).


https://reviews.llvm.org/D25482