[PATCH] D91066: [SlotIndexes] Consider existing index range in insertMBBIntoMaps

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SlotIndexes.h:621
         startEntry = createEntry(nullptr, 0);
-        endEntry = getInstructionIndex(*InsertionPoint).listEntry();
-        newItr = indexList.insert(endEntry->getIterator(), startEntry);
-      } else if (nextMBB == mbb->getParent()->end()) {
+        endEntry = IsLastMBB ? &indexList.back()
+                             : getMBBStartIdx(&*nextMBB).listEntry();
----------------
I am not sure I understand the patch.
When InsertionPoint is set the block will be inserted before it, thus the endEntry for the block is the entry of the insertion point.

Admittedly, that behavior is weird (because that means the users of this method must make sure to split/join the blocks accordingly), but this is what the comment says!

Side-comment: Is that code reachable? A quick grep tells me that InsertionPoint is never passed around. I.e., we should just delete it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91066