[PATCH] D114831: [NFC][regalloc] Move ExtraRegInfo and related to LiveRangeStageManager

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

I cannot shake the feeling that we should put the interfaces into a `RegAllocGreedy.h`, the interface doesn't feel generic enough for anything else (and FWIW I am not advocating to make it more generic, just set the right expectations that this is meant to be used in combination with RegAllocGreedy). And with expectations managed maybe it's good enough to just hand out a reference to `ExtraRegInfo` to some `friend` implementations of the eviction interface instead of introducing more abstractions and "Manager" classes. How do others working on the regallocs feel?



================
Comment at: llvm/lib/CodeGen/RegAllocEvictionAdvisor.h:89
+
+class LiveRangeStageManager final {
+  // RegInfo - Keep additional information about each live range.
----------------


Add a comment to document the role of this class and try to find a better name (`ExtraRegInfos` to match the `ExtraRegInfo` map this is appears to be wrapping?)


================
Comment at: llvm/lib/CodeGen/RegAllocEvictionAdvisor.h:102-107
+  MachineRegisterInfo *const MRI;
+  unsigned NextCascade = 1;
+
+public:
+  LiveRangeStageManager(MachineRegisterInfo *MRI) : MRI(MRI) {
+    ExtraRegInfo.resize(MRI->getNumVirtRegs());
----------------
Can we keep the MRI reference out of the interface?


================
Comment at: llvm/lib/CodeGen/RegAllocEvictionAdvisor.h:119-137
+    ExtraRegInfo.resize(MRI->getNumVirtRegs());
+    ExtraRegInfo[Reg].Stage = Stage;
+  }
+
+  void setStage(const LiveInterval &VirtReg, LiveRangeStage Stage) {
+    setStage(VirtReg.reg(), Stage);
+  }
----------------
Wondering if we can go without an MRI reference


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:176
   std::unique_ptr<VirtRegAuxInfo> VRAI;
+  std::unique_ptr<LiveRangeStageManager> LRSM;
 
----------------
Is it possible to embed this by value to avoid the extra indirections?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114831