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

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 13 08:59:27 PST 2021


mtrofin added a comment.

In D114831#3188279 <https://reviews.llvm.org/D114831#3188279>, @wenlei wrote:

> In D114831#3168435 <https://reviews.llvm.org/D114831#3168435>, @mtrofin wrote:
>
>> In D114831#3168425 <https://reviews.llvm.org/D114831#3168425>, @MatzeB wrote:
>>
>>>> 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.
>>>
>>> Yes, but it's just moving the declaration around without changing things otherwise. Which seems like the "smaller" factoring to me. (at the price of higher entanglement as I stated)
>>
>> Ack.
>>
>> I wonder if we can decouple this design problem from this patch. Should there end up being a strong preference for going to a `RegAllocGreedy.h`, that should be easy to do at any later point, wdyt?
>
> I agree with Matthias that the advisor now is really tied to Greedy and #1 makes sense. Handle this in a later patch is also reasonable, but leaving it in current state for too long probably isn't ideal. If you plan to handle it later, perhaps stick in a TODO comment?
>
> Other than that, the refactoring seems straightforward.

Done - I'll address it once everything is settled (i.e. all the components landed, incl. the ML advisor) - it will probably help with any nuances of the refactoring, too.


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