[PATCH] D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D98232#2630290 <https://reviews.llvm.org/D98232#2630290>, @mtrofin wrote:

> In D98232#2630273 <https://reviews.llvm.org/D98232#2630273>, @nikic wrote:
>
>> Unfortunately I'm not familiar with RegAlloc, but I somehow doubt that this kind of compile-time hit is intrinsically necessary to address this issue.
>
> The problem is that the previous use of the APIs was incorrect -  the code before was sometimes using stale data, because it was incorrectly not calling `collectInterferingVRegs`. The compile time regression is reflective of the effort required to refresh the data.
>
> I think we have a situation like this:
>
> - (status quo) no compile time regression, but incorrect functionality
> - (patch) compile time regression, correct functionality
> - remove patch that originally introduced this functionality (https://reviews.llvm.org/D35816), but then there are cases where we hit regressions in code quality
> - <options that I am not aware of - actionable suggestions very welcome!>

The best option would be to implement this without the compile-time impact or with low impact -- it's probably possible, but may require a larger time investment. If you don't have the resources for that, then please evaluate the third option. On the assumption that disabling this code only makes for a minor code quality regression, that would be preferred over a large compile-time regression. Of course, if disabling this makes for a large code quality regression, this becomes harder to answer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98232