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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

In D114831#3168361 <https://reviews.llvm.org/D114831#3168361>, @MatzeB wrote:

>> We'll want to use ExtraRegInfo & associated APIs in the ML-based implementation, hence the factoring.
>
> I understand that. I am hinting at possible designs on how much or how little we abstract:
>
> 1. Treat "eviction" as a strategy that is part of `RegAllocGreedy` and specifically embrace the fact that things only work in combination with RegAllocGreedy:
>
>   // RegAllocGreedy.h
>   class RegAllocGreedy;
>   
>   // This is interface only works for RAGreedy, hence it is in RegAllocGreedy.h. Benefit: we can directly
>   // re-use datastructures and call methods on RAGreedy.
>   class EvictionStrategy {
>     RegAllocGreedy *RAGreedy;
>     EvictionStrategy(RegAllocGreedy* RAGreedy) : RAGreedy(RAGreedy) {}
>   
>     void doStuff() {
>       // ...
>       RAGreedy->setStage(...);
>       // ...
>     }
>   };
>   
>   class RegAllocGreedy {
>   
>     void setStage(...);
>     friend class EvictionStrategy;
>     // ...
>     EvictionStrategy *Eviction;
>   };
>
> This will require fewer abstraction to be built up, with the downside of having a higher entanglement between the two.
>
> 2. The other end of the spectrum is pushing for more abstraction modeling eviction independently of the rest of the register allocator. This usually comes with the goal of having a cleaner separation of concerns and better re-usability of the algorithm (for other regallocs or compilers?).
>
> Now my point is that the current patches appear to aiming more at option 2) while at the same time the interfaces are clearly heavily inspired/only usable for `RegAllocGreedy`. So I was posing the question if we should rather acknowledge that reality and aim more for a 1) design instead...
>
> While I personally think 1) is the better choice in this context, I wouldn't be surprised if different people have different opinions here, so I was fishing for opinions...

Option 1 would mean extracting RAGreedy in that header, and that looks like a larger factoring in a .h, in comparison to the ExtraRegInfo stuff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114831