[PATCH] D113240: Move constructor for IntervalMap.

[Stanislav Funiak via Phabricator via llvm-commits]

sfuniak created this revision.
sfuniak added a reviewer: stoklund.
Herald added subscribers: Chia-hungDuan, dexonsmith, rriddle.
sfuniak requested review of this revision.
Herald added subscribers: llvm-commits, stephenneuendorffer.
Herald added a project: LLVM.

`IntervalMap` has a user-defined destructor but no user-defined copy constructor. The (compiler-generated) implicitly-declared copy constructor is wrong, because it performs a trivial copy, which is problematic when the IntervalMap contains more than N intervals. This copy constructor is invoked in some places that assume that IntervalMap can be copied, or at least moved, e.g. when generating the MLIR PDL bytecode. The solution is to define a move constructor that will be used instead of the implicitly-declared copy constructor.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113240

Files:
  llvm/include/llvm/ADT/IntervalMap.h
  llvm/unittests/ADT/IntervalMapTest.cpp


Index: llvm/unittests/ADT/IntervalMapTest.cpp
===================================================================
--- llvm/unittests/ADT/IntervalMapTest.cpp
+++ llvm/unittests/ADT/IntervalMapTest.cpp
@@ -664,6 +664,48 @@
   EXPECT_FALSE(map.overlaps(66, 67));
 }
 
+TEST(IntervalMapTest, MoveEmpty) {
+  UUMap::Allocator allocator;
+  UUMap map(allocator);
+  UUMap map2(std::move(map));
+  EXPECT_TRUE(map2.empty());
+}
+
+TEST(IntervalMapTest, MoveSingleEntry) {
+  UUMap::Allocator allocator;
+  UUMap map(allocator);
+  map.insert(10, 20, 0);
+  UUMap map2(std::move(map));
+
+  UUMap::iterator I = map2.begin();
+  ASSERT_TRUE(I.valid());
+  EXPECT_EQ(10u, I.start());
+  EXPECT_EQ(20u, I.stop());
+  EXPECT_EQ(0u, I.value());
+  ++I;
+  EXPECT_FALSE(I.valid());
+}
+
+TEST(IntervalMapTest, MoveLargeMap) {
+  UUMap::Allocator allocator;
+  UUMap map(allocator);
+  for (unsigned i = 0; i < 500; i += 20) {
+    map.insert(i, i + 10, 0);
+  }
+  UUMap map2(std::move(map));
+
+  UUMap::iterator I = map2.begin();
+  for (unsigned i = 0; i < 500; i += 20) {
+    ASSERT_TRUE(I.valid());
+    EXPECT_EQ(i, I.start());
+    EXPECT_EQ(i + 10, I.stop());
+    EXPECT_EQ(0u, I.value());
+    ++I;
+  }
+
+  EXPECT_FALSE(I.valid());
+}
+
 TEST(IntervalMapOverlapsTest, SmallMaps) {
   typedef IntervalMapOverlaps<UUMap,UUMap> UUOverlaps;
   UUMap::Allocator allocator;
Index: llvm/include/llvm/ADT/IntervalMap.h
===================================================================
--- llvm/include/llvm/ADT/IntervalMap.h
+++ llvm/include/llvm/ADT/IntervalMap.h
@@ -1046,6 +1046,13 @@
     new(&rootLeaf()) RootLeaf();
   }
 
+  IntervalMap(IntervalMap &&other)
+    : IntervalMap(other.allocator) {
+    std::swap(data, other.data);
+    std::swap(height, other.height);
+    std::swap(rootSize, other.rootSize);
+  }
+
   ~IntervalMap() {
     clear();
     rootLeaf().~RootLeaf();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113240.384954.patch
Type: text/x-patch
Size: 1880 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211105/21796f8e/attachment.bin>