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

[Eric Christopher via llvm-commits]

echristo added inline comments.


================
Comment at: lib/CodeGen/TargetPassConfig.cpp:559-560
+
+  if (getOptLevel() != CodeGenOpt::None &&
+      !TM->targetSchedulesPostRAScheduling()) {
+    if (MISchedPostRA)
----------------
echristo wrote:
> MatzeB wrote:
> > amehsan wrote:
> > > amehsan wrote:
> > > > MatzeB wrote:
> > > > > It looks like we supersede ™::targetSchedulesPostRAScheduling()"? It should be trivially for the targets reporting true here to instead just overload this method with an empty one. (We can do this in a follow up patch though).
> > > > My understanding is that if a target returns true, it means that they probably want to schedule PostRAScheduler pass somewhere else in the pipeline. I think this condition should guard the call to `addPostRASchedPass()`, even though the functionality will be the same. 
> > > I also leave this comment here as it talks about the functionality of targetSchedulesPostRAScheduling. I think we need a PR to clean up all the different ways and method of scheduling post ra sched and remove redundants and those that don't have a use case. There is the virtual function and this targetSchedulesPostRAScheduling() and enablePostRA.... and various methods of choosing between old scheduler and new scheduer (command line, substitutePass)
> > Yes, this was meant as a side remark and as I said we can deal with it another time, not as part of this patch.
> When were you planning on tackling this? I think unraveling this as part of a prepartory patch and splitting out the necessary elements would be general project goodness.
To elaborate more on this, I'm not sure we want to have the technical debt of even more ways to deal with this than we already have. I'm pretty unhappy with the existing aspects :)


https://reviews.llvm.org/D25482