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

[Aiden Grossman via Phabricator via llvm-commits]

aidengrossman marked 2 inline comments as done.
aidengrossman added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SlotIndexes.h:219-221
+    /// 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
----------------
MatzeB wrote:
> This assumption is also not true after inserting one instruction. You have to insert `InstrDist / Slot_Count` instructions I believe until you reach the limit (currently 4). And also the 5th insertion would trigger a renumbering making this approximate again...
That's a good point. Thank you.

If I'm understanding everything correctly in [[ https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/CodeGen/SlotIndexes.h#L534 | insertMachineInstrInMaps ]], it looks like it will only insert up to `(InstrDist / Slot_Count) - 1` instructions before the distance, with the next inserted instruction having a calculated distance of zero and a renumbering being requested. It also looks like the assumption can hold if you insert two instructions in between two normally spaced instructions, and you'll get the correct numbering within those three instructions. I've reworded the statement to talk specifically about the example of calculating the distance between two "normally" (no insertions in between, no renumbering) spaced instructions. Let me know if you think a different statement would better encapsulate some of the nuances and also if my indexing is incorrect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133386