[Mlir-commits] [mlir] 4dfd1b5 - [mlir] Optimize operand storage such that all operations can have resizable operand lists

River Riddle llvmlistbot at llvm.org
Sun Apr 26 21:37:27 PDT 2020


Author: River Riddle
Date: 2020-04-26T21:34:01-07:00
New Revision: 4dfd1b5fcb7316d5e8393183dd303352ca65d4ac

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

LOG: [mlir] Optimize operand storage such that all operations can have resizable operand lists

This revision refactors the structure of the operand storage such that there is no additional memory cost for resizable operand lists until it is required. This is done by using two different internal representations for the operand storage:
* One using trailing operands
* One using a dynamically allocated std::vector<OpOperand>

This allows for removing the resizable operand list bit, and will free up APIs from needing to workaround non-resizable operand lists.

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

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
    mlir/include/mlir/IR/OpBase.td
    mlir/include/mlir/IR/Operation.h
    mlir/include/mlir/IR/OperationSupport.h
    mlir/include/mlir/TableGen/Operator.h
    mlir/lib/Dialect/Affine/IR/AffineOps.cpp
    mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
    mlir/lib/IR/Operation.cpp
    mlir/lib/IR/OperationSupport.cpp
    mlir/lib/Parser/Parser.cpp
    mlir/lib/TableGen/Operator.cpp
    mlir/lib/Transforms/Utils/Utils.cpp
    mlir/test/mlir-tblgen/op-operand.td
    mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
    mlir/tools/mlir-tblgen/OpFormatGen.cpp
    mlir/unittests/IR/OperationSupportTest.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
index 3dcea90465bd..75aaff9468e9 100644
--- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
+++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
@@ -332,9 +332,7 @@ def AffineIfOp : Affine_Op<"if",
     IntegerSet getIntegerSet();
     void setIntegerSet(IntegerSet newSet);
 
-    /// Sets the integer set with its operands. The size of 'operands' must not
-    /// exceed the current number of operands for this instance, as the operands
-    /// list of AffineIf is not resizable.
+    /// Sets the integer set with its operands.
     void setConditional(IntegerSet set, ValueRange operands);
 
     /// Returns true if an else block exists.

diff  --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td
index 46eaa3b895d4..7679d8b1008e 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -1679,10 +1679,6 @@ class HasParent<string op>
 def FirstAttrDerivedResultType :
   GenInternalOpTrait<"FirstAttrDerivedResultType">;
 
-// Op has a resizable operand list. Auto-generated build and parse functions
-// should construct it as such.
-def ResizableOperandList : GenInternalOpTrait<"ResizableOperandList">;
-
 // TODO(antiagainst): Turn the following into normal traits and generate
 // verification for them.
 

diff  --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index e49be558a5f2..33c75ef97eda 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -34,16 +34,14 @@ class Operation final
   static Operation *create(Location location, OperationName name,
                            ArrayRef<Type> resultTypes, ArrayRef<Value> operands,
                            ArrayRef<NamedAttribute> attributes,
-                           ArrayRef<Block *> successors, unsigned numRegions,
-                           bool resizableOperandList);
+                           ArrayRef<Block *> successors, unsigned numRegions);
 
   /// Overload of create that takes an existing NamedAttributeList to avoid
   /// unnecessarily uniquing a list of attributes.
   static Operation *create(Location location, OperationName name,
                            ArrayRef<Type> resultTypes, ArrayRef<Value> operands,
                            NamedAttributeList attributes,
-                           ArrayRef<Block *> successors, unsigned numRegions,
-                           bool resizableOperandList);
+                           ArrayRef<Block *> successors, unsigned numRegions);
 
   /// Create a new Operation from the fields stored in `state`.
   static Operation *create(const OperationState &state);
@@ -53,8 +51,7 @@ class Operation final
                            ArrayRef<Type> resultTypes, ArrayRef<Value> operands,
                            NamedAttributeList attributes,
                            ArrayRef<Block *> successors = {},
-                           RegionRange regions = {},
-                           bool resizableOperandList = false);
+                           RegionRange regions = {});
 
   /// The name of an operation is the key identifier for it.
   OperationName getName() { return name; }
@@ -204,13 +201,8 @@ class Operation final
   // Operands
   //===--------------------------------------------------------------------===//
 
-  /// Returns if the operation has a resizable operation list, i.e. operands can
-  /// be added.
-  bool hasResizableOperandsList() { return getOperandStorage().isResizable(); }
-
   /// Replace the current operands of this operation with the ones provided in
-  /// 'operands'. If the operands list is not resizable, the size of 'operands'
-  /// must be less than or equal to the current number of operands.
+  /// 'operands'.
   void setOperands(ValueRange operands);
 
   unsigned getNumOperands() { return getOperandStorage().size(); }

