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

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 2 15:09:04 PST 2021


MatzeB added a comment.

> 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...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114831



More information about the llvm-commits mailing list