Skip to content

[SlotIndexes] Use simple_ilist instead of ilist. NFC. #96747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Jun 26, 2024

simple_ilist does not take ownership of its nodes, which is fine for
SlotIndexes because the IndexListEntry nodes are allocated with a
BumpPtrAllocator and do not need to be freed.

simple_ilist does not take ownership of its nodes, which is fine for
SlotIndexes because the IndexListEntry nodes are allocated with a
BumpPtrAllocator and do not need to be freed.
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2024

@llvm/pr-subscribers-llvm-regalloc

Author: Jay Foad (jayfoad)

Changes

simple_ilist does not take ownership of its nodes, which is fine for
SlotIndexes because the IndexListEntry nodes are allocated with a
BumpPtrAllocator and do not need to be freed.


Full diff: https://github.com/llvm/llvm-project/pull/96747.diff

3 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/SlotIndexes.h (+4-8)
  • (modified) llvm/lib/CodeGen/SlotIndexes.cpp (+4-4)
  • (modified) llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp (+8-8)
diff --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index 72f4a6876b6cb..21a9df22c2e1c 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -22,7 +22,7 @@
 #include "llvm/ADT/IntervalMap.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/ilist.h"
+#include "llvm/ADT/simple_ilist.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
@@ -60,10 +60,6 @@ class raw_ostream;
     }
   };
 
