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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
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());
----------------
mtrofin wrote:
> 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? 
> So I learn: what's the downside of MRI here?

No real downside.

I would just say that reducing dependencies/entanglement with other classes (switching to `VirtRegMap` is as bad in my view). I just think it's a worthwhile general goal when creating new abstractions. And this seemed to be fixable without any real efforts/or lost convenience.

> The goal was to avoid needing to pass the exact same thing (the MRI->getNumVirtRegs()) at the points where we need to resize.

It just seemed to me like we can simply get rid of the need for `getNumVirtRegs()` in the `setStage`/`setCascade` functions by switching to the `grow()` interface. I hope you can see the code suggestions I made in phabricator that show that (I think phab sometimes does not reproduce them properly in the E-Mails, so just making sure we're not talking past each other because you don't see the suggestions)


Anyway these here are nitpicks/suggestions, no harm done if we cannot implement them after all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114831