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

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 16 16:15:24 PDT 2021


nikic added a comment.

@mtrofin I'm not really concerned about regressions on individual test cases, but the average impact. I found https://github.com/llvm/llvm-project/commit/f649f24d388c745d20fab5573d27b822b92818ed with some data from when the option was enabled to AArch64, and it looks like this option has essentially zero average impact and only improves a few rare outliers. This was totally fine at the time, because enabling the option was considered to be essentially free compile-time wise. But now it turns out that this is actually not the case, and the small compile-time impact is actually the result of an implementation bug. The change would have never landed at the time if this fact had been known. A zero geomean improvement on run-time is a terrible trade-off for a 2-3% geomean regression on compile-time.

Unless the situation on X86 is significantly different than for AArch64, it seems pretty clear to me that we should just disable the option until it can be implemented in a way that is both correct and sufficiently cheap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98232



More information about the llvm-commits mailing list