[PATCH] D133386: [nfc] Refactor SlotIndex::getInstrDistance to better reflect actual functionality

[Aiden Grossman via Phabricator via llvm-commits]

aidengrossman created this revision.
Herald added subscribers: arphaman, hiraditya, qcolombet, MatzeB.
Herald added a project: All.
aidengrossman requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This patch refactors SlotIndex::getInstrDistance to
SlotIndex::getApproxInstrDistance to better describe the actual
functionality of this function. This patch also adds in some additional
comments better documenting the assumptions that this function makes to
increase clarity.

Based on discussion on the LLVM Discourse:
https://discourse.llvm.org/t/odd-behavior-in-slotindex-getinstrdistance/64934/5


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133386

Files:
  llvm/include/llvm/CodeGen/SlotIndexes.h
  llvm/lib/CodeGen/RegAllocGreedy.cpp


Index: llvm/lib/CodeGen/RegAllocGreedy.cpp
===================================================================
--- llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -327,12 +327,12 @@
       // are singly defined, this produces optimal coloring in the absence of
       // global interference and other constraints.
       if (!ReverseLocalAssignment)
-        Prio = LI.beginIndex().getInstrDistance(Indexes->getLastIndex());
+        Prio = LI.beginIndex().getApproxInstrDistance(Indexes->getLastIndex());
       else {
         // Allocating bottom up may allow many short LRGs to be assigned first
         // to one of the cheap registers. This could be much faster for very
         // large blocks on targets with many physical registers.
-        Prio = Indexes->getZeroIndex().getInstrDistance(LI.endIndex());
+        Prio = Indexes->getZeroIndex().getApproxInstrDistance(LI.endIndex());
       }
     } else {
       // Allocate global and split ranges in long->short order. Long ranges that
Index: llvm/include/llvm/CodeGen/SlotIndexes.h
===================================================================
--- llvm/include/llvm/CodeGen/SlotIndexes.h
+++ llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -215,8 +215,12 @@
     }
 
     /// Return the scaled distance from this index to the given one, where all
-    /// slots on the same instruction have zero distance.
-    int getInstrDistance(SlotIndex other) const {
+    /// slots on the same instruction have zero distance, assuming that the slot
+    /// indices are packed as densely as possible. This assumption is only true
+    /// in practice if an instruction is inserted in between two existing
+    /// instructions. Otherwise, this function will return a value greater than
+    /// the true instruction distance.
+    int getApproxInstrDistance(SlotIndex other) const {
       return (other.listEntry()->getIndex() - listEntry()->getIndex())
         / Slot_Count;
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133386.458319.patch
Type: text/x-patch
Size: 1973 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220907/de56fe55/attachment.bin>