[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)
----------------
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.


https://reviews.llvm.org/D25482