From b3f5472c2b9c8cf99239a9ac655555e9f0ba9e5d Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Fri, 17 Apr 2020 19:15:01 +0200
Subject: [PATCH] [ValueLattice] Add move constructor (NFC)

Following the rule of five, declare move constructor and move
assignment operator for ValueLatticeElement. This allows moving
the ConstantRange rather than copying it.

This does not matter in most cases, where we're dealing with
APInts <= 64 bits. It does avoid unnecessary copies of allocations
for larger APInts.

Additionally we change the implementation approach to make the
copy/move assignment operators make use of the copy/move constructors,
rather than the other way around. The constructors are the more
primitive operations.

Differential Revision: https://reviews.llvm.org/D78425
---
 llvm/include/llvm/Analysis/ValueLattice.h | 79 ++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 33 deletions(-)

diff --git a/llvm/include/llvm/Analysis/ValueLattice.h b/llvm/include/llvm/Analysis/ValueLattice.h
index 04230cd..52ee591 100644
--- a/llvm/include/llvm/Analysis/ValueLattice.h
+++ b/llvm/include/llvm/Analysis/ValueLattice.h
@@ -87,6 +87,22 @@ class ValueLatticeElement {
     ConstantRange Range;
   };
 
+  /// Destroy contents of lattice value, without destructing the object.
+  void destroy() {
+    switch (Tag) {
+    case overdefined:
+    case unknown:
+    case undef:
+    case constant:
+    case notconstant:
+      break;
+    case constantrange_including_undef:
+    case constantrange:
+      Range.~ConstantRange();
+      break;
+    };
+  }
+
 public:
   /// Struct to control some aspects related to merging constant ranges.
   struct MergeOptions {
@@ -113,47 +129,34 @@ public:
     }
   };
 
-  // Const and Range are initialized on-demand.
+  // ConstVal and Range are initialized on-demand.
   ValueLatticeElement() : Tag(unknown) {}
 
-  /// Custom destructor to ensure Range is properly destroyed, when the object
-  /// is deallocated.
-  ~ValueLatticeElement() {
-    switch (Tag) {
-    case overdefined:
-    case unknown:
-    case undef:
+  ~ValueLatticeElement() { destroy(); }
+
+  ValueLatticeElement(const ValueLatticeElement &Other) : Tag(Other.Tag) {
+    switch (Other.Tag) {
+    case constantrange:
+    case constantrange_including_undef:
+      new (&Range) ConstantRange(Other.Range);
+      NumRangeExtensions = Other.NumRangeExtensions;
+      break;
     case constant:
     case notconstant:
+      ConstVal = Other.ConstVal;
       break;
-    case constantrange_including_undef:
-    case constantrange:
-      Range.~ConstantRange();
+    case overdefined:
+    case unknown:
+    case undef:
       break;
-    };
-  }
-
-  /// Custom copy constructor, to ensure Range gets initialized when
-  /// copying a constant range lattice element.
-  ValueLatticeElement(const ValueLatticeElement &Other) : Tag(unknown) {
-    *this = Other;
+    }
   }
 
-  /// Custom assignment operator, to ensure Range gets initialized when
-  /// assigning a constant range lattice element.
-  ValueLatticeElement &operator=(const ValueLatticeElement &Other) {
-    // If we change the state of this from constant range to non constant range,
-    // destroy Range.
-    if (isConstantRange() && !Other.isConstantRange())
-      Range.~ConstantRange();
-
+  ValueLatticeElement(ValueLatticeElement &&Other) : Tag(Other.Tag) {
     switch (Other.Tag) {
     case constantrange:
     case constantrange_including_undef:
-      if (!isConstantRange())
-        new (&Range) ConstantRange(Other.Range);
-      else
-        Range = Other.Range;
+      new (&Range) ConstantRange(std::move(Other.Range));
       NumRangeExtensions = Other.NumRangeExtensions;
       break;
     case constant:
@@ -165,7 +168,18 @@ public:
     case undef:
       break;
     }
-    Tag = Other.Tag;
+    Other.Tag = unknown;
+  }
+
+  ValueLatticeElement &operator=(const ValueLatticeElement &Other) {
+    destroy();
+    new (this) ValueLatticeElement(Other);
+    return *this;
+  }
+
+  ValueLatticeElement &operator=(ValueLatticeElement &&Other) {
+    destroy();
+    new (this) ValueLatticeElement(std::move(Other));
     return *this;
   }
 
@@ -249,8 +263,7 @@ public:
   bool markOverdefined() {
     if (isOverdefined())
       return false;
-    if (isConstantRange())
-      Range.~ConstantRange();
+    destroy();
     Tag = overdefined;
     return true;
   }
-- 
2.7.4