[PATCH] D131220: [NFC][MLGO] ML Regalloc Priority Advisor

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:355
+
+  float Ret = PriorityAdvisor->tryFindPriority(Prio, Size, Stage, LI->weight());
+
----------------
rename to `getPriority`, and it should just take `LI`as parameter. Then, its implementation is all the stuff above.


================
Comment at: llvm/lib/CodeGen/RegAllocPriorityAdvisor.h:34
+  /// Log a reward value needed by ML model.
+  virtual void logReward(float reward) = 0;
+
----------------
remove logReward. this is something that only matters in the training scenario, so it doesn't belong on the abstraction.


================
Comment at: llvm/lib/CodeGen/RegAllocPriorityAdvisor.h:37
+protected:
+  RegAllocPriorityAdvisor(const MachineFunction &MF, const RAGreedy &RA);
+
----------------
do you need MF and RA here? IIUC, you just need the LiveInterval (passed in); maybe have the advisor take just RAGreedy, so it can get to ExtraInfo and LIS from it. At this stage, too, the ctor can be public.


================
Comment at: llvm/lib/CodeGen/RegAllocPriorityAdvisor.h:44
+/// ImmutableAnalysis abstraction for fetching the Priority Advisor.
+class RegAllocPriorityAdvisorAnalysis : public ImmutablePass {
+public:
----------------
remove the analysis stuff, it'll come in with a subsequent patch 


================
Comment at: llvm/lib/CodeGen/RegAllocPriorityAdvisor.h:82
+
+  void logReward(float reward) override{};
+
----------------
remove logReward. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131220