Thank you very much for the response.
I’m not trying to say that functionally it is unreasonable. As a distance heuristic it definitely makes sense (although the magnitude of the difference between that and just using SlotIndex::distance doesn’t seem like it would be too great for this purpose).
It doesn’t matter exactly what units we’re using, but the current implementation of the greedy register allocator seem to be comparing different units depending upon some different circumstances handled in the greedy register allocator (mainly switching between LiveInterval::getSize(), and SlotIndex::InstrDistance). Changing to getInstrDistance all those years ago didn’t seem to induce a functional change at that point in time (no tests were changed), but swapping the units now does induce a functional change. Not totally sure why this is. Doesn’t seem to be due to any obvious change in the implementation of the containing function.
The function is correct assuming that the instructions are packed as densely as possible, but I don’t believe this will ever be the case in practice. They are by default spaced SlotIndex::InstrDistance and the SlotIndexes::renumberIndexes function still spaces them half that apart.
Definitely won’t give reliable results in a lot of cases, but the accurate alternative would be a pretty expensive function for something that is called many times per invocation.
The original commit message still doesn’t make a whole lot of sense, but elements of the priority calculation are definitely starting to make a bit more sense now.