diff  --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 6d31660cfbd1..852404f4e071 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -273,8 +273,6 @@ struct OperationState {
   SmallVector<Block *, 1> successors;
   /// Regions that the op will hold.
   SmallVector<std::unique_ptr<Region>, 1> regions;
-  /// If the operation has a resizable operand list.
-  bool resizableOperandList = false;
 
 public:
   OperationState(Location location, StringRef name);
@@ -284,8 +282,7 @@ struct OperationState {
   OperationState(Location location, StringRef name, ValueRange operands,
                  ArrayRef<Type> types, ArrayRef<NamedAttribute> attributes,
                  ArrayRef<Block *> successors = {},
-                 MutableArrayRef<std::unique_ptr<Region>> regions = {},
-                 bool resizableOperandList = false);
+                 MutableArrayRef<std::unique_ptr<Region>> regions = {});
 
   void addOperands(ValueRange newOperands);
 
@@ -330,11 +327,6 @@ struct OperationState {
   /// region is null, a new empty region will be attached to the Operation.
   void addRegion(std::unique_ptr<Region> &&region);
 
-  /// Sets the operand list of the operation as resizable.
-  void setOperandListToResizable(bool isResizable = true) {
-    resizableOperandList = isResizable;
-  }
-
   /// Get the context held by this operation state.
   MLIRContext *getContext() { return location->getContext(); }
 };
@@ -344,113 +336,96 @@ struct OperationState {
 //===----------------------------------------------------------------------===//
 
 namespace detail {
-/// A utility class holding the information necessary to dynamically resize
-/// operands.
-struct ResizableStorage {
-  ResizableStorage() : firstOp(nullptr), capacity(0) {}
-  ResizableStorage(ResizableStorage &&other)
-      : firstOp(other.firstOp), capacity(other.capacity) {
-    other.firstOp = nullptr;
+/// This class contains the information for a trailing operand storage.
+struct TrailingOperandStorage final
+    : public llvm::TrailingObjects<TrailingOperandStorage, OpOperand> {
+  ~TrailingOperandStorage() {
+    for (auto &operand : getOperands())
+      operand.~OpOperand();
   }
-  ~ResizableStorage() { cleanupStorage(); }
 
-  /// Cleanup any allocated storage.
-  void cleanupStorage() { free(firstOp); }
-
-  /// Sets the storage pointer to a new dynamically allocated block.
-  void setDynamicStorage(OpOperand *opBegin) {
-    /// Cleanup the old storage if necessary.
-    cleanupStorage();
-    firstOp = opBegin;
+  /// Return the operands held by this storage.
+  MutableArrayRef<OpOperand> getOperands() {
+    return {getTrailingObjects<OpOperand>(), numOperands};
   }
 
-  /// A pointer to the first operand element in a dynamically allocated block.
-  OpOperand *firstOp;
-
-  /// The maximum number of operands that can be currently held by the storage.
-  unsigned capacity;
+  /// The number of operands within the storage.
+  unsigned numOperands;
+  /// The total capacity number of operands that the storage can hold.
+  unsigned capacity : 31;
+  /// We reserve a range of bits for use by the operand storage.
+  unsigned reserved : 1;
 };
 
 /// This class handles the management of operation operands. Operands are
-/// stored similarly to the elements of a SmallVector except for two key
-/// 
diff erences. The first is the inline storage, which is a trailing objects
-/// array. The second is that being able to dynamically resize the operand list
-/// is optional.
+/// stored either in a trailing array, or a dynamically resizable vector.
 class OperandStorage final
     : private llvm::TrailingObjects<OperandStorage, OpOperand> {
 public:
-  OperandStorage(unsigned numOperands, bool resizable)
-      : numOperands(numOperands), resizable(resizable),
-        storageIsDynamic(false) {}
-
-  ~OperandStorage() {
-    // Manually destruct the operands.
-    for (auto &operand : getOperands())
-      operand.~OpOperand();
-
-    // If the storage is currently in a dynamic allocation, then destruct the
-    // resizable storage.
-    if (storageIsDynamic)
-      getResizableStorage().~ResizableStorage();
-  }
+  OperandStorage(Operation *owner, ValueRange values);
+  ~OperandStorage();
 
   /// Replace the operands contained in the storage with the ones provided in
-  /// 'operands'.
-  void setOperands(Operation *owner, ValueRange operands);
+  /// 'values'.
+  void setOperands(Operation *owner, ValueRange values);
 
   /// Erase an operand held by the storage.
   void eraseOperand(unsigned index);
 
   /// Get the operation operands held by the storage.
   MutableArrayRef<OpOperand> getOperands() {
-    return {getRawOperands(), size()};
+    return getStorage().getOperands();
   }
 
   /// Return the number of operands held in the storage.
-  unsigned size() const { return numOperands; }
+  unsigned size() { return getStorage().numOperands; }
 
   /// Returns the additional size necessary for allocating this object.
-  static size_t additionalAllocSize(unsigned numOperands, bool resizable) {
-    // The trailing storage must be able to hold enough space for the number of
-    // operands, or at least the resizable storage if necessary.
-    return std::max(additionalSizeToAlloc<OpOperand>(numOperands),
-                    sizeof(ResizableStorage));
+  static size_t additionalAllocSize(unsigned numOperands) {
+    return additionalSizeToAlloc<OpOperand>(numOperands);
   }
 
-  /// Returns if this storage is resizable.
-  bool isResizable() const { return resizable; }
-
 private:
-  /// Clear the storage and destroy the current operands held by the storage.
-  void clear() { numOperands = 0; }
-
-  /// Returns the current pointer for the raw operands array.
-  OpOperand *getRawOperands() {
-    return storageIsDynamic ? getResizableStorage().firstOp
-                            : getTrailingObjects<OpOperand>();
+  enum : uint64_t {
+    /// The bit used to mark the storage as dynamic.
+    DynamicStorageBit = 1ull << 63ull
+  };
+
+  /// Resize the storage to the given size. Returns the array containing the new
+  /// operands.
+  MutableArrayRef<OpOperand> resize(Operation *owner, unsigned newSize);
+
+  /// Returns the current internal storage instance.
+  TrailingOperandStorage &getStorage() {
+    return LLVM_UNLIKELY(isDynamicStorage()) ? getDynamicStorage()
+                                             : getInlineStorage();
   }
 
-  /// Returns the resizable operand utility class.
-  ResizableStorage &getResizableStorage() {
-    // We represent the resizable storage in the same location as the first
-    // operand.
-    assert(storageIsDynamic);
-    return *reinterpret_cast<ResizableStorage *>(
-        getTrailingObjects<OpOperand>());
+  /// Returns the storage container if the storage is inline.
+  TrailingOperandStorage &getInlineStorage() {
+    assert(!isDynamicStorage() && "expected storage to be inline");
+    static_assert(sizeof(TrailingOperandStorage) == sizeof(uint64_t),
+                  "inline storage representation must match the opaque "
+                  "representation");
+    return inlineStorage;
   }
 
-  /// Grow the internal resizable operand storage.
-  void grow(ResizableStorage &resizeUtil, size_t minSize);
-
-  /// The current number of operands, and the current max operand capacity.
-  unsigned numOperands : 30;
+  /// Returns the storage container if this storage is dynamic.
+  TrailingOperandStorage &getDynamicStorage() {
+    assert(isDynamicStorage() && "expected dynamic storage");
+    uint64_t maskedRepresentation = representation & ~DynamicStorageBit;
+    return *reinterpret_cast<TrailingOperandStorage *>(maskedRepresentation);
+  }
 
-  /// Whether this storage is resizable or not.
-  bool resizable : 1;
+  /// Returns true if the storage is currently dynamic.
+  bool isDynamicStorage() const { return representation & DynamicStorageBit; }
 
-  /// If the storage is resizable, this indicates if the operands array is
-  /// currently in the resizable storage or the trailing array.
-  bool storageIsDynamic : 1;
+  /// The current representation of the storage. This is either a
+  /// InlineOperandStorage, or a pointer to a InlineOperandStorage.
+  union {
+    TrailingOperandStorage inlineStorage;
+    uint64_t representation;
+  };
 
   /// This stuff is used by the TrailingObjects template.
   friend llvm::TrailingObjects<OperandStorage, OpOperand>;

diff  --git a/mlir/include/mlir/TableGen/Operator.h b/mlir/include/mlir/TableGen/Operator.h
index 91f10bd8e29c..e65bc55a84f5 100644
--- a/mlir/include/mlir/TableGen/Operator.h
+++ b/mlir/include/mlir/TableGen/Operator.h
@@ -165,9 +165,6 @@ class Operator {
   // requiring the raw MLIR trait here.
   const OpTrait *getTrait(llvm::StringRef trait) const;
 
-  // Returns "true" if Op has a ResizableOperandList trait.
-  bool hasResizableOperandList() const;
-
   // Regions.
   using const_region_iterator = const NamedRegion *;
   const_region_iterator region_begin() const;

diff  --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index ca35bafcd2c4..5f490b0d3721 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1085,9 +1085,6 @@ void AffineForOp::build(Builder *builder, OperationState &result,
   body->addArgument(IndexType::get(builder->getContext()));
   bodyRegion->push_back(body);
   ensureTerminator(*bodyRegion, *builder, result.location);
-
-  // Set the operands list as resizable so that we can freely modify the bounds.
-  result.setOperandListToResizable();
 }
 
 void AffineForOp::build(Builder *builder, OperationState &result, int64_t lb,
@@ -1259,12 +1256,7 @@ static ParseResult parseAffineForOp(OpAsmParser &parser,
   AffineForOp::ensureTerminator(*body, builder, result.location);
 
   // Parse the optional attribute list.
-  if (parser.parseOptionalAttrDict(result.attributes))
-    return failure();
-
-  // Set the operands list as resizable so that we can freely modify the bounds.
-  result.setOperandListToResizable();
-  return success();
+  return parser.parseOptionalAttrDict(result.attributes);
 }
 
 static void printBound(AffineMapAttr boundMap,

diff  --git a/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp b/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
index 90f098662086..7b58d3a5ca0d 100644
--- a/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
@@ -1055,8 +1055,7 @@ static Operation *vectorizeOneOperation(Operation *opInst,
   OpBuilder b(opInst);
   OperationState newOp(opInst->getLoc(), opInst->getName().getStringRef(),
                        vectorOperands, vectorTypes, opInst->getAttrs(),
-                       /*successors=*/{},
-                       /*regions=*/{}, opInst->hasResizableOperandsList());
+                       /*successors=*/{}, /*regions=*/{});
   return b.createOperation(newOp);
 }
 

diff  --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index a2293af0ffdc..97ff4cd72a22 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -62,19 +62,17 @@ Operation *Operation::create(Location location, OperationName name,
                              ArrayRef<Type> resultTypes,
                              ArrayRef<Value> operands,
                              ArrayRef<NamedAttribute> attributes,
-                             ArrayRef<Block *> successors, unsigned numRegions,
-                             bool resizableOperandList) {
+                             ArrayRef<Block *> successors,
+                             unsigned numRegions) {
   return create(location, name, resultTypes, operands,
-                NamedAttributeList(attributes), successors, numRegions,
-                resizableOperandList);
+                NamedAttributeList(attributes), successors, numRegions);
 }
 
 /// Create a new Operation from operation state.
 Operation *Operation::create(const OperationState &state) {
   return Operation::create(state.location, state.name, state.types,
                            state.operands, NamedAttributeList(state.attributes),
-                           state.successors, state.regions,
-                           state.resizableOperandList);
+                           state.successors, state.regions);
 }
 
 /// Create a new Operation with the specific fields.
@@ -82,11 +80,11 @@ Operation *Operation::create(Location location, OperationName name,
                              ArrayRef<Type> resultTypes,
                              ArrayRef<Value> operands,
                              NamedAttributeList attributes,
-                             ArrayRef<Block *> successors, RegionRange regions,
-                             bool resizableOperandList) {
+                             ArrayRef<Block *> successors,
+                             RegionRange regions) {
   unsigned numRegions = regions.size();
   Operation *op = create(location, name, resultTypes, operands, attributes,
-                         successors, numRegions, resizableOperandList);
+                         successors, numRegions);
   for (unsigned i = 0; i < numRegions; ++i)
     if (regions[i])
       op->getRegion(i).takeBody(*regions[i]);
@@ -99,8 +97,8 @@ Operation *Operation::create(Location location, OperationName name,
                              ArrayRef<Type> resultTypes,
                              ArrayRef<Value> operands,
                              NamedAttributeList attributes,
-                             ArrayRef<Block *> successors, unsigned numRegions,
-                             bool resizableOperandList) {
+                             ArrayRef<Block *> successors,
+                             unsigned numRegions) {
   // We only need to allocate additional memory for a subset of results.
   unsigned numTrailingResults = OpResult::getNumTrailing(resultTypes.size());
   unsigned numInlineResults = OpResult::getNumInline(resultTypes.size());
@@ -113,9 +111,9 @@ Operation *Operation::create(Location location, OperationName name,
                        BlockOperand, Region, detail::OperandStorage>(
           numInlineResults, numTrailingResults, numSuccessors, numRegions,
           /*detail::OperandStorage*/ 1);
-  byteSize += llvm::alignTo(detail::OperandStorage::additionalAllocSize(
-                                numOperands, resizableOperandList),
-                            alignof(Operation));
+  byteSize +=
+      llvm::alignTo(detail::OperandStorage::additionalAllocSize(numOperands),
+                    alignof(Operation));
   void *rawMem = malloc(byteSize);
 
   // Create the new Operation.
@@ -136,11 +134,7 @@ Operation *Operation::create(Location location, OperationName name,
     new (&op->getRegion(i)) Region(op);
 
   // Initialize the operands.
-  new (&op->getOperandStorage())
-      detail::OperandStorage(numOperands, resizableOperandList);
-  auto opOperands = op->getOpOperands();
-  for (unsigned i = 0; i != numOperands; ++i)
-    new (&opOperands[i]) OpOperand(op, operands[i]);
+  new (&op->getOperandStorage()) detail::OperandStorage(op, operands);
 
   // Initialize the successors.
   auto blockOperands = op->getBlockOperands();
@@ -229,8 +223,7 @@ void Operation::replaceUsesOfWith(Value from, Value to) {
 }
 
 /// Replace the current operands of this operation with the ones provided in
-/// 'operands'. If the operands list is not resizable, the size of 'operands'
-/// must be less than or equal to the current number of operands.
+/// 'operands'.
 void Operation::setOperands(ValueRange operands) {
   getOperandStorage().setOperands(this, operands);
 }
@@ -558,8 +551,7 @@ Operation *Operation::cloneWithoutRegions(BlockAndValueMapping &mapper) {
 
   // Create the new operation.
   auto *newOp = Operation::create(getLoc(), getName(), getResultTypes(),
-                                  operands, attrs, successors, getNumRegions(),
-                                  hasResizableOperandsList());
+                                  operands, attrs, successors, getNumRegions());
 
   // Remember the mapping of any results.
   for (unsigned i = 0, e = getNumResults(); i != e; ++i)

diff  --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index d4960dfab2c4..5f698b1bdc53 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -30,8 +30,7 @@ OperationState::OperationState(Location location, StringRef name,
                                ValueRange operands, ArrayRef<Type> types,
                                ArrayRef<NamedAttribute> attributes,
                                ArrayRef<Block *> successors,
-                               MutableArrayRef<std::unique_ptr<Region>> regions,
-                               bool resizableOperandList)
+                               MutableArrayRef<std::unique_ptr<Region>> regions)
     : location(location), name(name, location->getContext()),
       operands(operands.begin(), operands.end()),
       types(types.begin(), types.end()),
@@ -62,86 +61,106 @@ void OperationState::addRegion(std::unique_ptr<Region> &&region) {
 // OperandStorage
 //===----------------------------------------------------------------------===//
 
+detail::OperandStorage::OperandStorage(Operation *owner, ValueRange values)
+    : representation(0) {
+  auto &inlineStorage = getInlineStorage();
+  inlineStorage.numOperands = inlineStorage.capacity = values.size();
+  auto *operandPtrBegin = getTrailingObjects<OpOperand>();
+  for (unsigned i = 0, e = inlineStorage.numOperands; i < e; ++i)
+    new (&operandPtrBegin[i]) OpOperand(owner, values[i]);
+}
+
+detail::OperandStorage::~OperandStorage() {
+  // Destruct the current storage container.
+  if (isDynamicStorage()) {
+    TrailingOperandStorage &storage = getDynamicStorage();
+    storage.~TrailingOperandStorage();
+    free(&storage);
+  } else {
+    getInlineStorage().~TrailingOperandStorage();
+  }
+}
+
 /// Replace the operands contained in the storage with the ones provided in
-/// 'operands'.
-void detail::OperandStorage::setOperands(Operation *owner,
-                                         ValueRange operands) {
+/// 'values'.
+void detail::OperandStorage::setOperands(Operation *owner, ValueRange values) {
+  MutableArrayRef<OpOperand> storageOperands = resize(owner, values.size());
+  for (unsigned i = 0, e = values.size(); i != e; ++i)
+    storageOperands[i].set(values[i]);
+}
+
+/// Resize the storage to the given size. Returns the array containing the new
+/// operands.
+MutableArrayRef<OpOperand> detail::OperandStorage::resize(Operation *owner,
+                                                          unsigned newSize) {
+  TrailingOperandStorage &storage = getStorage();
+
   // If the number of operands is less than or equal to the current amount, we
   // can just update in place.
-  if (operands.size() <= numOperands) {
-    auto opOperands = getOperands();
-
-    // If the number of new operands is less than the current count, then remove
-    // any extra operands.
-    for (unsigned i = operands.size(); i != numOperands; ++i)
-      opOperands[i].~OpOperand();
-
-    // Set the operands in place.
-    numOperands = operands.size();
-    for (unsigned i = 0; i != numOperands; ++i)
-      opOperands[i].set(operands[i]);
-    return;
+  unsigned &numOperands = storage.numOperands;
+  MutableArrayRef<OpOperand> operands = storage.getOperands();
+  if (newSize <= numOperands) {
+    // If the number of new size is less than the current, remove any extra
+    // operands.
+    for (unsigned i = newSize; i != numOperands; ++i)
+      operands[i].~OpOperand();
+    numOperands = newSize;
+    return operands.take_front(newSize);
   }
 
-  // Otherwise, we need to be resizable.
-  assert(resizable && "Only resizable operations may add operands");
-
-  // If the storage isn't already dynamic, we need to allocate a new buffer for
-  // it.
-  OpOperand *opBegin = nullptr;
-  if (!storageIsDynamic) {
-    // Grow a new storage first to avoid overwriting the existing operands.
-    ResizableStorage newStorage;
-    grow(newStorage, operands.size());
-    opBegin = newStorage.firstOp;
-    storageIsDynamic = true;
-    new (&getResizableStorage()) ResizableStorage(std::move(newStorage));
-  } else {
-    // Otherwise, grow the existing storage if necessary.
-    auto &resizeUtil = getResizableStorage();
-    if (resizeUtil.capacity < operands.size())
-      grow(resizeUtil, operands.size());
-    opBegin = resizeUtil.firstOp;
+  // If the new size is within the original inline capacity, grow inplace.
+  if (newSize <= storage.capacity) {
+    OpOperand *opBegin = operands.data();
+    for (unsigned e = newSize; numOperands != e; ++numOperands)
+      new (&opBegin[numOperands]) OpOperand(owner);
+    return MutableArrayRef<OpOperand>(opBegin, newSize);
   }
 
-  // Set the operands.
-  for (unsigned i = 0; i != numOperands; ++i)
-    opBegin[i].set(operands[i]);
-  for (unsigned e = operands.size(); numOperands != e; ++numOperands)
-    new (&opBegin[numOperands]) OpOperand(owner, operands[numOperands]);
+  // Otherwise, we need to allocate a new storage.
+  unsigned newCapacity =
+      std::max(unsigned(llvm::NextPowerOf2(storage.capacity + 2)), newSize);
+  auto *newStorageMem =
+      malloc(TrailingOperandStorage::totalSizeToAlloc<OpOperand>(newCapacity));
+  auto *newStorage = ::new (newStorageMem) TrailingOperandStorage();
+  newStorage->numOperands = newSize;
+  newStorage->capacity = newCapacity;
+
+  // Move the current operands to the new storage.
+  MutableArrayRef<OpOperand> newOperands = newStorage->getOperands();
+  std::uninitialized_copy(std::make_move_iterator(operands.begin()),
+                          std::make_move_iterator(operands.end()),
+                          newOperands.begin());
+
+  // Destroy the original operands.
+  for (auto &operand : operands)
+    operand.~OpOperand();
+
+  // Initialize any new operands.
+  for (unsigned e = newSize; numOperands != e; ++numOperands)
+    new (&newOperands[numOperands]) OpOperand(owner);
+
+  // If the current storage is also dynamic, free it.
+  if (isDynamicStorage())
+    free(&storage);
+
+  // Update the storage representation to use the new dynamic storage.
+  representation = reinterpret_cast<intptr_t>(newStorage);
+  representation |= DynamicStorageBit;
+  return newOperands;
 }
 
 /// Erase an operand held by the storage.
 void detail::OperandStorage::eraseOperand(unsigned index) {
   assert(index < size());
-  auto operands = getOperands();
-  --numOperands;
+  TrailingOperandStorage &storage = getStorage();
+  MutableArrayRef<OpOperand> operands = storage.getOperands();
+  --storage.numOperands;
 
   // Shift all operands down by 1 if the operand to remove is not at the end.
   auto indexIt = std::next(operands.begin(), index);
-  if (index != numOperands)
+  if (index != storage.numOperands)
     std::rotate(indexIt, std::next(indexIt), operands.end());
-  operands[numOperands].~OpOperand();
-}
-
-/// Grow the internal operand storage.
-void detail::OperandStorage::grow(ResizableStorage &resizeUtil,
-                                  size_t minSize) {
-  // Allocate a new storage array.
-  resizeUtil.capacity =
-      std::max(size_t(llvm::NextPowerOf2(resizeUtil.capacity + 2)), minSize);
-  OpOperand *newStorage = static_cast<OpOperand *>(
-      llvm::safe_malloc(resizeUtil.capacity * sizeof(OpOperand)));
-
-  // Move the current operands to the new storage.
-  auto operands = getOperands();
-  std::uninitialized_copy(std::make_move_iterator(operands.begin()),
-                          std::make_move_iterator(operands.end()), newStorage);
-
-  // Destroy the original operands and update the resizable storage pointer.
-  for (auto &operand : operands)
-    operand.~OpOperand();
-  resizeUtil.setDynamicStorage(newStorage);
+  operands[storage.numOperands].~OpOperand();
 }
 
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index c5b68ddff1b0..8ba391de7baf 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -3788,8 +3788,7 @@ Value OperationParser::createForwardRefPlaceholder(SMLoc loc, Type type) {
   auto name = OperationName("placeholder", getContext());
   auto *op = Operation::create(
       getEncodedSourceLocation(loc), name, type, /*operands=*/{},
-      /*attributes=*/llvm::None, /*successors=*/{}, /*numRegions=*/0,
-      /*resizableOperandList=*/false);
+      /*attributes=*/llvm::None, /*successors=*/{}, /*numRegions=*/0);
   forwardRefPlaceholders[op->getResult(0)] = loc;
   return op->getResult(0);
 }
@@ -3950,9 +3949,6 @@ Operation *OperationParser::parseGenericOperation() {
 
   OperationState result(srcLocation, name);
 
-  // Generic operations have a resizable operation list.
-  result.setOperandListToResizable();
-
   // Parse the operand list.
   SmallVector<SSAUseInfo, 8> operandInfos;
   if (parseToken(Token::l_paren, "expected '(' to start operand list") ||

diff  --git a/mlir/lib/TableGen/Operator.cpp b/mlir/lib/TableGen/Operator.cpp
index f967b76d074f..808ba7aabc76 100644
--- a/mlir/lib/TableGen/Operator.cpp
+++ b/mlir/lib/TableGen/Operator.cpp
@@ -169,10 +169,6 @@ const tblgen::OpTrait *tblgen::Operator::getTrait(StringRef trait) const {
   return nullptr;
 }
 
-bool tblgen::Operator::hasResizableOperandList() const {
-  return getTrait("OpTrait::ResizableOperandList") != nullptr;
-}
-
 auto tblgen::Operator::region_begin() const -> const_region_iterator {
   return regions.begin();
 }

diff  --git a/mlir/lib/Transforms/Utils/Utils.cpp b/mlir/lib/Transforms/Utils/Utils.cpp
index 481b7575ba0c..5cc83456a9fe 100644
--- a/mlir/lib/Transforms/Utils/Utils.cpp
+++ b/mlir/lib/Transforms/Utils/Utils.cpp
@@ -175,7 +175,6 @@ LogicalResult mlir::replaceAllMemRefUsesWith(Value oldMemRef, Value newMemRef,
 
   // Construct the new operation using this memref.
   OperationState state(op->getLoc(), op->getName());
-  state.setOperandListToResizable(op->hasResizableOperandsList());
   state.operands.reserve(op->getNumOperands() + extraIndices.size());
   // Insert the non-memref operands.
   state.operands.append(op->operand_begin(),

diff  --git a/mlir/test/mlir-tblgen/op-operand.td b/mlir/test/mlir-tblgen/op-operand.td
index 42aadfbdc119..2ffde33c5331 100644
--- a/mlir/test/mlir-tblgen/op-operand.td
+++ b/mlir/test/mlir-tblgen/op-operand.td
@@ -58,23 +58,3 @@ def OpD : NS_Op<"mix_variadic_and_normal_inputs_op", [SameVariadicOperandSize]>
 // CHECK-NEXT: odsState.addOperands(input1);
 // CHECK-NEXT: odsState.addOperands(input2);
 // CHECK-NEXT: odsState.addOperands(input3);
-// CHECK-NOT: odsState.setOperandListToResizable
-
-// Check that resizable operand list flag is set up correctly in all generated
-// builders and in the parser.
-def OpE : NS_Op<"resizable_operand_list", [ResizableOperandList]> {
-  let arguments = (ins Variadic<AnyType>:$input);
-  let assemblyFormat = "$input attr-dict `:` type($input)";
-}
-
-// CHECK-LABEL: OpE::build(Builder *odsBuilder, OperationState &odsState, ValueRange
-// CHECK: odsState.setOperandListToResizable()
-
-// CHECK-LABEL: OpE::build(Builder *odsBuilder, OperationState &odsState, ArrayRef<Type>
-// CHECK: odsState.setOperandListToResizable()
-
-// CHECK-LABEL: OpE::build(Builder *, OperationState
-// CHECK: odsState.setOperandListToResizable()
-
-// CHECK-LABEL: OpE::parse
-// CHECK: result.setOperandListToResizable()

diff  --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 7195165a3572..2d6c91f31817 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -809,8 +809,6 @@ void OpEmitter::genUseOperandAsResultTypeCollectiveParamBuilder() {
 
   // Operands
   body << "  " << builderOpState << ".addOperands(operands);\n";
-  if (op.hasResizableOperandList())
-    body << formatv("  {0}.setOperandListToResizable();\n\n", builderOpState);
 
   // Attributes
   body << "  " << builderOpState << ".addAttributes(attributes);\n";
@@ -892,8 +890,6 @@ void OpEmitter::genUseAttrAsResultTypeBuilder() {
 
   // Operands
   body << "  " << builderOpState << ".addOperands(operands);\n";
-  if (op.hasResizableOperandList())
-    body << formatv("  {0}.setOperandListToResizable();\n\n", builderOpState);
 
   // Attributes
   body << "  " << builderOpState << ".addAttributes(attributes);\n";
@@ -981,8 +977,6 @@ void OpEmitter::genCollectiveParamBuilder() {
          << numNonVariadicOperands
          << "u && \"mismatched number of parameters\");\n";
   body << "  " << builderOpState << ".addOperands(operands);\n";
-  if (op.hasResizableOperandList())
-    body << formatv("  {0}.setOperandListToResizable();\n\n", builderOpState);
 
   // Attributes
   body << "  " << builderOpState << ".addAttributes(attributes);\n";
@@ -1152,8 +1146,6 @@ void OpEmitter::genCodeForAddingArgAndRegionForBuilder(OpMethodBody &body,
       body << "  if (" << argName << ")\n  ";
     body << "  " << builderOpState << ".addOperands(" << argName << ");\n";
   }
-  if (op.hasResizableOperandList())
-    body << formatv("  {0}.setOperandListToResizable();\n", builderOpState);
 
   // If the operation has the operand segment size attribute, add it here.
   if (op.getTrait("OpTrait::AttrSizedOperandSegments")) {

diff  --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index f76a2d3af9e8..a8116e4290b4 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -706,10 +706,6 @@ void OperationFormat::genParser(Operator &op, OpClass &opClass) {
   genParserSuccessorResolution(op, body);
   genParserVariadicSegmentResolution(op, body);
 
-  // Mark the operation as having resizable operand list if required.
-  if (op.hasResizableOperandList())
-    body << "  result.setOperandListToResizable();\n";
-
   body << "  return success();\n";
 }
 

diff  --git a/mlir/unittests/IR/OperationSupportTest.cpp b/mlir/unittests/IR/OperationSupportTest.cpp
index b1cd5ef098dc..3cfb62553bfb 100644
--- a/mlir/unittests/IR/OperationSupportTest.cpp
+++ b/mlir/unittests/IR/OperationSupportTest.cpp
@@ -14,13 +14,13 @@
 using namespace mlir;
 using namespace mlir::detail;
 
-static Operation *createOp(MLIRContext *context, bool resizableOperands,
+static Operation *createOp(MLIRContext *context,
                            ArrayRef<Value> operands = llvm::None,
                            ArrayRef<Type> resultTypes = llvm::None) {
   context->allowUnregisteredDialects();
-  return Operation::create(
-      UnknownLoc::get(context), OperationName("foo.bar", context), resultTypes,
-      operands, llvm::None, llvm::None, 0, resizableOperands);
+  return Operation::create(UnknownLoc::get(context),
+                           OperationName("foo.bar", context), resultTypes,
+                           operands, llvm::None, llvm::None, 0);
 }
 
 namespace {
@@ -29,16 +29,11 @@ TEST(OperandStorageTest, NonResizable) {
   Builder builder(&context);
 
   Operation *useOp =
-      createOp(&context, /*resizableOperands=*/false, /*operands=*/llvm::None,
-               builder.getIntegerType(16));
+      createOp(&context, /*operands=*/llvm::None, builder.getIntegerType(16));
   Value operand = useOp->getResult(0);
 
   // Create a non-resizable operation with one operand.
-  Operation *user = createOp(&context, /*resizableOperands=*/false, operand,
-                             builder.getIntegerType(16));
-
-  // Sanity check the storage.
-  EXPECT_EQ(user->hasResizableOperandsList(), false);
+  Operation *user = createOp(&context, operand, builder.getIntegerType(16));
 
   // The same number of operands is okay.
   user->setOperands(operand);
@@ -53,41 +48,16 @@ TEST(OperandStorageTest, NonResizable) {
   useOp->destroy();
 }
 
-TEST(OperandStorageDeathTest, AddToNonResizable) {
-  MLIRContext context;
-  Builder builder(&context);
-
-  Operation *useOp =
-      createOp(&context, /*resizableOperands=*/false, /*operands=*/llvm::None,
-               builder.getIntegerType(16));
-  Value operand = useOp->getResult(0);
-
-  // Create a non-resizable operation with one operand.
-  Operation *user = createOp(&context, /*resizableOperands=*/false, operand,
-                             builder.getIntegerType(16));
-
-  // Sanity check the storage.
-  EXPECT_EQ(user->hasResizableOperandsList(), false);
-
-  // Adding operands to a non resizable operation should result in a failure.
-  ASSERT_DEATH(user->setOperands({operand, operand}), "");
-}
-
 TEST(OperandStorageTest, Resizable) {
   MLIRContext context;
   Builder builder(&context);
 
   Operation *useOp =
-      createOp(&context, /*resizableOperands=*/false, /*operands=*/llvm::None,
-               builder.getIntegerType(16));
+      createOp(&context, /*operands=*/llvm::None, builder.getIntegerType(16));
   Value operand = useOp->getResult(0);
 
   // Create a resizable operation with one operand.
-  Operation *user = createOp(&context, /*resizableOperands=*/true, operand,
-                             builder.getIntegerType(16));
-
-  // Sanity check the storage.
-  EXPECT_EQ(user->hasResizableOperandsList(), true);
+  Operation *user = createOp(&context, operand, builder.getIntegerType(16));
 
   // The same number of operands is okay.
   user->setOperands(operand);


        


More information about the Mlir-commits mailing list