[PATCH] D39235: [MachineScheduler] minor refactoring

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Looks like a reasonable improvement to me, although I am not entirely sure if adding 2 methods to `SchedBoundary` is the best thing to do here, as the only thing `checkResourceLimit` uses from `SchedBoundary` is `SchedModel`. Just having `static bool checkResourceLimit(unsigned LFactor, unsigned Count, unsigned Lat)` seems slightly simpler.



================
Comment at: include/llvm/CodeGen/MachineScheduler.h:685
 public:
+  bool checkResourceLimit(unsigned Count, unsigned Lat);
+
----------------
I think a comment here would be good to avoid confusion with `isResourceLimited` below. Also, `Lat` -> `Latency` ?


================
Comment at: include/llvm/CodeGen/MachineScheduler.h:687
+
+protected:
+  void updateIsResourceLimited() {
----------------
popping that between 2 public blocks seems unnecessary. Could `checkResourceLimit` be moved to the public: block at the beginning of SchedBoundary? Also, what's the reasoning behind making `updateIsResourceLimited` the only protected function in SchedBoundary?


https://reviews.llvm.org/D39235