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

Tue Mar 16 15:28:04 PDT 2021

mtrofin added a comment.

In D98232#2630344 <https://reviews.llvm.org/D98232#2630344>, @nikic wrote:

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

Looking at the original patch (https://reviews.llvm.org/D35816) it mentions https://bugs.llvm.org/show_bug.cgi?id=26810 which quotes a 25% regression.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98232