[llvm] r252538 - CodeGen: Be clear about semantics in SlotIndex::getNextSlot(), NFC

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 9 15:31:02 PST 2015


Author: dexonsmith
Date: Mon Nov  9 17:31:01 2015
New Revision: 252538

URL: http://llvm.org/viewvc/llvm-project?rev=252538&view=rev
Log:
CodeGen: Be clear about semantics in SlotIndex::getNextSlot(), NFC

Be honest about using iterator semantics in `SlotIndex::getNextSlot()`
and `SlotIndex::getPrevSlot()`.  Instead of calling `getNextNode()` --
which is documented (but fails) to check for the sentinel -- call
`&*++getIterator()`.

This is (surprisingly!) a NFC commit.  `ilist_traits<IndexListEntry>`
has an `ilist_half_node<IndexListEntry>` as a sentinel (and no other
fields), and so the layout of `ilist<IndexListEntry>` is:
--
struct ilist<IndexListEntry> {
  ilist_half_node<IndexListEntry> Sentinel;
  IndexListEntry *Head;

  IndexListEntry *getHead() { return Head; }
  IndexListEntry *getSentinel() { return cast<...>(&Sentinel); }
};
--
In memory, this happens to look just like:
--
struct ilist<IndexListEntry> {
  ilist_node<IndexListEntry> Sentinel;

  IndexListEntry *getHead() { return Sentinel.getNext(); }
  IndexListEntry *getSentinel() { return cast<...>(&Sentinel); }
};
--
As a result, `ilist_node<IndexListEntry>::getNextNode()` that checks
`getNext()` of the possible sentinel will get a pointer to the head of
the list; it will never detect the sentinel, and will return the
sentinel itself instead of `nullptr` in the special cases.

Since `getNextNode()` and `getPrevNode()` don't work, just be honest
that we're not checking for the end/beginning of the list here.  Since
this code works, I guess we must never go past the sentinel.

(It's possible we're just getting lucky, and the new code will get
"lucky" in the same situations.  To properly fix that hypothetical bug,
we would need to check the iterator against `end()`/`begin()`.)

Modified:
    llvm/trunk/include/llvm/CodeGen/SlotIndexes.h

Modified: llvm/trunk/include/llvm/CodeGen/SlotIndexes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SlotIndexes.h?rev=252538&r1=252537&r2=252538&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/SlotIndexes.h (original)
+++ llvm/trunk/include/llvm/CodeGen/SlotIndexes.h Mon Nov  9 17:31:01 2015
@@ -272,7 +272,7 @@ namespace llvm {
     SlotIndex getNextSlot() const {
       Slot s = getSlot();
       if (s == Slot_Dead) {
-        return SlotIndex(listEntry()->getNextNode(), Slot_Block);
+        return SlotIndex(&*++listEntry()->getIterator(), Slot_Block);
       }
       return SlotIndex(listEntry(), s + 1);
     }
@@ -280,7 +280,7 @@ namespace llvm {
     /// Returns the next index. This is the index corresponding to the this
     /// index's slot, but for the next instruction.
     SlotIndex getNextIndex() const {
-      return SlotIndex(listEntry()->getNextNode(), getSlot());
+      return SlotIndex(&*++listEntry()->getIterator(), getSlot());
     }
 
     /// Returns the previous slot in the index list. This could be either the
@@ -292,7 +292,7 @@ namespace llvm {
     SlotIndex getPrevSlot() const {
       Slot s = getSlot();
       if (s == Slot_Block) {
-        return SlotIndex(listEntry()->getPrevNode(), Slot_Dead);
+        return SlotIndex(&*--listEntry()->getIterator(), Slot_Dead);
       }
       return SlotIndex(listEntry(), s - 1);
     }
@@ -300,7 +300,7 @@ namespace llvm {
     /// Returns the previous index. This is the index corresponding to this
     /// index's slot, but for the previous instruction.
     SlotIndex getPrevIndex() const {
-      return SlotIndex(listEntry()->getPrevNode(), getSlot());
+      return SlotIndex(&*--listEntry()->getIterator(), getSlot());
     }
 
   };




More information about the llvm-commits mailing list