[PATCH] D86684: [NFC][Refactor] Add the SchedHeuristic for Scheduler to allow platform customizing the heuristics

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello. This looks interesting. I've run into places before where we just wanted to add an extra heuristics. I think some of our downstream schedules go a bit beyond this and would still need to override tryCandidate (the merge is going to be a pain!), but I've run into situations where it would definitely be useful.

This does add some non-inlineable virtual dispatch to a rather hot part of the compiler though. Do you have compile time numbers, to make sure this isn't going to cause any problems?



================
Comment at: llvm/include/llvm/CodeGen/MachineScheduler.h:1099
 
+private:
+  // Keep all the heuristics. Notice that, the earlier it is pushed into the
----------------
I would personally just keep everything as protected.


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineScheduler.h:56-59
   void initialize(ScheduleDAGMI *Dag) override;
   SUnit *pickNode(bool &IsTopNode) override;
   void enterMBB(MachineBasicBlock *MBB) override;
   void leaveMBB() override;
----------------
None of these seems to do anything, beyond calling the base class. It's almost a shame to have to create a subclass of PostGenericScheduler just to add an extra heuristic. As opposed to being able to register them like mutations.

I imagine that a lot of the time these would end up doing something though and the scheduler would end up holding some state. That would be the case for our downstream schedulers I think. So I think this is fine as-is, but you might want to think a bit about how it would be possible to register without needing to subclass.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86684/new/

https://reviews.llvm.org/D86684