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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin marked an inline comment as done.
mtrofin added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocEvictionAdvisor.h:89
+
+class LiveRangeStageManager final {
+  // RegInfo - Keep additional information about each live range.
----------------
MatzeB wrote:
> 
> 
> 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?)
Hmm, I thought both the `Stage` and the `Cascade` relate to tracking the transitions of the virtual register through the allocation; but I can see how, if we move this to a RegAllocGreedy.h, and we want to track more state later, `ExtraRegInfo` may work better. How about we use that and rename the vector to just Info (it's private anyway)?


================
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());
----------------
MatzeB wrote:
> Can we keep the MRI reference out of the interface?
The goal was to avoid needing to pass the exact same thing (the MRI->getNumVirtRegs()) at the points where we need to resize. 

So I learn: what's the downside of MRI here?

Would passing the `VirtRegMap` be a reasonable alternative? 


================
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);
+  }
----------------
MatzeB wrote:
> Wondering if we can go without an MRI reference
ack (consolidating this in the comment above)


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:176
   std::unique_ptr<VirtRegAuxInfo> VRAI;
+  std::unique_ptr<LiveRangeStageManager> LRSM;
 
----------------
MatzeB wrote:
> Is it possible to embed this by value to avoid the extra indirections?
Oh, is this the reason for avoiding nondefault ctor? Let me fire up a https://llvm-compile-time-tracker.com/ experiment, if overhead is a concern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114831