[PATCH] D43329: [SystemZ, MachineScheduler] Refactor GenericScheduler::tryCandidate() to reuse parts in a new SystemZ scheduling strategy.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: include/llvm/CodeGen/MachineScheduler.h:981
+                                   SchedBoundary *Zone);
+  virtual void tryCandidate(SchedCandidate &Cand, SchedCandidate &TryCand,
+                            SchedBoundary *Zone);
----------------
jonpa wrote:
> fhahn wrote:
> > jonpa wrote:
> > > fhahn wrote:
> > > > I think for now it seems like making tryCandidate a virtual function gives peopl eenough freedom to ship their own heuristics while re-using most of the existing bits. I don't see the benefit of making the helper functions class members however.
> > > > 
> > > > Also, tryCandidate should not modify the scheduler state, so I think it should be `const`.
> > > > 
> > > > And as this is part of the interface now and we expect people to extend it, it would be good to document it.
> > > How are those bits going to be reused if not by inheritance?
> > > 
> > > How do you picture SystemZ adding this simple heuristic (see top of page) while reusing all else?
> > > 
> > > How do you picture SystemZ adding this simple heuristic (see top of page) while reusing all else?
> > 
> > They do not have to be member functions, they could be regular exported functions. They don't access the scheduler object (I think), so I don't think they would need to be member functions. To me, it seems simpler not to couple the scheduler and the heuristics too much, but that's again mostly personal perference.
> I tried removing the 'static' from such a function in MachineScheduler.cpp, and then adding an 'extern' declaration in SystemZMachineScheduler.cpp, but that lead to a lot of linker errors. How would you do this?
> 
ah sorry. I meant removing static and moving the declaration to the header file.


https://reviews.llvm.org/D43329