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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin 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:
> > 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.
> I like the .grow() idea, my concern was wether we'd see any compile time impact. Let me try it and see.
Yup, [[ https://llvm-compile-time-tracker.com/compare.php?from=e0b259f22c003ffe909693c6ab0d508d1814434d&to=447c8d6c740e186aa2098579eb532f74760e371b&stat=instructions | no measurable effect ]] when removing `resize` and just `grow`-ing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114831