-  template <>
-  struct ilist_alloc_traits<IndexListEntry>
-      : public ilist_noalloc_traits<IndexListEntry> {};
-
   /// SlotIndex - An opaque wrapper around machine indexes.
   class SlotIndex {
     friend class SlotIndexes;
@@ -302,7 +298,7 @@ class raw_ostream;
     // IndexListEntry allocator.
     BumpPtrAllocator ileAllocator;
 
-    using IndexList = ilist<IndexListEntry>;
+    using IndexList = simple_ilist<IndexListEntry>;
     IndexList indexList;
 
     MachineFunction *mf = nullptr;
@@ -549,7 +545,7 @@ class raw_ostream;
 
       // Insert a new list entry for MI.
       IndexList::iterator newItr =
-          indexList.insert(nextItr, createEntry(&MI, newNumber));
+          indexList.insert(nextItr, *createEntry(&MI, newNumber));
 
       // Renumber locally if we need to.
       if (dist == 0)
@@ -608,7 +604,7 @@ class raw_ostream;
           mbb->empty() ? endEntry
                        : getInstructionIndex(mbb->front()).listEntry();
       IndexList::iterator newItr =
-          indexList.insert(insEntry->getIterator(), startEntry);
+          indexList.insert(insEntry->getIterator(), *startEntry);
 
       SlotIndex startIdx(startEntry, SlotIndex::Slot_Block);
       SlotIndex endIdx(endEntry, SlotIndex::Slot_Block);
diff --git a/llvm/lib/CodeGen/SlotIndexes.cpp b/llvm/lib/CodeGen/SlotIndexes.cpp
index 8b80c6ccb4389..a038b09c765c0 100644
--- a/llvm/lib/CodeGen/SlotIndexes.cpp
+++ b/llvm/lib/CodeGen/SlotIndexes.cpp
@@ -26,7 +26,7 @@ SlotIndexes::SlotIndexes() : MachineFunctionPass(ID) {
 
 SlotIndexes::~SlotIndexes() {
   // The indexList's nodes are all allocated in the BumpPtrAllocator.
-  indexList.clearAndLeakNodesUnsafely();
+  indexList.clear();
 }
 
 INITIALIZE_PASS(SlotIndexes, DEBUG_TYPE,
@@ -75,7 +75,7 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) {
   MBBRanges.resize(mf->getNumBlockIDs());
   idx2MBBMap.reserve(mf->size());
 
-  indexList.push_back(createEntry(nullptr, index));
+  indexList.push_back(*createEntry(nullptr, index));
 
   // Iterate over the function.
   for (MachineBasicBlock &MBB : *mf) {
@@ -87,7 +87,7 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) {
         continue;
 
       // Insert a store index for the instr.
-      indexList.push_back(createEntry(&MI, index += SlotIndex::InstrDist));
+      indexList.push_back(*createEntry(&MI, index += SlotIndex::InstrDist));
 
       // Save this base index in the maps.
       mi2iMap.insert(std::make_pair(
@@ -95,7 +95,7 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) {
     }
 
     // We insert one blank instructions between basic blocks.
-    indexList.push_back(createEntry(nullptr, index += SlotIndex::InstrDist));
+    indexList.push_back(*createEntry(nullptr, index += SlotIndex::InstrDist));
 
     MBBRanges[MBB.getNumber()].first = blockStartIndex;
     MBBRanges[MBB.getNumber()].second = SlotIndex(&indexList.back(),
diff --git a/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp b/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp
index bc2d8729474d7..6c863046e159e 100644
--- a/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp
+++ b/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp
@@ -46,7 +46,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
 protected:
   SmallVector<LRStartEndInfo>
   setupOverlapProblem(const SmallVectorImpl<LRPosInfoIndexes> &Segments,
-                      ilist<IndexListEntry> &IndexList) {
+                      simple_ilist<IndexListEntry> &IndexList) {
     SmallVector<LRStartEndInfo> PositionsToReturn;
     PositionsToReturn.reserve(Segments.size());
     for (auto CurrentPosIndexInfo : Segments) {
@@ -61,7 +61,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
           Allocator.Allocate(sizeof(IndexListEntry), alignof(IndexListEntry)));
       auto *CurrentListEntry =
           new (CurrentLEMem) IndexListEntry(nullptr, CurrentIndex);
-      IndexList.push_back(CurrentListEntry);
+      IndexList.push_back(*CurrentListEntry);
       for (size_t CurrentPosInfoIndex = 0;
            CurrentPosInfoIndex < Segments.size(); ++CurrentPosInfoIndex) {
         if ((CurrentIndex / SlotIndex::InstrDist) ==
@@ -107,7 +107,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
   }
 
   void runOverlapTest(SmallVectorImpl<LRPosInfoIndexes> &OverlapSetup) {
-    ilist<IndexListEntry> IndexList;
+    simple_ilist<IndexListEntry> IndexList;
     auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
     NoInferenceModelRunner ModelRunner = setupModelRunner();
     size_t MaxIndex = 0;
@@ -131,7 +131,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
             NumberOfInterferences * ModelMaxSupportedInstructionCount);
     ASSERT_THAT(MappingMatrix,
                 ContainerEq(getExpectedMappingMatrix(OverlapSetup)));
-    IndexList.clearAndLeakNodesUnsafely();
+    IndexList.clear();
   }
 
   BumpPtrAllocator Allocator;
@@ -144,7 +144,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest,
   SmallVector<LRPosInfoIndexes, 2> OverlapSetup;
   OverlapSetup.push_back({0, 5, 0});
   OverlapSetup.push_back({5, 10, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   ASSERT_EQ(OverlapProblem[0].End.distance(OverlapProblem[1].End),
             5 * SlotIndex::InstrDist);
@@ -154,7 +154,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest,
 TEST_F(RegAllocDevelopmentFeaturesTest, MetaSlotIndicesAreValid) {
   SmallVector<LRPosInfoIndexes, 1> OverlapSetup;
   OverlapSetup.push_back({0, 10, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   ASSERT_TRUE(OverlapProblem[0].Begin.isValid());
   ASSERT_TRUE(OverlapProblem[0].End.isValid());
@@ -165,7 +165,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest, MetaSlotIndicesAreValid) {
 TEST_F(RegAllocDevelopmentFeaturesTest, InstructionOpcodesAreCorrect) {
   SmallVector<LRPosInfoIndexes, 1> OverlapSetup;
   OverlapSetup.push_back({0, ModelMaxSupportedInstructionCount - 1, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   NoInferenceModelRunner ModelRunner = setupModelRunner();
   SlotIndex LastIndex = OverlapProblem[0].End;
@@ -247,7 +247,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest, SingleMBBTest) {
 TEST_F(RegAllocDevelopmentFeaturesTest, MBBFullTruncated) {
   SmallVector<LRPosInfoIndexes, 1> OverlapSetup;
   OverlapSetup.push_back({0, ModelMaxSupportedInstructionCount - 1, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   NoInferenceModelRunner ModelRunner = setupModelRunner();
   SlotIndex LastIndex = OverlapProblem[0].End;

@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2024

@llvm/pr-subscribers-mlgo

Author: Jay Foad (jayfoad)

Changes

simple_ilist does not take ownership of its nodes, which is fine for
SlotIndexes because the IndexListEntry nodes are allocated with a
BumpPtrAllocator and do not need to be freed.


Full diff: https://github.com/llvm/llvm-project/pull/96747.diff

3 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/SlotIndexes.h (+4-8)
  • (modified) llvm/lib/CodeGen/SlotIndexes.cpp (+4-4)
  • (modified) llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp (+8-8)
diff --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index 72f4a6876b6cb..21a9df22c2e1c 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -22,7 +22,7 @@
 #include "llvm/ADT/IntervalMap.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/ilist.h"
+#include "llvm/ADT/simple_ilist.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
@@ -60,10 +60,6 @@ class raw_ostream;
     }
   };
 
-  template <>
-  struct ilist_alloc_traits<IndexListEntry>
-      : public ilist_noalloc_traits<IndexListEntry> {};
-
   /// SlotIndex - An opaque wrapper around machine indexes.
   class SlotIndex {
     friend class SlotIndexes;
@@ -302,7 +298,7 @@ class raw_ostream;
     // IndexListEntry allocator.
     BumpPtrAllocator ileAllocator;
 
-    using IndexList = ilist<IndexListEntry>;
+    using IndexList = simple_ilist<IndexListEntry>;
     IndexList indexList;
 
     MachineFunction *mf = nullptr;
@@ -549,7 +545,7 @@ class raw_ostream;
 
       // Insert a new list entry for MI.
       IndexList::iterator newItr =
-          indexList.insert(nextItr, createEntry(&MI, newNumber));
+          indexList.insert(nextItr, *createEntry(&MI, newNumber));
 
       // Renumber locally if we need to.
       if (dist == 0)
@@ -608,7 +604,7 @@ class raw_ostream;
           mbb->empty() ? endEntry
                        : getInstructionIndex(mbb->front()).listEntry();
       IndexList::iterator newItr =
-          indexList.insert(insEntry->getIterator(), startEntry);
+          indexList.insert(insEntry->getIterator(), *startEntry);
 
       SlotIndex startIdx(startEntry, SlotIndex::Slot_Block);
       SlotIndex endIdx(endEntry, SlotIndex::Slot_Block);
diff --git a/llvm/lib/CodeGen/SlotIndexes.cpp b/llvm/lib/CodeGen/SlotIndexes.cpp
index 8b80c6ccb4389..a038b09c765c0 100644
--- a/llvm/lib/CodeGen/SlotIndexes.cpp
+++ b/llvm/lib/CodeGen/SlotIndexes.cpp
@@ -26,7 +26,7 @@ SlotIndexes::SlotIndexes() : MachineFunctionPass(ID) {
 
 SlotIndexes::~SlotIndexes() {
   // The indexList's nodes are all allocated in the BumpPtrAllocator.
-  indexList.clearAndLeakNodesUnsafely();
+  indexList.clear();
 }
 
 INITIALIZE_PASS(SlotIndexes, DEBUG_TYPE,
@@ -75,7 +75,7 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) {
   MBBRanges.resize(mf->getNumBlockIDs());
   idx2MBBMap.reserve(mf->size());
 
-  indexList.push_back(createEntry(nullptr, index));
+  indexList.push_back(*createEntry(nullptr, index));
 
   // Iterate over the function.
   for (MachineBasicBlock &MBB : *mf) {
@@ -87,7 +87,7 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) {
         continue;
 
       // Insert a store index for the instr.
-      indexList.push_back(createEntry(&MI, index += SlotIndex::InstrDist));
+      indexList.push_back(*createEntry(&MI, index += SlotIndex::InstrDist));
 
       // Save this base index in the maps.
       mi2iMap.insert(std::make_pair(
@@ -95,7 +95,7 @@ bool SlotIndexes::runOnMachineFunction(MachineFunction &fn) {
     }
 
     // We insert one blank instructions between basic blocks.
-    indexList.push_back(createEntry(nullptr, index += SlotIndex::InstrDist));
+    indexList.push_back(*createEntry(nullptr, index += SlotIndex::InstrDist));
 
     MBBRanges[MBB.getNumber()].first = blockStartIndex;
     MBBRanges[MBB.getNumber()].second = SlotIndex(&indexList.back(),
diff --git a/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp b/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp
index bc2d8729474d7..6c863046e159e 100644
--- a/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp
+++ b/llvm/unittests/CodeGen/MLRegAllocDevelopmentFeatures.cpp
@@ -46,7 +46,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
 protected:
   SmallVector<LRStartEndInfo>
   setupOverlapProblem(const SmallVectorImpl<LRPosInfoIndexes> &Segments,
-                      ilist<IndexListEntry> &IndexList) {
+                      simple_ilist<IndexListEntry> &IndexList) {
     SmallVector<LRStartEndInfo> PositionsToReturn;
     PositionsToReturn.reserve(Segments.size());
     for (auto CurrentPosIndexInfo : Segments) {
@@ -61,7 +61,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
           Allocator.Allocate(sizeof(IndexListEntry), alignof(IndexListEntry)));
       auto *CurrentListEntry =
           new (CurrentLEMem) IndexListEntry(nullptr, CurrentIndex);
-      IndexList.push_back(CurrentListEntry);
+      IndexList.push_back(*CurrentListEntry);
       for (size_t CurrentPosInfoIndex = 0;
            CurrentPosInfoIndex < Segments.size(); ++CurrentPosInfoIndex) {
         if ((CurrentIndex / SlotIndex::InstrDist) ==
@@ -107,7 +107,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
   }
 
   void runOverlapTest(SmallVectorImpl<LRPosInfoIndexes> &OverlapSetup) {
-    ilist<IndexListEntry> IndexList;
+    simple_ilist<IndexListEntry> IndexList;
     auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
     NoInferenceModelRunner ModelRunner = setupModelRunner();
     size_t MaxIndex = 0;
@@ -131,7 +131,7 @@ class RegAllocDevelopmentFeaturesTest : public ::Test {
             NumberOfInterferences * ModelMaxSupportedInstructionCount);
     ASSERT_THAT(MappingMatrix,
                 ContainerEq(getExpectedMappingMatrix(OverlapSetup)));
-    IndexList.clearAndLeakNodesUnsafely();
+    IndexList.clear();
   }
 
   BumpPtrAllocator Allocator;
@@ -144,7 +144,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest,
   SmallVector<LRPosInfoIndexes, 2> OverlapSetup;
   OverlapSetup.push_back({0, 5, 0});
   OverlapSetup.push_back({5, 10, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   ASSERT_EQ(OverlapProblem[0].End.distance(OverlapProblem[1].End),
             5 * SlotIndex::InstrDist);
@@ -154,7 +154,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest,
 TEST_F(RegAllocDevelopmentFeaturesTest, MetaSlotIndicesAreValid) {
   SmallVector<LRPosInfoIndexes, 1> OverlapSetup;
   OverlapSetup.push_back({0, 10, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   ASSERT_TRUE(OverlapProblem[0].Begin.isValid());
   ASSERT_TRUE(OverlapProblem[0].End.isValid());
@@ -165,7 +165,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest, MetaSlotIndicesAreValid) {
 TEST_F(RegAllocDevelopmentFeaturesTest, InstructionOpcodesAreCorrect) {
   SmallVector<LRPosInfoIndexes, 1> OverlapSetup;
   OverlapSetup.push_back({0, ModelMaxSupportedInstructionCount - 1, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   NoInferenceModelRunner ModelRunner = setupModelRunner();
   SlotIndex LastIndex = OverlapProblem[0].End;
@@ -247,7 +247,7 @@ TEST_F(RegAllocDevelopmentFeaturesTest, SingleMBBTest) {
 TEST_F(RegAllocDevelopmentFeaturesTest, MBBFullTruncated) {
   SmallVector<LRPosInfoIndexes, 1> OverlapSetup;
   OverlapSetup.push_back({0, ModelMaxSupportedInstructionCount - 1, 0});
-  ilist<IndexListEntry> IndexList;
+  simple_ilist<IndexListEntry> IndexList;
   auto OverlapProblem = setupOverlapProblem(OverlapSetup, IndexList);
   NoInferenceModelRunner ModelRunner = setupModelRunner();
   SlotIndex LastIndex = OverlapProblem[0].End;

@jayfoad jayfoad merged commit d6f906e into llvm:main Jun 26, 2024
8 of 9 checks passed
@jayfoad jayfoad deleted the slotindexes-simple-ilist branch June 26, 2024 11:59
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
simple_ilist does not take ownership of its nodes, which is fine for
SlotIndexes because the IndexListEntry nodes are allocated with a
BumpPtrAllocator and do not need to be freed.
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
simple_ilist does not take ownership of its nodes, which is fine for
SlotIndexes because the IndexListEntry nodes are allocated with a
BumpPtrAllocator and do not need to be freed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants