[Mlir-commits] [mlir] c89dff5 - [mlir][NFC] Split the non-templated bits out of IROperand into a base class

River Riddle llvmlistbot at llvm.org
Wed Jun 2 12:54:56 PDT 2021


Author: River Riddle
Date: 2021-06-02T12:48:37-07:00
New Revision: c89dff5855bb32d47751cce087537c2b12a90f1b

URL: https://github.com/llvm/llvm-project/commit/c89dff5855bb32d47751cce087537c2b12a90f1b
DIFF: https://github.com/llvm/llvm-project/commit/c89dff5855bb32d47751cce087537c2b12a90f1b.diff

LOG: [mlir][NFC] Split the non-templated bits out of IROperand into a base class

This removes the need to define the derived Operand class before the derived
Value class. The major benefit of this refactoring is that we no longer need
the OpaqueValue class, as OpOperand can now be defined after Value. As part of
this refactoring the BlockOperand and OpOperand classes are moved out of
UseDefLists.h and to more suitable locations in BlockSupport and Value. After
this change, UseDefLists.h is almost entirely composed of generic use def utilities.

Differential Revision: https://reviews.llvm.org/D103353

Added: 
    

Modified: 
    mlir/include/mlir/IR/BlockSupport.h
    mlir/include/mlir/IR/UseDefLists.h
    mlir/include/mlir/IR/Value.h
    mlir/lib/IR/Value.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/BlockSupport.h b/mlir/include/mlir/IR/BlockSupport.h
