Automated rollback of commit f603a50109

PiperOrigin-RevId: 286924059

mlir/bindings/python/pybind.cpp

@@ -87,8 +87,7 @@ struct PythonValueHandle {
   operator ValueHandle &() { return value; }
 
   std::string str() const {
-    return std::to_string(
-        reinterpret_cast<intptr_t>(value.getValue().getAsOpaquePointer()));
+    return std::to_string(reinterpret_cast<intptr_t>(value.getValue()));
   }
 
   PythonValueHandle call(const std::vector<PythonValueHandle> &args) {

mlir/include/mlir/Analysis/AffineAnalysis.h

@@ -15,7 +15,9 @@
 #ifndef MLIR_ANALYSIS_AFFINE_ANALYSIS_H
 #define MLIR_ANALYSIS_AFFINE_ANALYSIS_H
 
-#include "mlir/IR/Value.h"
+#include "mlir/Support/LLVM.h"
+#include "mlir/Support/LogicalResult.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 
@@ -26,9 +28,10 @@ class AffineForOp;
 class AffineValueMap;
 class FlatAffineConstraints;
 class Operation;
+class Value;
 
 // TODO(riverriddle) Remove this after Value is value-typed.
-using ValuePtr = Value;
+using ValuePtr = Value *;
 
 /// Returns in `affineApplyOps`, the sequence of those AffineApplyOp
 /// Operations that are reachable via a search starting from `operands` and

mlir/include/mlir/Analysis/Liveness.h

@@ -33,7 +33,7 @@ class Region;
 class Value;
 
 // TODO(riverriddle) Remove this after Value is value-typed.
-using ValuePtr = Value;
+using ValuePtr = Value *;
 
 /// Represents an analysis for computing liveness information from a
 /// given top-level operation. The analysis iterates over all associated

mlir/include/mlir/Analysis/LoopAnalysis.h

@@ -28,7 +28,7 @@ class Operation;
 class Value;
 
 // TODO(riverriddle) Remove this after Value is value-typed.
-using ValuePtr = Value;
+using ValuePtr = Value *;
 
 /// Returns the trip count of the loop as an affine map with its corresponding
 /// operands if the latter is expressible as an affine expression, and nullptr

mlir/include/mlir/Conversion/AffineToStandard/AffineToStandard.h

@@ -22,7 +22,7 @@ class RewritePattern;
 class Value;
 
 // TODO(riverriddle) Remove this after Value is value-typed.
-using ValuePtr = Value;
+using ValuePtr = Value *;
 
 // Owning list of rewriting patterns.
 class OwningRewritePatternList;

mlir/include/mlir/Conversion/LoopsToGPU/LoopsToGPU.h

@@ -16,7 +16,7 @@ struct LogicalResult;
 class Value;
 
 // TODO(riverriddle) Remove this after Value is value-typed.
-using ValuePtr = Value;
+using ValuePtr = Value *;
 
 namespace loop {
 class ForOp;

mlir/include/mlir/Dialect/VectorOps/Utils.h

@@ -26,7 +26,7 @@ class Value;
 class VectorType;
 
 // TODO(riverriddle) Remove this after Value is value-typed.
-using ValuePtr = Value;
+using ValuePtr = Value *;
 
 /// Computes and returns the multi-dimensional ratio of `superShape` to
 /// `subShape`. This is calculated by performing a traversal from minor to major

mlir/include/mlir/EDSC/Builders.h

@@ -329,7 +329,6 @@ public:
 
   /// Implicit conversion useful for automatic conversion to Container<Value>.
   operator ValuePtr() const { return getValue(); }
-  operator bool() const { return hasValue(); }
 
   /// Generic mlir::Op create. This is the key to being extensible to the whole
   /// of MLIR without duplicating the type system or the op definitions.

mlir/include/mlir/IR/Block.h

@@ -63,7 +63,7 @@ public:
   //===--------------------------------------------------------------------===//
 
   // This is the list of arguments to the block.
-  using BlockArgListType = MutableArrayRef<BlockArgumentPtr>;
+  using BlockArgListType = ArrayRef<BlockArgumentPtr>;
 
   BlockArgListType getArguments() { return arguments; }
 

mlir/include/mlir/IR/BlockAndValueMapping.h

@@ -28,18 +28,14 @@ public:
   /// Inserts a new mapping for 'from' to 'to'. If there is an existing mapping,
   /// it is overwritten.
   void map(Block *from, Block *to) { valueMap[from] = to; }
-  void map(Value from, Value to) {
-    valueMap[from.getAsOpaquePointer()] = to.getAsOpaquePointer();
-  }
+  void map(ValuePtr from, ValuePtr to) { valueMap[from] = to; }
 
   /// Erases a mapping for 'from'.
-  void erase(Block *from) { valueMap.erase(from); }
-  void erase(Value from) { valueMap.erase(from.getAsOpaquePointer()); }
+  void erase(IRObjectWithUseList *from) { valueMap.erase(from); }
 
   /// Checks to see if a mapping for 'from' exists.
-  bool contains(Block *from) const { return valueMap.count(from); }
-  bool contains(Value from) const {
-    return valueMap.count(from.getAsOpaquePointer());
+  bool contains(IRObjectWithUseList *from) const {
+    return valueMap.count(from);
   }
 
   /// Lookup a mapped value within the map. If a mapping for the provided value
@@ -47,19 +43,23 @@ public:
   Block *lookupOrNull(Block *from) const {
     return lookupOrValue(from, (Block *)nullptr);
   }
-  Value lookupOrNull(Value from) const { return lookupOrValue(from, Value()); }
+  ValuePtr lookupOrNull(ValuePtr from) const {
+    return lookupOrValue(from, (ValuePtr) nullptr);
+  }
 
   /// Lookup a mapped value within the map. If a mapping for the provided value
   /// does not exist then return the provided value.
   Block *lookupOrDefault(Block *from) const {
     return lookupOrValue(from, from);
   }
-  Value lookupOrDefault(Value from) const { return lookupOrValue(from, from); }
+  ValuePtr lookupOrDefault(ValuePtr from) const {
+    return lookupOrValue(from, from);
+  }
 
   /// Lookup a mapped value within the map. This asserts the provided value
   /// exists within the map.
-  template <typename T> T lookup(T from) const {
-    auto result = lookupOrNull(from);
+  template <typename T> T *lookup(T *from) const {
+    auto *result = lookupOrNull(from);
     assert(result && "expected 'from' to be contained within the map");
     return result;
   }
@@ -69,18 +69,14 @@ public:
 
 private:
   /// Utility lookupOrValue that looks up an existing key or returns the
-  /// provided value.
-  Block *lookupOrValue(Block *from, Block *value) const {
+  /// provided value. This function assumes that if a mapping does exist, then
+  /// it is of 'T' type.
+  template <typename T> T *lookupOrValue(T *from, T *value) const {
     auto it = valueMap.find(from);
-    return it != valueMap.end() ? reinterpret_cast<Block *>(it->second) : value;
-  }
-  Value lookupOrValue(Value from, Value value) const {
-    auto it = valueMap.find(from.getAsOpaquePointer());
-    return it != valueMap.end() ? Value::getFromOpaquePointer(it->second)
-                                : value;
+    return it != valueMap.end() ? static_cast<T *>(it->second) : value;
   }
 
-  DenseMap<void *, void *> valueMap;
+  DenseMap<IRObjectWithUseList *, IRObjectWithUseList *> valueMap;
 };
 
 } // end namespace mlir

mlir/include/mlir/IR/OpImplementation.h

@@ -142,14 +142,17 @@ private:
 
 // Make the implementations convenient to use.
 inline OpAsmPrinter &operator<<(OpAsmPrinter &p, ValueRef value) {
-  p.printOperand(value);
+  p.printOperand(&value);
   return p;
 }
+inline OpAsmPrinter &operator<<(OpAsmPrinter &p, ValuePtr value) {
+  return p << *value;
+}
 
-template <typename T,
-          typename std::enable_if<std::is_convertible<T &, ValueRange>::value &&
-                                      !std::is_convertible<T &, Value &>::value,
-                                  T>::type * = nullptr>
+template <typename T, typename std::enable_if<
+                          std::is_convertible<T &, ValueRange>::value &&
+                              !std::is_convertible<T &, ValuePtr>::value,
+                          T>::type * = nullptr>
 inline OpAsmPrinter &operator<<(OpAsmPrinter &p, const T &values) {
   p.printOperands(values);
   return p;
@@ -169,7 +172,8 @@ inline OpAsmPrinter &operator<<(OpAsmPrinter &p, Attribute attr) {
 // even if it isn't exactly one of them.  For example, we want to print
 // FunctionType with the Type version above, not have it match this.
 template <typename T, typename std::enable_if<
-                          !std::is_convertible<T &, Value &>::value &&
+                          !std::is_convertible<T &, ValueRef>::value &&
+                              !std::is_convertible<T &, ValuePtr>::value &&
                               !std::is_convertible<T &, Type &>::value &&
                               !std::is_convertible<T &, Attribute &>::value &&
                               !std::is_convertible<T &, ValueRange>::value &&

mlir/include/mlir/IR/Operation.h

@@ -246,7 +246,7 @@ public:
 
   unsigned getNumResults() { return numResults; }
 
-  ValuePtr getResult(unsigned idx) { return getOpResult(idx); }
+  ValuePtr getResult(unsigned idx) { return &getOpResult(idx); }
 
   /// Support result iteration.
   using result_range = ResultRange;

mlir/include/mlir/IR/OperationSupport.h

@@ -525,8 +525,8 @@ private:
 /// This class implements iteration on the types of a given range of values.
 template <typename ValueIteratorT>
 class ValueTypeIterator final
-    : public llvm::mapped_iterator<ValueIteratorT, Type (*)(Value)> {
-  static Type unwrap(Value value) { return value.getType(); }
+    : public llvm::mapped_iterator<ValueIteratorT, Type (*)(ValuePtr)> {
+  static Type unwrap(ValuePtr value) { return value->getType(); }
 
 public:
   using reference = Type;
@@ -536,7 +536,8 @@ public:
 
   /// Initializes the type iterator to the specified value iterator.
   ValueTypeIterator(ValueIteratorT it)
-      : llvm::mapped_iterator<ValueIteratorT, Type (*)(Value)>(it, &unwrap) {}
+      : llvm::mapped_iterator<ValueIteratorT, Type (*)(ValuePtr)>(it, &unwrap) {
+  }
 };
 
 //===----------------------------------------------------------------------===//
@@ -545,7 +546,7 @@ public:
 /// This class implements the operand iterators for the Operation class.
 class OperandRange final
     : public detail::indexed_accessor_range_base<OperandRange, OpOperand *,
-                                                 Value, Value, Value> {
+                                                 ValuePtr, ValuePtr, ValuePtr> {
 public:
   using RangeBaseT::RangeBaseT;
   OperandRange(Operation *op);
@@ -560,7 +561,7 @@ private:
     return object + index;
   }
   /// See `detail::indexed_accessor_range_base` for details.
-  static Value dereference_iterator(OpOperand *object, ptrdiff_t index) {
+  static ValuePtr dereference_iterator(OpOperand *object, ptrdiff_t index) {
     return object[index].get();
   }
 
@@ -573,8 +574,8 @@ private:
 
 /// This class implements the result iterators for the Operation class.
 class ResultRange final
-    : public detail::indexed_accessor_range_base<ResultRange, OpResult *, Value,
-                                                 Value, Value> {
+    : public detail::indexed_accessor_range_base<ResultRange, OpResultPtr,
+                                                 ValuePtr, ValuePtr, ValuePtr> {
 public:
   using RangeBaseT::RangeBaseT;
   ResultRange(Operation *op);
@@ -585,12 +586,12 @@ public:
 
 private:
   /// See `detail::indexed_accessor_range_base` for details.
-  static OpResult *offset_base(OpResult *object, ptrdiff_t index) {
+  static OpResultPtr offset_base(OpResultPtr object, ptrdiff_t index) {
     return object + index;
   }
   /// See `detail::indexed_accessor_range_base` for details.
-  static Value dereference_iterator(OpResult *object, ptrdiff_t index) {
-    return object[index];
+  static ValuePtr dereference_iterator(OpResultPtr object, ptrdiff_t index) {
+    return &object[index];
   }
 
   /// Allow access to `offset_base` and `dereference_iterator`.
@@ -607,24 +608,25 @@ private:
 /// parameter.
 class ValueRange final
     : public detail::indexed_accessor_range_base<
-          ValueRange, PointerUnion<const Value *, OpOperand *, OpResult *>,
-          Value, Value, Value> {
+          ValueRange, PointerUnion<ValuePtr const *, OpOperand *, OpResultPtr>,
+          ValuePtr, ValuePtr, ValuePtr> {
 public:
   using RangeBaseT::RangeBaseT;
 
   template <typename Arg,
             typename = typename std::enable_if_t<
-                std::is_constructible<ArrayRef<Value>, Arg>::value &&
-                !std::is_convertible<Arg, Value>::value>>
-  ValueRange(Arg &&arg) : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
-  ValueRange(const Value &value) : ValueRange(&value, /*count=*/1) {}
-  ValueRange(const std::initializer_list<Value> &values)
-      : ValueRange(ArrayRef<Value>(values)) {}
+                std::is_constructible<ArrayRef<ValuePtr>, Arg>::value &&
+                !std::is_convertible<Arg, ValuePtr>::value>>
+  ValueRange(Arg &&arg)
+      : ValueRange(ArrayRef<ValuePtr>(std::forward<Arg>(arg))) {}
+  ValueRange(ValuePtr const &value) : ValueRange(&value, /*count=*/1) {}
+  ValueRange(const std::initializer_list<ValuePtr> &values)
+      : ValueRange(ArrayRef<ValuePtr>(values)) {}
   ValueRange(iterator_range<OperandRange::iterator> values)
       : ValueRange(OperandRange(values)) {}
   ValueRange(iterator_range<ResultRange::iterator> values)
       : ValueRange(ResultRange(values)) {}
-  ValueRange(ArrayRef<Value> values = llvm::None);
+  ValueRange(ArrayRef<ValuePtr> values = llvm::None);
   ValueRange(OperandRange values);
   ValueRange(ResultRange values);
 
@@ -635,12 +637,12 @@ public:
 private:
   /// The type representing the owner of this range. This is either a list of
   /// values, operands, or results.
-  using OwnerT = PointerUnion<const Value *, OpOperand *, OpResult *>;
+  using OwnerT = PointerUnion<ValuePtr const *, OpOperand *, OpResultPtr>;
 
   /// See `detail::indexed_accessor_range_base` for details.
   static OwnerT offset_base(const OwnerT &owner, ptrdiff_t index);
   /// See `detail::indexed_accessor_range_base` for details.
-  static Value dereference_iterator(const OwnerT &owner, ptrdiff_t index);
+  static ValuePtr dereference_iterator(const OwnerT &owner, ptrdiff_t index);
 
   /// Allow access to `offset_base` and `dereference_iterator`.
   friend RangeBaseT;

mlir/include/mlir/IR/TypeUtilities.h

@@ -33,6 +33,7 @@ Type getElementTypeOrSelf(Type type);
 /// Return the element type or return the type itself.
 Type getElementTypeOrSelf(Attribute attr);
 Type getElementTypeOrSelf(ValuePtr val);
+Type getElementTypeOrSelf(ValueRef val);
 
 /// Get the types within a nested Tuple. A helper for the class method that
 /// handles storage concerns, which is tricky to do in tablegen.

mlir/include/mlir/IR/UseDefLists.h

@@ -21,7 +21,6 @@ namespace mlir {
 
 class IROperand;
 class Operation;
-class Value;
 template <typename OperandType> class ValueUseIterator;
 template <typename OperandType> class ValueUserIterator;
 
@@ -168,22 +167,6 @@ private:
   }
 };
 
-/// A reference to a value, suitable for use as an operand of an operation.
-class OpOperand : public IROperand {
-public:
-  OpOperand(Operation *owner) : IROperand(owner) {}
-  OpOperand(Operation *owner, Value value);
-
-  /// Return the current value being used by this operand.
-  Value get();
-
-  /// Set the current value being used by this operand.
-  void set(Value newValue);
-
-  /// Return which operand this is in the operand list of the User.
-  unsigned getOperandNumber();
-};
-
 /// A reference to a value, suitable for use as an operand of an operation,
 /// operation, etc.  IRValueTy is the root type to use for values this tracks,
 /// and SSAUserTy is the type that will contain operands.

mlir/include/mlir/IR/Value.h

@@ -25,101 +25,40 @@ class OpResult;
 class Region;
 class Value;
 
-namespace detail {
-/// The internal implementation of a Value.
-class ValueImpl : public IRObjectWithUseList {
-protected:
-  /// This enumerates all of the SSA value kinds.
-  enum class Kind {
-    BlockArgument,
-    OpResult,
-  };
+/// Using directives that simplify the transition of Value to being value typed.
+using BlockArgumentPtr = BlockArgument *;
+using OpResultPtr = OpResult *;
+using ValueRef = Value &;
+using ValuePtr = Value *;
 
-  ValueImpl(Kind kind, Type type) : typeAndKind(type, kind) {}
+/// Operands contain a Value.
+using OpOperand = IROperandImpl<Value>;
 
-private:
-  /// The type of the value and its kind.
-  llvm::PointerIntPair<Type, 1, Kind> typeAndKind;
-
-  /// Allow access to 'typeAndKind'.
-  friend Value;
-};
-
-/// The internal implementation of a BlockArgument.
-class BlockArgumentImpl : public ValueImpl {
-  BlockArgumentImpl(Type type, Block *owner)
-      : ValueImpl(Kind::BlockArgument, type), owner(owner) {}
-
-  /// The owner of this argument.
-  Block *owner;
-
-  /// Allow access to owner and constructor.
-  friend BlockArgument;
-};
-
-class OpResultImpl : public ValueImpl {
-  OpResultImpl(Type type, Operation *owner)
-      : ValueImpl(Kind::OpResult, type), owner(owner) {}
-
-  /// The owner of this result.
-  Operation *owner;
-
-  /// Allow access to owner and the constructor.
-  friend OpResult;
-};
-} // end namespace detail
-
-/// This class represents an instance of an SSA value in the MLIR system,
-/// representing a computable value that has a type and a set of users. An SSA
-/// value is either a BlockArgument or the result of an operation. Note: This
-/// class has value-type semantics and is just a simple wrapper around a
-/// ValueImpl that is either owner by a block(in the case of a BlockArgument) or
-/// an Operation(in the case of an OpResult).
+/// This is the common base class for all SSA values in the MLIR system,
+/// representing a computable value that has a type and a set of users.
 ///
-class Value {
+class Value : public IRObjectWithUseList {
 public:
   /// This enumerates all of the SSA value kinds in the MLIR system.
   enum class Kind {
-    BlockArgument,
-    OpResult,
+    BlockArgument, // block argument
+    OpResult,      // operation result
   };
 
-  Value(std::nullptr_t) : impl(nullptr) {}
-  Value(detail::ValueImpl *impl = nullptr) : impl(impl) {}
-  Value(const Value &) = default;
-  Value &operator=(const Value &) = default;
   ~Value() {}
 
-  template <typename U> bool isa() const {
-    assert(impl && "isa<> used on a null type.");
-    return U::classof(*this);
+  template <typename U> bool isa() const { return U::classof(this); }
+  template <typename U> U *dyn_cast() const {
+    return isa<U>() ? (U *)this : nullptr;
   }
-  template <typename U> U dyn_cast() const {
-    return isa<U>() ? U(impl) : U(nullptr);
-  }
-  template <typename U> U dyn_cast_or_null() const {
-    return (impl && isa<U>()) ? U(impl) : U(nullptr);
-  }
-  template <typename U> U cast() const {
+  template <typename U> U *cast() const {
     assert(isa<U>());
-    return U(impl);
+    return (U *)this;
   }
 
-  /// Temporary methods to enable transition of Value to being used as a
-  /// value-type.
-  /// TODO(riverriddle) Remove these when all usages have been removed.
-  Value operator*() const { return *this; }
-  Value *operator->() const { return (Value *)this; }
+  Kind getKind() const { return typeAndKind.getInt(); }
 
-  operator bool() const { return impl; }
-  bool operator==(const Value &other) const { return impl == other.impl; }
-  bool operator!=(const Value &other) const { return !(*this == other); }
-
-  /// Return the kind of this value.
-  Kind getKind() const { return (Kind)impl->typeAndKind.getInt(); }
-
-  /// Return the type of this value.
-  Type getType() const { return impl->typeAndKind.getPointer(); }
+  Type getType() const { return typeAndKind.getPointer(); }
 
   /// Utility to get the associated MLIRContext that this value is defined in.
   MLIRContext *getContext() const { return getType().getContext(); }
@@ -130,18 +69,18 @@ public:
   /// completely invalid IR very easily.  It is strongly recommended that you
   /// recreate IR objects with the right types instead of mutating them in
   /// place.
-  void setType(Type newType) { impl->typeAndKind.setPointer(newType); }
+  void setType(Type newType) { typeAndKind.setPointer(newType); }
 
   /// 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 {
-    impl->replaceAllUsesWith(newValue.impl);
+  void replaceAllUsesWith(ValuePtr newValue) {
+    IRObjectWithUseList::replaceAllUsesWith(newValue);
   }
 
   /// If this value is the result of an operation, return the operation that
   /// defines it.
-  Operation *getDefiningOp() const;
+  Operation *getDefiningOp();
 
   /// If this value is the result of an operation, use it as a location,
   /// otherwise return an unknown location.
@@ -159,51 +98,24 @@ public:
   /// Returns a range of all uses, which is useful for iterating over all uses.
   inline use_range getUses();
 
-  using user_iterator = ValueUserIterator<IROperand>;
-  using user_range = iterator_range<user_iterator>;
-
-  user_iterator user_begin() const { return impl->user_begin(); }
-  user_iterator user_end() const { return impl->user_end(); }
-
-  /// Returns a range of all users.
-  user_range getUsers() const { return impl->getUsers(); }
-
-  /// Returns true if this value has no uses.
-  bool use_empty() const { return impl->use_empty(); }
-
-  /// Returns true if this value has exactly one use.
-  bool hasOneUse() const { return impl->hasOneUse(); }
-
-  /// Drop all uses of this object from their respective owners.
-  void dropAllUses() const { impl->dropAllUses(); }
-
   void print(raw_ostream &os);
   void dump();
 
-  /// Methods for supporting PointerLikeTypeTraits.
-  void *getAsOpaquePointer() const { return static_cast<void *>(impl); }
-  static Value getFromOpaquePointer(const void *pointer) {
-    return reinterpret_cast<detail::ValueImpl *>(const_cast<void *>(pointer));
-  }
-
-  friend ::llvm::hash_code hash_value(Value arg);
-
 protected:
-  /// The internal implementation of this value.
-  mutable detail::ValueImpl *impl;
+  Value(Kind kind, Type type) : typeAndKind(type, kind) {}
 
-  /// Allow access to 'impl'.
-  friend OpOperand;
+private:
+  llvm::PointerIntPair<Type, 1, Kind> typeAndKind;
 };
 
-inline raw_ostream &operator<<(raw_ostream &os, Value value) {
+inline raw_ostream &operator<<(raw_ostream &os, ValueRef value) {
   value.print(os);
   return os;
 }
 
 // Utility functions for iterating through Value uses.
 inline auto Value::use_begin() -> use_iterator {
-  return use_iterator((OpOperand *)impl->getFirstUse());
+  return use_iterator((OpOperand *)getFirstUse());
 }
 
 inline auto Value::use_end() -> use_iterator { return use_iterator(nullptr); }
@@ -215,154 +127,47 @@ inline auto Value::getUses() -> iterator_range<use_iterator> {
 /// Block arguments are values.
 class BlockArgument : public Value {
 public:
-  using Value::Value;
-
-  /// Temporary methods to enable transition of Value to being used as a
-  /// value-type.
-  /// TODO(riverriddle) Remove this when all usages have been removed.
-  BlockArgument *operator->() { return this; }
-
-  static bool classof(Value value) {
-    return value.getKind() == Kind::BlockArgument;
+  static bool classof(const Value *value) {
+    return const_cast<Value *>(value)->getKind() == Kind::BlockArgument;
   }
 
-  /// Returns the block that owns this argument.
-  Block *getOwner() const { return getImpl()->owner; }
+  Block *getOwner() { return owner; }
 
   /// Returns the number of this argument.
-  unsigned getArgNumber() const;
+  unsigned getArgNumber();
 
 private:
-  /// Allocate a new argument with the given type and owner.
-  static BlockArgument create(Type type, Block *owner) {
-    return new detail::BlockArgumentImpl(type, owner);
-  }
+  friend class Block; // For access to private constructor.
+  BlockArgument(Type type, Block *owner)
+      : Value(Value::Kind::BlockArgument, type), owner(owner) {}
 
-  /// Destroy and deallocate this argument.
-  void destroy() { delete getImpl(); }
-
-  /// Get a raw pointer to the internal implementation.
-  detail::BlockArgumentImpl *getImpl() const {
-    return reinterpret_cast<detail::BlockArgumentImpl *>(impl);
-  }
-
-  /// Allow access to `create` and `destroy`.
-  friend Block;
+  /// The owner of this operand.
+  /// TODO: can encode this more efficiently to avoid the space hit of this
+  /// through bitpacking shenanigans.
+  Block *const owner;
 };
 
 /// This is a value defined by a result of an operation.
 class OpResult : public Value {
 public:
-  using Value::Value;
+  OpResult(Type type, Operation *owner)
+      : Value(Value::Kind::OpResult, type), owner(owner) {}
 
-  /// Temporary methods to enable transition of Value to being used as a
-  /// value-type.
-  /// TODO(riverriddle) Remove these when all usages have been removed.
-  OpResult *operator*() { return this; }
-  OpResult *operator->() { return this; }
+  static bool classof(const Value *value) {
+    return const_cast<Value *>(value)->getKind() == Kind::OpResult;
+  }
 
-  static bool classof(Value value) { return value.getKind() == Kind::OpResult; }
-
-  /// Returns the operation that owns this result.
-  Operation *getOwner() const { return getImpl()->owner; }
+  Operation *getOwner() { return owner; }
 
   /// Returns the number of this result.
-  unsigned getResultNumber() const;
+  unsigned getResultNumber();
 
 private:
-  /// Allocate a new result with the given type and owner.
-  static OpResult create(Type type, Operation *owner) {
-    return new detail::OpResultImpl(type, owner);
-  }
-
-  /// Destroy and deallocate this result.
-  void destroy() { delete getImpl(); }
-
-  /// Get a raw pointer to the internal implementation.
-  detail::OpResultImpl *getImpl() const {
-    return reinterpret_cast<detail::OpResultImpl *>(impl);
-  }
-
-  /// Allow access to `create` and `destroy`.
-  friend Operation;
+  /// The owner of this operand.
+  /// TODO: can encode this more efficiently to avoid the space hit of this
+  /// through bitpacking shenanigans.
+  Operation *const owner;
 };
-
-/// Make Value hashable.
-inline ::llvm::hash_code hash_value(Value arg) {
-  return ::llvm::hash_value(arg.impl);
-}
-
-/// Using directives that simplify the transition of Value to being value typed.
-using BlockArgumentPtr = BlockArgument;
-using OpResultPtr = OpResult;
-using ValueRef = Value;
-using ValuePtr = Value;
-
 } // namespace mlir
 
-namespace llvm {
-
-template <> struct DenseMapInfo<mlir::Value> {
-  static mlir::Value getEmptyKey() {
-    auto pointer = llvm::DenseMapInfo<void *>::getEmptyKey();
-    return mlir::Value(static_cast<mlir::detail::ValueImpl *>(pointer));
-  }
-  static mlir::Value getTombstoneKey() {
-    auto pointer = llvm::DenseMapInfo<void *>::getTombstoneKey();
-    return mlir::Value(static_cast<mlir::detail::ValueImpl *>(pointer));
-  }
-  static unsigned getHashValue(mlir::Value val) {
-    return mlir::hash_value(val);
-  }
-  static bool isEqual(mlir::Value LHS, mlir::Value RHS) { return LHS == RHS; }
-};
-
-/// Allow stealing the low bits of a value.
-template <> struct PointerLikeTypeTraits<mlir::Value> {
-public:
-  static inline void *getAsVoidPointer(mlir::Value I) {
-    return const_cast<void *>(I.getAsOpaquePointer());
-  }
-  static inline mlir::Value getFromVoidPointer(void *P) {
-    return mlir::Value::getFromOpaquePointer(P);
-  }
-  enum {
-    NumLowBitsAvailable =
-        PointerLikeTypeTraits<mlir::detail::ValueImpl *>::NumLowBitsAvailable
-  };
-};
-
-template <> struct DenseMapInfo<mlir::BlockArgument> {
-  static mlir::BlockArgument getEmptyKey() {
-    auto pointer = llvm::DenseMapInfo<void *>::getEmptyKey();
-    return mlir::BlockArgument(static_cast<mlir::detail::ValueImpl *>(pointer));
-  }
-  static mlir::BlockArgument getTombstoneKey() {
-    auto pointer = llvm::DenseMapInfo<void *>::getTombstoneKey();
-    return mlir::BlockArgument(static_cast<mlir::detail::ValueImpl *>(pointer));
-  }
-  static unsigned getHashValue(mlir::BlockArgument val) {
-    return mlir::hash_value(val);
-  }
-  static bool isEqual(mlir::BlockArgument LHS, mlir::BlockArgument RHS) {
-    return LHS == RHS;
-  }
-};
-
-/// Allow stealing the low bits of a value.
-template <> struct PointerLikeTypeTraits<mlir::BlockArgument> {
-public:
-  static inline void *getAsVoidPointer(mlir::Value I) {
-    return const_cast<void *>(I.getAsOpaquePointer());
-  }
-  static inline mlir::BlockArgument getFromVoidPointer(void *P) {
-    return mlir::Value::getFromOpaquePointer(P).cast<mlir::BlockArgument>();
-  }
-  enum {
-    NumLowBitsAvailable =
-        PointerLikeTypeTraits<mlir::detail::ValueImpl *>::NumLowBitsAvailable
-  };
-};
-} // end namespace llvm
-
 #endif

mlir/lib/Analysis/AffineStructures.cpp

@@ -1965,7 +1965,7 @@ void FlatAffineConstraints::addLocalFloorDiv(ArrayRef<int64_t> dividend,
 bool FlatAffineConstraints::findId(ValueRef id, unsigned *pos) const {
   unsigned i = 0;
   for (const auto &mayBeId : ids) {
-    if (mayBeId.hasValue() && mayBeId.getValue() == id) {
+    if (mayBeId.hasValue() && mayBeId.getValue() == &id) {
       *pos = i;
       return true;
     }
@@ -1976,7 +1976,7 @@ bool FlatAffineConstraints::findId(ValueRef id, unsigned *pos) const {
 
 bool FlatAffineConstraints::containsId(ValueRef id) const {
   return llvm::any_of(ids, [&](const Optional<ValuePtr> &mayBeId) {
-    return mayBeId.hasValue() && mayBeId.getValue() == id;
+    return mayBeId.hasValue() && mayBeId.getValue() == &id;
   });
 }
 

mlir/lib/Analysis/Dominance.cpp

@@ -129,7 +129,7 @@ bool DominanceInfo::properlyDominates(ValuePtr a, Operation *b) {
 
   // block arguments properly dominate all operations in their own block, so
   // we use a dominates check here, not a properlyDominates check.
-  return dominates(a.cast<BlockArgument>()->getOwner(), b->getBlock());
+  return dominates(cast<BlockArgument>(a)->getOwner(), b->getBlock());
 }
 
 DominanceInfoNode *DominanceInfo::getNode(Block *a) {

mlir/lib/Analysis/Liveness.cpp

@@ -174,7 +174,7 @@ Liveness::OperationListT Liveness::resolveLiveness(ValuePtr value) const {
   if (Operation *defOp = value->getDefiningOp())
     currentBlock = defOp->getBlock();
   else
-    currentBlock = value.cast<BlockArgument>()->getOwner();
+    currentBlock = cast<BlockArgument>(value)->getOwner();
   toProcess.push_back(currentBlock);
   visited.insert(currentBlock);
 
@@ -272,7 +272,7 @@ void Liveness::print(raw_ostream &os) const {
     if (Operation *defOp = value->getDefiningOp())
       os << "val_" << defOp->getName();
     else {
-      auto blockArg = value.cast<BlockArgument>();
+      auto blockArg = cast<BlockArgument>(value);
       os << "arg" << blockArg->getArgNumber() << "@"
          << blockIds[blockArg->getOwner()];
     }

mlir/lib/Analysis/SliceAnalysis.cpp

@@ -96,7 +96,7 @@ static void getBackwardSliceImpl(Operation *op,
 
   for (auto en : llvm::enumerate(op->getOperands())) {
     auto operand = en.value();
-    if (auto blockArg = operand.dyn_cast<BlockArgument>()) {
+    if (auto blockArg = dyn_cast<BlockArgument>(operand)) {
       if (auto affIv = getForInductionVarOwner(operand)) {
         auto *affOp = affIv.getOperation();
         if (backwardSlice->count(affOp) == 0)

mlir/lib/Dialect/AffineOps/AffineOps.cpp

@@ -107,7 +107,7 @@ static bool isFunctionRegion(Region *region) {
 /// function. A value of index type defined at the top level is always a valid
 /// symbol.
 bool mlir::isTopLevelValue(ValuePtr value) {
-  if (auto arg = value.dyn_cast<BlockArgument>())
+  if (auto arg = dyn_cast<BlockArgument>(value))
     return isFunctionRegion(arg->getOwner()->getParent());
   return isFunctionRegion(value->getDefiningOp()->getParentRegion());
 }
@@ -134,7 +134,7 @@ bool mlir::isValidDim(ValuePtr value) {
     return false;
   }
   // This value has to be a block argument for a FuncOp or an affine.for.
-  auto *parentOp = value.cast<BlockArgument>()->getOwner()->getParentOp();
+  auto *parentOp = cast<BlockArgument>(value)->getOwner()->getParentOp();
   return isa<FuncOp>(parentOp) || isa<AffineForOp>(parentOp);
 }
 
@@ -1571,7 +1571,7 @@ bool mlir::isForInductionVar(ValuePtr val) {
 /// Returns the loop parent of an induction variable. If the provided value is
 /// not an induction variable, then return nullptr.
 AffineForOp mlir::getForInductionVarOwner(ValuePtr val) {
-  auto ivArg = val.dyn_cast<BlockArgument>();
+  auto ivArg = dyn_cast<BlockArgument>(val);
   if (!ivArg || !ivArg->getOwner())
     return AffineForOp();
   auto *containingInst = ivArg->getOwner()->getParent()->getParentOp();

mlir/lib/Dialect/Linalg/Analysis/DependenceAnalysis.cpp

@@ -41,7 +41,7 @@ static StringRef toStringRef(LinalgDependenceGraph::DependenceType dt) {
 }
 
 ValuePtr Aliases::find(ValuePtr v) {
-  if (v.isa<BlockArgument>())
+  if (isa<BlockArgument>(v))
     return v;
 
   auto it = aliases.find(v);
@@ -51,7 +51,7 @@ ValuePtr Aliases::find(ValuePtr v) {
   }
 
   while (true) {
-    if (v.isa<BlockArgument>())
+    if (isa<BlockArgument>(v))
       return v;
     if (auto alloc = dyn_cast_or_null<AllocOp>(v->getDefiningOp())) {
       if (isStrided(alloc.getType()))

mlir/lib/Dialect/LoopOps/LoopOps.cpp

@@ -136,7 +136,7 @@ LogicalResult ForOp::moveOutOfLoop(ArrayRef<Operation *> ops) {
 }
 
 ForOp mlir::loop::getForInductionVarOwner(ValuePtr val) {
-  auto ivArg = val.dyn_cast<BlockArgument>();
+  auto ivArg = dyn_cast<BlockArgument>(val);
   if (!ivArg)
     return ForOp();
   assert(ivArg->getOwner() && "unlinked block argument");

mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp

@@ -509,7 +509,7 @@ void Serializer::printValueIDMap(raw_ostream &os) {
        << "id = " << valueIDPair.second << ' ';
     if (auto *op = val->getDefiningOp()) {
       os << "from op '" << op->getName() << "'";
-    } else if (auto arg = val.dyn_cast<BlockArgument>()) {
+    } else if (auto arg = dyn_cast<BlockArgument>(val)) {
       Block *block = arg->getOwner();
       os << "from argument of block " << block << ' ';
       os << " in op '" << block->getParentOp()->getName() << "'";

mlir/lib/IR/AsmPrinter.cpp

@@ -1612,7 +1612,7 @@ void OperationPrinter::numberValuesInRegion(Region &region) {
 void OperationPrinter::numberValuesInBlock(Block &block) {
   auto setArgNameFn = [&](ValuePtr arg, StringRef name) {
     assert(!valueIDs.count(arg) && "arg numbered multiple times");
-    assert(arg.cast<BlockArgument>()->getOwner() == &block &&
+    assert(cast<BlockArgument>(arg)->getOwner() == &block &&
            "arg not defined in 'block'");
     setValueName(arg, name);
   };
@@ -1658,7 +1658,7 @@ void OperationPrinter::numberValuesInOp(Operation &op) {
     setValueName(result, name);
 
     // Record the result number for groups not anchored at 0.
-    if (int resultNo = result.cast<OpResult>()->getResultNumber())
+    if (int resultNo = cast<OpResult>(result)->getResultNumber())
       resultGroups.push_back(resultNo);
   };
 
@@ -1831,7 +1831,7 @@ void OperationPrinter::printValueIDImpl(ValuePtr value, bool printResultNo,
   // If this is a reference to the result of a multi-result operation or
   // operation, print out the # identifier and make sure to map our lookup
   // to the first result of the operation.
-  if (OpResultPtr result = value.dyn_cast<OpResult>())
+  if (OpResultPtr result = dyn_cast<OpResult>(value))
     getResultIDAndNumber(result, lookupValue, resultNo);
 
   auto it = valueIDs.find(lookupValue);

mlir/lib/IR/Block.cpp

@@ -16,10 +16,10 @@ using namespace mlir;
 //===----------------------------------------------------------------------===//
 
 /// Returns the number of this argument.
-unsigned BlockArgument::getArgNumber() const {
+unsigned BlockArgument::getArgNumber() {
   // Arguments are not stored in place, so we have to find it within the list.
   auto argList = getOwner()->getArguments();
-  return std::distance(argList.begin(), llvm::find(argList, *this));
+  return std::distance(argList.begin(), llvm::find(argList, this));
 }
 
 //===----------------------------------------------------------------------===//
@@ -29,8 +29,7 @@ unsigned BlockArgument::getArgNumber() const {
 Block::~Block() {
   assert(!verifyOpOrder() && "Expected valid operation ordering.");
   clear();
-  for (BlockArgument arg : arguments)
-    arg.destroy();
+  llvm::DeleteContainerPointers(arguments);
 }
 
 Region *Block::getParent() const { return parentValidOpOrderPair.getPointer(); }
@@ -144,7 +143,7 @@ void Block::recomputeOpOrder() {
 //===----------------------------------------------------------------------===//
 
 BlockArgumentPtr Block::addArgument(Type type) {
-  BlockArgument arg = BlockArgument::create(type, this);
+  auto *arg = new BlockArgument(type, this);
   arguments.push_back(arg);
   return arg;
 }
@@ -164,7 +163,7 @@ void Block::eraseArgument(unsigned index, bool updatePredTerms) {
   assert(index < arguments.size());
 
   // Delete the argument.
-  arguments[index].destroy();
+  delete arguments[index];
   arguments.erase(arguments.begin() + index);
 
   // If we aren't updating predecessors, there is nothing left to do.

mlir/lib/IR/Operation.cpp

@@ -68,29 +68,23 @@ OperationName OperationName::getFromOpaquePointer(void *pointer) {
 //===----------------------------------------------------------------------===//
 
 /// Return the result number of this result.
-unsigned OpResult::getResultNumber() const {
-  // Results are not stored in place, so we have to find it within the list.
-  auto resList = getOwner()->getOpResults();
-  return std::distance(resList.begin(), llvm::find(resList, *this));
+unsigned OpResult::getResultNumber() {
+  // Results are always stored consecutively, so use pointer subtraction to
+  // figure out what number this is.
+  return this - &getOwner()->getOpResults()[0];
 }
 
 //===----------------------------------------------------------------------===//
 // OpOperand
 //===----------------------------------------------------------------------===//
 
-OpOperand::OpOperand(Operation *owner, Value value)
-    : IROperand(owner, value.impl) {}
-
-/// Return the current value being used by this operand.
-Value OpOperand::get() { return (detail::ValueImpl *)IROperand::get(); }
-
-/// Set the current value being used by this operand.
-void OpOperand::set(Value newValue) { IROperand::set(newValue.impl); }
-
+// TODO: This namespace is only required because of a bug in GCC<7.0.
+namespace mlir {
 /// Return which operand this is in the operand list.
-unsigned OpOperand::getOperandNumber() {
+template <> unsigned OpOperand::getOperandNumber() {
   return this - &getOwner()->getOpOperands()[0];
 }
+} // end namespace mlir
 
 //===----------------------------------------------------------------------===//
 // BlockOperand
@@ -185,7 +179,7 @@ Operation *Operation::create(Location location, OperationName name,
 
   auto instResults = op->getOpResults();
   for (unsigned i = 0, e = resultTypes.size(); i != e; ++i)
-    new (&instResults[i]) OpResult(OpResult::create(resultTypes[i], op));
+    new (&instResults[i]) OpResult(resultTypes[i], op);
 
   auto opOperands = op->getOpOperands();
 
@@ -262,7 +256,7 @@ Operation::~Operation() {
   getOperandStorage().~OperandStorage();
 
   for (auto &result : getOpResults())
-    result.destroy();
+    result.~OpResult();
 
   // Explicitly run the destructors for the successors.
   for (auto &successor : getBlockOperands())

mlir/lib/IR/OperationSupport.cpp

@@ -155,7 +155,7 @@ ResultRange::ResultRange(Operation *op)
 //===----------------------------------------------------------------------===//
 // ValueRange
 
-ValueRange::ValueRange(ArrayRef<Value> values)
+ValueRange::ValueRange(ArrayRef<ValuePtr> values)
     : ValueRange(values.data(), values.size()) {}
 ValueRange::ValueRange(OperandRange values)
     : ValueRange(values.begin().getBase(), values.size()) {}
@@ -167,18 +167,19 @@ ValueRange::OwnerT ValueRange::offset_base(const OwnerT &owner,
                                            ptrdiff_t index) {
   if (OpOperand *operand = owner.dyn_cast<OpOperand *>())
     return operand + index;
-  if (OpResult *result = owner.dyn_cast<OpResult *>())
+  if (OpResultPtr result = owner.dyn_cast<OpResultPtr>())
     return result + index;
-  return owner.get<const Value *>() + index;
+  return owner.get<ValuePtr const *>() + index;
 }
 /// See `detail::indexed_accessor_range_base` for details.
-Value ValueRange::dereference_iterator(const OwnerT &owner, ptrdiff_t index) {
+ValuePtr ValueRange::dereference_iterator(const OwnerT &owner,
+                                          ptrdiff_t index) {
   // Operands access the held value via 'get'.
   if (OpOperand *operand = owner.dyn_cast<OpOperand *>())
     return operand[index].get();
   // An OpResult is a value, so we can return it directly.
-  if (OpResult *result = owner.dyn_cast<OpResult *>())
-    return result[index];
+  if (OpResultPtr result = owner.dyn_cast<OpResultPtr>())
+    return &result[index];
   // Otherwise, this is a raw value array so just index directly.
-  return owner.get<const Value *>()[index];
+  return owner.get<ValuePtr const *>()[index];
 }

mlir/lib/IR/TypeUtilities.cpp

@@ -28,6 +28,10 @@ Type mlir::getElementTypeOrSelf(ValuePtr val) {
   return getElementTypeOrSelf(val->getType());
 }
 
+Type mlir::getElementTypeOrSelf(ValueRef val) {
+  return getElementTypeOrSelf(val.getType());
+}
+
 Type mlir::getElementTypeOrSelf(Attribute attr) {
   return getElementTypeOrSelf(attr.getType());
 }

mlir/lib/IR/Value.cpp

@@ -13,8 +13,8 @@ using namespace mlir;
 
 /// If this value is the result of an Operation, return the operation that
 /// defines it.
-Operation *Value::getDefiningOp() const {
-  if (auto result = dyn_cast<OpResult>())
+Operation *Value::getDefiningOp() {
+  if (auto *result = dyn_cast<OpResult>())
     return result->getOwner();
   return nullptr;
 }