index 6cf2df9a1406..b945111f5efa 100644
--- a/mlir/include/mlir/IR/BlockSupport.h
+++ b/mlir/include/mlir/IR/BlockSupport.h
@@ -21,6 +21,23 @@
 namespace mlir {
 class Block;
 
+//===----------------------------------------------------------------------===//
+// BlockOperand
+//===----------------------------------------------------------------------===//
+
+/// A block operand represents an operand that holds a reference to a Block,
+/// e.g. for terminator operations.
+class BlockOperand : public IROperand<BlockOperand, Block *> {
+public:
+  using IROperand<BlockOperand, Block *>::IROperand;
+
+  /// Provide the use list that is attached to the given block.
+  static IRObjectWithUseList<BlockOperand> *getUseList(Block *value);
+
+  /// Return which operand this is in the BlockOperand list of the Operation.
+  unsigned getOperandNumber();
+};
+
 //===----------------------------------------------------------------------===//
 // Predecessors
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/include/mlir/IR/UseDefLists.h b/mlir/include/mlir/IR/UseDefLists.h
index de74b01e1391..d9c403810942 100644
--- a/mlir/include/mlir/IR/UseDefLists.h
+++ b/mlir/include/mlir/IR/UseDefLists.h
@@ -19,109 +19,121 @@
 
 namespace mlir {
 
-class Block;
 class Operation;
-class Value;
 template <typename OperandType> class ValueUseIterator;
 template <typename OperandType> class FilteredValueUseIterator;
 template <typename UseIteratorT, typename OperandType> class ValueUserIterator;
 
 //===----------------------------------------------------------------------===//
-// IRObjectWithUseList
+// IROperand
 //===----------------------------------------------------------------------===//
 
-/// This class represents a single IR object that contains a use list.
-template <typename OperandType> class IRObjectWithUseList {
+namespace detail {
+/// This class is the base for IROperand, and provides all of the non-templated
+/// facilities for operand use management.
+class IROperandBase {
 public:
-  ~IRObjectWithUseList() {
-    assert(use_empty() && "Cannot destroy a value that still has uses!");
-  }
+  /// Return the owner of this operand.
+  Operation *getOwner() const { return owner; }
 
-  /// Drop all uses of this object from their respective owners.
-  void dropAllUses() {
-    while (!use_empty())
-      use_begin()->drop();
-  }
+  /// Return the next operand on the use-list of the value we are referring to.
+  /// This should generally only be used by the internal implementation details
+  /// of the SSA machinery.
+  IROperandBase *getNextOperandUsingThisValue() { return nextUse; }
 
-  /// Replace all uses of 'this' value with the new value, updating anything in
-  /// the IR that uses 'this' to use the other value instead.  When this returns
-  /// there are zero uses of 'this'.
-  void replaceAllUsesWith(typename OperandType::ValueType newValue) {
-    assert((!newValue || this != OperandType::getUseList(newValue)) &&
-           "cannot RAUW a value with itself");
-    while (!use_empty())
-      use_begin()->set(newValue);
+protected:
+  IROperandBase(Operation *owner) : owner(owner) {}
+  IROperandBase(IROperandBase &&other) : owner(other.owner) {
+    *this = std::move(other);
   }
-
-  //===--------------------------------------------------------------------===//
-  // Uses
-  //===--------------------------------------------------------------------===//
-
-  using use_iterator = ValueUseIterator<OperandType>;
-  using use_range = iterator_range<use_iterator>;
-
-  use_iterator use_begin() const { return use_iterator(firstUse); }
-  use_iterator use_end() const { return use_iterator(nullptr); }
-
-  /// Returns a range of all uses, which is useful for iterating over all uses.
-  use_range getUses() const { return {use_begin(), use_end()}; }
-
-  /// Returns true if this value has exactly one use.
-  bool hasOneUse() const {
-    return firstUse && firstUse->getNextOperandUsingThisValue() == nullptr;
+  IROperandBase &operator=(IROperandBase &&other) {
+    removeFromCurrent();
+    other.removeFromCurrent();
+    other.back = nullptr;
+    nextUse = nullptr;
+    back = nullptr;
+    return *this;
   }
+  /// Operands are not copyable or assignable.
+  IROperandBase(const IROperandBase &use) = delete;
+  IROperandBase &operator=(const IROperandBase &use) = delete;
 
-  /// Returns true if this value has no uses.
-  bool use_empty() const { return firstUse == nullptr; }
-
-  //===--------------------------------------------------------------------===//
-  // Users
-  //===--------------------------------------------------------------------===//
+  ~IROperandBase() { removeFromCurrent(); }
 
-  using user_iterator = ValueUserIterator<use_iterator, OperandType>;
-  using user_range = iterator_range<user_iterator>;
+  /// Remove this use of the operand.
+  void drop() {
+    removeFromCurrent();
+    nextUse = nullptr;
+    back = nullptr;
+  }
 
-  user_iterator user_begin() const { return user_iterator(use_begin()); }
-  user_iterator user_end() const { return user_iterator(use_end()); }
+  /// Remove this operand from the current use list.
+  void removeFromCurrent() {
+    if (!back)
+      return;
+    *back = nextUse;
+    if (nextUse)
+      nextUse->back = back;
+  }
 
-  /// Returns a range of all users.
-  user_range getUsers() const { return {user_begin(), user_end()}; }
+  /// Insert this operand into the given use list.
+  template <typename UseListT> void insertInto(UseListT *useList) {
+    back = &useList->firstUse;
+    nextUse = useList->firstUse;
+    if (nextUse)
+      nextUse->back = &nextUse;
+    useList->firstUse = this;
+  }
 
-protected:
-  IRObjectWithUseList() {}
+  /// The next operand in the use-chain.
+  IROperandBase *nextUse = nullptr;
 
-  /// Return the first operand that is using this value, for use by custom
-  /// use/def iterators.
-  OperandType *getFirstUse() const { return firstUse; }
+  /// This points to the previous link in the use-chain.
+  IROperandBase **back = nullptr;
 
 private:
-  template <typename DerivedT, typename IRValueTy> friend class IROperand;
-  OperandType *firstUse = nullptr;
+  /// The operation owner of this operand.
+  Operation *const owner;
 };
+} // namespace detail
 
 //===----------------------------------------------------------------------===//
 // IROperand
 //===----------------------------------------------------------------------===//
 
 /// A reference to a value, suitable for use as an operand of an operation.
-/// IRValueTy is the root type to use for values this tracks. Derived operand
+/// IRValueT is the root type to use for values this tracks. Derived operand
 /// types are expected to provide the following:
-///  * static IRObjectWithUseList *getUseList(IRValueTy value);
+///  * static IRObjectWithUseList *getUseList(IRValueT value);
 ///    - Provide the use list that is attached to the given value.
-template <typename DerivedT, typename IRValueTy> class IROperand {
+template <typename DerivedT, typename IRValueT>
+class IROperand : public detail::IROperandBase {
 public:
-  using ValueType = IRValueTy;
-
-  IROperand(Operation *owner) : owner(owner) {}
-  IROperand(Operation *owner, ValueType value) : value(value), owner(owner) {
+  IROperand(Operation *owner) : detail::IROperandBase(owner) {}
+  IROperand(Operation *owner, IRValueT value)
+      : detail::IROperandBase(owner), value(value) {
     insertIntoCurrent();
   }
 
+  /// We support a move constructor so IROperand's can be in vectors, but this
+  /// shouldn't be used by general clients.
+  IROperand(IROperand &&other) : detail::IROperandBase(std::move(other)) {
+    *this = std::move(other);
+  }
+  IROperand &operator=(IROperand &&other) {
+    detail::IROperandBase::operator=(std::move(other));
+    value = other.value;
+    other.value = nullptr;
+    if (value)
+      insertIntoCurrent();
+    return *this;
+  }
+
   /// Return the current value being used by this operand.
-  ValueType get() const { return value; }
+  IRValueT get() const { return value; }
 
   /// Set the current value being used by this operand.
-  void set(ValueType newValue) {
+  void set(IRValueT newValue) {
     // It isn't worth optimizing for the case of switching operands on a single
     // value.
     removeFromCurrent();
@@ -130,142 +142,97 @@ template <typename DerivedT, typename IRValueTy> class IROperand {
   }
 
   /// Returns true if this operand contains the given value.
-  bool is(ValueType other) const { return value == other; }
-
-  /// Return the owner of this operand.
-  Operation *getOwner() { return owner; }
-  Operation *getOwner() const { return owner; }
+  bool is(IRValueT other) const { return value == other; }
 
   /// \brief Remove this use of the operand.
   void drop() {
-    removeFromCurrent();
+    detail::IROperandBase::drop();
     value = nullptr;
-    nextUse = nullptr;
-    back = nullptr;
-  }
-
-  ~IROperand() { removeFromCurrent(); }
-
-  /// Return the next operand on the use-list of the value we are referring to.
-  /// This should generally only be used by the internal implementation details
-  /// of the SSA machinery.
-  DerivedT *getNextOperandUsingThisValue() { return nextUse; }
-
-  /// We support a move constructor so IROperand's can be in vectors, but this
-  /// shouldn't be used by general clients.
-  IROperand(IROperand &&other) : owner(other.owner) {
-    *this = std::move(other);
-  }
-  IROperand &operator=(IROperand &&other) {
-    removeFromCurrent();
-    other.removeFromCurrent();
-    value = other.value;
-    other.value = nullptr;
-    other.back = nullptr;
-    nextUse = nullptr;
-    back = nullptr;
-    if (value)
-      insertIntoCurrent();
-    return *this;
   }
 
 private:
   /// The value used as this operand. This can be null when in a 'dropAllUses'
   /// state.
-  ValueType value = {};
-
-  /// The next operand in the use-chain.
-  DerivedT *nextUse = nullptr;
+  IRValueT value = {};
 
-  /// This points to the previous link in the use-chain.
-  DerivedT **back = nullptr;
+  /// Insert this operand into the given use list.
+  void insertIntoCurrent() { insertInto(DerivedT::getUseList(value)); }
+};
 
-  /// The operation owner of this operand.
-  Operation *const owner;
+//===----------------------------------------------------------------------===//
+// IRObjectWithUseList
+//===----------------------------------------------------------------------===//
 
-  /// Operands are not copyable or assignable.
-  IROperand(const IROperand &use) = delete;
-  IROperand &operator=(const IROperand &use) = delete;
+/// This class represents a single IR object that contains a use list.
+template <typename OperandType> class IRObjectWithUseList {
+public:
+  ~IRObjectWithUseList() {
+    assert(use_empty() && "Cannot destroy a value that still has uses!");
+  }
 
-  void removeFromCurrent() {
-    if (!back)
-      return;
-    *back = nextUse;
-    if (nextUse)
-      nextUse->back = back;
+  /// Drop all uses of this object from their respective owners.
+  void dropAllUses() {
+    while (!use_empty())
+      use_begin()->drop();
   }
 
-  void insertIntoCurrent() {
-    auto *useList = DerivedT::getUseList(value);
-    back = &useList->firstUse;
-    nextUse = useList->firstUse;
-    if (nextUse)
-      nextUse->back = &nextUse;
-    useList->firstUse = static_cast<DerivedT *>(this);
+  /// Replace all uses of 'this' value with the new value, updating anything in
+  /// the IR that uses 'this' to use the other value instead.  When this returns
+  /// there are zero uses of 'this'.
+  template <typename ValueT>
+  void replaceAllUsesWith(ValueT &&newValue) {
+    assert((!newValue || this != OperandType::getUseList(newValue)) &&
+           "cannot RAUW a value with itself");
+    while (!use_empty())
+      use_begin()->set(newValue);
   }
-};
 
-//===----------------------------------------------------------------------===//
-// BlockOperand
-//===----------------------------------------------------------------------===//
+  //===--------------------------------------------------------------------===//
+  // Uses
+  //===--------------------------------------------------------------------===//
 
-/// Terminator operations can have Block operands to represent successors.
-class BlockOperand : public IROperand<BlockOperand, Block *> {
-public:
-  using IROperand<BlockOperand, Block *>::IROperand;
+  using use_iterator = ValueUseIterator<OperandType>;
+  using use_range = iterator_range<use_iterator>;
 
-  /// Provide the use list that is attached to the given block.
-  static IRObjectWithUseList<BlockOperand> *getUseList(Block *value);
+  use_iterator use_begin() const { return use_iterator(firstUse); }
+  use_iterator use_end() const { return use_iterator(nullptr); }
 
-  /// Return which operand this is in the operand list of the User.
-  unsigned getOperandNumber();
-};
+  /// Returns a range of all uses, which is useful for iterating over all uses.
+  use_range getUses() const { return {use_begin(), use_end()}; }
 
-//===----------------------------------------------------------------------===//
-// OpOperand
-//===----------------------------------------------------------------------===//
+  /// Returns true if this value has exactly one use.
+  bool hasOneUse() const {
+    return firstUse && firstUse->getNextOperandUsingThisValue() == nullptr;
+  }
 
-namespace detail {
-/// This class provides an opaque type erased wrapper around a `Value`.
-class OpaqueValue {
-public:
-  /// Implicit conversion from 'Value'.
-  OpaqueValue(Value value);
-  OpaqueValue(std::nullptr_t = nullptr) : impl(nullptr) {}
-  OpaqueValue(const OpaqueValue &) = default;
-  OpaqueValue &operator=(const OpaqueValue &) = default;
-  bool operator==(const OpaqueValue &other) const { return impl == other.impl; }
-  operator bool() const { return impl; }
+  /// Returns true if this value has no uses.
+  bool use_empty() const { return firstUse == nullptr; }
 
-  /// Implicit conversion back to 'Value'.
-  operator Value() const;
+  //===--------------------------------------------------------------------===//
+  // Users
+  //===--------------------------------------------------------------------===//
 
-private:
-  void *impl;
-};
-} // namespace detail
+  using user_iterator = ValueUserIterator<use_iterator, OperandType>;
+  using user_range = iterator_range<user_iterator>;
 
-/// This class represents an operand of an operation. Instances of this class
-/// contain a reference to a specific `Value`.
-class OpOperand : public IROperand<OpOperand, detail::OpaqueValue> {
-public:
-  /// Provide the use list that is attached to the given value.
-  static IRObjectWithUseList<OpOperand> *getUseList(Value value);
+  user_iterator user_begin() const { return user_iterator(use_begin()); }
+  user_iterator user_end() const { return user_iterator(use_end()); }
 
-  /// Return the current value being used by this operand.
-  Value get() const;
+  /// Returns a range of all users.
+  user_range getUsers() const { return {user_begin(), user_end()}; }
 
-  /// Set the operand to the given value.
-  void set(Value value);
+protected:
+  IRObjectWithUseList() = default;
 
-  /// Return which operand this is in the operand list of the User.
-  unsigned getOperandNumber();
+  /// Return the first operand that is using this value, for use by custom
+  /// use/def iterators.
+  OperandType *getFirstUse() const { return (OperandType *)firstUse; }
 
 private:
-  /// Keep the constructor private and accessible to the OperandStorage class
-  /// only to avoid hard-to-debug typo/programming mistakes.
-  friend class OperandStorage;
-  using IROperand<OpOperand, detail::OpaqueValue>::IROperand;
+  detail::IROperandBase *firstUse = nullptr;
+
+  /// Allow access to `firstUse`.
+  friend detail::IROperandBase;
 };
 
 //===----------------------------------------------------------------------===//
@@ -280,14 +247,14 @@ class ValueUseIterator
                                         std::forward_iterator_tag,
                                         OperandType> {
 public:
-  ValueUseIterator(OperandType *current = nullptr) : current(current) {}
+  ValueUseIterator(detail::IROperandBase *use = nullptr) : current(use) {}
 
-  /// Returns the user that owns this use.
+  /// Returns the operation that owns this use.
   Operation *getUser() const { return current->getOwner(); }
 
   /// Returns the current operands.
-  OperandType *getOperand() const { return current; }
-  OperandType &operator*() const { return *current; }
+  OperandType *getOperand() const { return (OperandType *)current; }
+  OperandType &operator*() const { return *getOperand(); }
 
   using llvm::iterator_facade_base<ValueUseIterator<OperandType>,
                                    std::forward_iterator_tag,
@@ -303,7 +270,7 @@ class ValueUseIterator
   }
 
 protected:
-  OperandType *current;
+  detail::IROperandBase *current;
 };
 
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/include/mlir/IR/Value.h b/mlir/include/mlir/IR/Value.h
index 1901802e3dff..7ea8b265705f 100644
--- a/mlir/include/mlir/IR/Value.h
+++ b/mlir/include/mlir/IR/Value.h
@@ -20,8 +20,10 @@
 
 namespace mlir {
 class AsmState;
+class Block;
 class BlockArgument;
 class Operation;
+class OpOperand;
 class OpResult;
 class Region;
 class Value;
@@ -152,16 +154,15 @@ class Value {
   // UseLists
   //===--------------------------------------------------------------------===//
 
-  /// Provide the use list that is attached to this value.
-  IRObjectWithUseList<OpOperand> *getUseList() const { return impl; }
-
   /// Drop all uses of this object from their respective owners.
-  void dropAllUses() const { return getUseList()->dropAllUses(); }
+  void dropAllUses() const { return impl->dropAllUses(); }
 
   /// Replace all uses of 'this' value with the new value, updating anything in
   /// the IR that uses 'this' to use the other value instead.  When this returns
   /// there are zero uses of 'this'.
-  void replaceAllUsesWith(Value newValue) const;
+  void replaceAllUsesWith(Value newValue) const {
+    impl->replaceAllUsesWith(newValue);
+  }
 
   /// Replace all uses of 'this' value with 'newValue', updating anything in the
   /// IR that uses 'this' to use the other value instead except if the user is
@@ -190,17 +191,17 @@ class Value {
   using use_iterator = ValueUseIterator<OpOperand>;
   using use_range = iterator_range<use_iterator>;
 
-  use_iterator use_begin() const { return getUseList()->use_begin(); }
+  use_iterator use_begin() const { return impl->use_begin(); }
   use_iterator use_end() const { return use_iterator(); }
 
   /// Returns a range of all uses, which is useful for iterating over all uses.
   use_range getUses() const { return {use_begin(), use_end()}; }
 
   /// Returns true if this value has exactly one use.
-  bool hasOneUse() const { return getUseList()->hasOneUse(); }
+  bool hasOneUse() const { return impl->hasOneUse(); }
 
   /// Returns true if this value has no uses.
-  bool use_empty() const { return getUseList()->use_empty(); }
+  bool use_empty() const { return impl->use_empty(); }
 
   //===--------------------------------------------------------------------===//
   // Users
@@ -241,6 +242,29 @@ inline raw_ostream &operator<<(raw_ostream &os, Value value) {
   return os;
 }
 
+//===----------------------------------------------------------------------===//
+// OpOperand
+//===----------------------------------------------------------------------===//
+
+/// This class represents an operand of an operation. Instances of this class
+/// contain a reference to a specific `Value`.
+class OpOperand : public IROperand<OpOperand, Value> {
+public:
+  /// Provide the use list that is attached to the given value.
+  static IRObjectWithUseList<OpOperand> *getUseList(Value value) {
+    return value.getImpl();
+  }
+
+  /// Return which operand this is in the OpOperand list of the Operation.
+  unsigned getOperandNumber();
+
+private:
+  /// Keep the constructor private and accessible to the OperandStorage class
+  /// only to avoid hard-to-debug typo/programming mistakes.
+  friend class OperandStorage;
+  using IROperand<OpOperand, Value>::IROperand;
+};
+
 //===----------------------------------------------------------------------===//
 // BlockArgument
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/lib/IR/Value.cpp b/mlir/lib/IR/Value.cpp
index 1ff0bba64fdf..ea730eb46422 100644
--- a/mlir/lib/IR/Value.cpp
+++ b/mlir/lib/IR/Value.cpp
@@ -55,13 +55,6 @@ Block *Value::getParentBlock() {
 // Value::UseLists
 //===----------------------------------------------------------------------===//
 
-/// Replace all uses of 'this' value with the new value, updating anything in
-/// the IR that uses 'this' to use the other value instead.  When this returns
-/// there are zero uses of 'this'.
-void Value::replaceAllUsesWith(Value newValue) const {
-  return getUseList()->replaceAllUsesWith(newValue);
-}
-
 /// Replace all uses of 'this' value with the new value, updating anything in
 /// the IR that uses 'this' to use the other value instead except if the user is
 /// listed in 'exceptions' .
@@ -215,34 +208,8 @@ unsigned BlockOperand::getOperandNumber() {
 // OpOperand
 //===----------------------------------------------------------------------===//
 
-/// Provide the use list that is attached to the given value.
-IRObjectWithUseList<OpOperand> *OpOperand::getUseList(Value value) {
-  return value.getUseList();
-}
-
-/// Return the current value being used by this operand.
-Value OpOperand::get() const {
-  return IROperand<OpOperand, OpaqueValue>::get();
-}
-
-/// Set the operand to the given value.
-void OpOperand::set(Value value) {
-  IROperand<OpOperand, OpaqueValue>::set(value);
-}
-
 /// Return which operand this is in the operand list.
 unsigned OpOperand::getOperandNumber() {
   return this - &getOwner()->getOpOperands()[0];
 }
 
-//===----------------------------------------------------------------------===//
-// OpaqueValue
-//===----------------------------------------------------------------------===//
-
-/// Implicit conversion from 'Value'.
-OpaqueValue::OpaqueValue(Value value) : impl(value.getAsOpaquePointer()) {}
-
-/// Implicit conversion back to 'Value'.
-OpaqueValue::operator Value() const {
-  return Value::getFromOpaquePointer(impl);
-}


        


More information about the Mlir-commits mailing list