diff --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td --- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td +++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td @@ -332,9 +332,7 @@ 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 --- a/mlir/include/mlir/IR/OpBase.td +++ b/mlir/include/mlir/IR/OpBase.td @@ -1679,10 +1679,6 @@ 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 --- a/mlir/include/mlir/IR/Operation.h +++ b/mlir/include/mlir/IR/Operation.h @@ -34,16 +34,14 @@ static Operation *create(Location location, OperationName name, ArrayRef resultTypes, ArrayRef operands, ArrayRef attributes, - ArrayRef successors, unsigned numRegions, - bool resizableOperandList); + ArrayRef 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 resultTypes, ArrayRef operands, NamedAttributeList attributes, - ArrayRef successors, unsigned numRegions, - bool resizableOperandList); + ArrayRef successors, unsigned numRegions); /// Create a new Operation from the fields stored in `state`. static Operation *create(const OperationState &state); @@ -53,8 +51,7 @@ ArrayRef resultTypes, ArrayRef operands, NamedAttributeList attributes, ArrayRef 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 @@ // 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 --- a/mlir/include/mlir/IR/OperationSupport.h +++ b/mlir/include/mlir/IR/OperationSupport.h @@ -273,8 +273,6 @@ SmallVector successors; /// Regions that the op will hold. SmallVector, 1> regions; - /// If the operation has a resizable operand list. - bool resizableOperandList = false; public: OperationState(Location location, StringRef name); @@ -284,8 +282,7 @@ OperationState(Location location, StringRef name, ValueRange operands, ArrayRef types, ArrayRef attributes, ArrayRef successors = {}, - MutableArrayRef> regions = {}, - bool resizableOperandList = false); + MutableArrayRef> regions = {}); void addOperands(ValueRange newOperands); @@ -330,11 +327,6 @@ /// region is null, a new empty region will be attached to the Operation. void addRegion(std::unique_ptr &®ion); - /// 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,55 +336,23 @@ //===----------------------------------------------------------------------===// 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; - } - ~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; - } - - /// 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; +/// A utility class representing the information for an inline operand storage. +struct InlineOperandStorage { + /// 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 -/// differences. 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 { 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 operands); + ~OperandStorage(); /// Replace the operands contained in the storage with the ones provided in /// 'operands'. @@ -403,54 +363,55 @@ /// Get the operation operands held by the storage. MutableArrayRef getOperands() { - return {getRawOperands(), size()}; + return LLVM_UNLIKELY(isDynamicStorage()) + ? MutableArrayRef(getDynamicStorage()) + : getInlineOperands(); } /// Return the number of operands held in the storage. - unsigned size() const { return numOperands; } + unsigned size() { + return LLVM_UNLIKELY(isDynamicStorage()) ? getDynamicStorage().size() + : getInlineStorage().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(numOperands), - sizeof(ResizableStorage)); + static size_t additionalAllocSize(unsigned numOperands) { + return additionalSizeToAlloc(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(); + enum : uint64_t { + /// The bit used to mark the storage as dynamic. + DynamicStorageBit = 1ull << 63ull + }; + + /// Returns the inline storage if the storage is current not dynamic. + InlineOperandStorage &getInlineStorage() { + assert(!isDynamicStorage() && "expected storage to be inline"); + static_assert(sizeof(InlineOperandStorage) <= sizeof(uint64_t), + "inline storage representation must fit within the opaque " + "representation"); + return *reinterpret_cast(&representation); } - /// 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( - getTrailingObjects()); + /// Returns the operands that are trailing this storage. + MutableArrayRef getInlineOperands() { + return {getTrailingObjects(), getInlineStorage().numOperands}; } - /// 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 dynamic storage container. + std::vector &getDynamicStorage() { + assert(isDynamicStorage() && "expected dynamic storage"); + uint64_t maskedRepresentation = representation & ~DynamicStorageBit; + return *reinterpret_cast *>(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 structured, or a pointer to a vector of OpOperand. + uint64_t representation; /// This stuff is used by the TrailingObjects template. friend llvm::TrailingObjects; diff --git a/mlir/include/mlir/IR/UseDefLists.h b/mlir/include/mlir/IR/UseDefLists.h --- a/mlir/include/mlir/IR/UseDefLists.h +++ b/mlir/include/mlir/IR/UseDefLists.h @@ -116,6 +116,11 @@ IROperand(Operation *owner, ValueType value) : value(value), owner(owner) { insertIntoCurrent(); } + IROperand(const IROperand &other) : owner(other.owner) { + // Only allow using the copy constructor if the other operand doesn't have a + // valid value. + assert(!other.value && "cannot construct from an operand with a value"); + } /// Return the current value being used by this operand. ValueType get() const { return value; } @@ -182,8 +187,7 @@ /// The operation owner of this operand. Operation *const owner; - /// Operands are not copyable or assignable. - IROperand(const IROperand &use) = delete; + /// Operands are not assignable. IROperand &operator=(const IROperand &use) = delete; void removeFromCurrent() { diff --git a/mlir/include/mlir/TableGen/Operator.h b/mlir/include/mlir/TableGen/Operator.h --- a/mlir/include/mlir/TableGen/Operator.h +++ b/mlir/include/mlir/TableGen/Operator.h @@ -165,9 +165,6 @@ // 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 --- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp +++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp @@ -1085,9 +1085,6 @@ 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 @@ 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 --- a/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp +++ b/mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp @@ -1055,8 +1055,7 @@ 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 --- a/mlir/lib/IR/Operation.cpp +++ b/mlir/lib/IR/Operation.cpp @@ -62,19 +62,17 @@ ArrayRef resultTypes, ArrayRef operands, ArrayRef attributes, - ArrayRef successors, unsigned numRegions, - bool resizableOperandList) { + ArrayRef 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 @@ ArrayRef resultTypes, ArrayRef operands, NamedAttributeList attributes, - ArrayRef successors, RegionRange regions, - bool resizableOperandList) { + ArrayRef 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 @@ ArrayRef resultTypes, ArrayRef operands, NamedAttributeList attributes, - ArrayRef successors, unsigned numRegions, - bool resizableOperandList) { + ArrayRef 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 @@ 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 @@ 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 @@ } /// 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 @@ // 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 --- a/mlir/lib/IR/OperationSupport.cpp +++ b/mlir/lib/IR/OperationSupport.cpp @@ -30,8 +30,7 @@ ValueRange operands, ArrayRef types, ArrayRef attributes, ArrayRef successors, - MutableArrayRef> regions, - bool resizableOperandList) + MutableArrayRef> regions) : location(location), name(name, location->getContext()), operands(operands.begin(), operands.end()), types(types.begin(), types.end()), @@ -62,86 +61,111 @@ // OperandStorage //===----------------------------------------------------------------------===// +detail::OperandStorage::OperandStorage(Operation *owner, ValueRange operands) + : representation(0) { + auto &inlineStorage = getInlineStorage(); + inlineStorage.numOperands = inlineStorage.capacity = operands.size(); + auto *operandPtrBegin = getTrailingObjects(); + for (unsigned i = 0, e = inlineStorage.numOperands; i < e; ++i) + new (&operandPtrBegin[i]) OpOperand(owner, operands[i]); +} + +detail::OperandStorage::~OperandStorage() { + // If the storage is dynamic, simply delete the dynamic storage container. + if (isDynamicStorage()) { + delete &getDynamicStorage(); + return; + } + + // Otherwise, manually destruct the trailing operands. + InlineOperandStorage &storage = getInlineStorage(); + ArrayRef trailingOperands(getTrailingObjects(), + storage.numOperands); + for (auto &operand : trailingOperands) + operand.~OpOperand(); +} + /// Replace the operands contained in the storage with the ones provided in /// 'operands'. void detail::OperandStorage::setOperands(Operation *owner, ValueRange operands) { - // 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 storage is dynamic, simply set update the operand list. + if (LLVM_UNLIKELY(isDynamicStorage())) { + auto &dynamicStorage = getDynamicStorage(); + dynamicStorage.resize(operands.size(), OpOperand(owner)); + for (unsigned i = 0, e = operands.size(); i != e; ++i) + dynamicStorage[i].set(operands[i]); + return; + } + // Otherwise, the storage is the trailing array. If the number of operands is + // less than or equal to the current amount, we can just update in place. + InlineOperandStorage &inlineStorage = getInlineStorage(); + unsigned &numOperands = inlineStorage.numOperands; + if (operands.size() <= numOperands) { // If the number of new operands is less than the current count, then remove // any extra operands. + MutableArrayRef inlineOperands = getInlineOperands(); for (unsigned i = operands.size(); i != numOperands; ++i) - opOperands[i].~OpOperand(); + inlineOperands[i].~OpOperand(); // Set the operands in place. numOperands = operands.size(); for (unsigned i = 0; i != numOperands; ++i) - opOperands[i].set(operands[i]); + inlineOperands[i].set(operands[i]); return; } - // 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 operand count is within the original inline capacity, we can + // grow inplace. + if (operands.size() <= inlineStorage.capacity) { + OpOperand *opBegin = getTrailingObjects(); + 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]); + return; } - // 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 change the storage to dynamic. + MutableArrayRef inlineOperands = getInlineOperands(); + auto *dynamicStorage = new std::vector( + std::make_move_iterator(inlineOperands.begin()), + std::make_move_iterator(inlineOperands.end())); + + // Destroy the original operands. + for (auto &operand : inlineOperands) + operand.~OpOperand(); + + // Update the dynamic storage with the new operands. + dynamicStorage->resize(operands.size(), OpOperand(owner)); + for (unsigned i = 0, e = operands.size(); i != e; ++i) + (*dynamicStorage)[i].set(operands[i]); + + // Update the storage representation to use the new dynamic storage. + representation = reinterpret_cast(dynamicStorage); + representation |= DynamicStorageBit; } /// Erase an operand held by the storage. void detail::OperandStorage::eraseOperand(unsigned index) { - assert(index < size()); - auto operands = getOperands(); - --numOperands; + if (LLVM_UNLIKELY(isDynamicStorage())) { + auto &dynamicStorage = getDynamicStorage(); + dynamicStorage.erase(dynamicStorage.begin() + index); + return; + } - // 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) - std::rotate(indexIt, std::next(indexIt), operands.end()); - operands[numOperands].~OpOperand(); -} + InlineOperandStorage &inlineStorage = getInlineStorage(); + MutableArrayRef inlineOperands = getInlineOperands(); + assert(index < inlineOperands.size()); + --inlineStorage.numOperands; -/// 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( - 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); + // Shift all operands down by 1 if the operand to remove is not at the end. + if (index != inlineStorage.numOperands) { + auto indexIt = std::next(inlineOperands.begin(), index); + std::rotate(indexIt, std::next(indexIt), inlineOperands.end()); + } + inlineOperands.back().~OpOperand(); } //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp --- a/mlir/lib/Parser/Parser.cpp +++ b/mlir/lib/Parser/Parser.cpp @@ -3788,8 +3788,7 @@ 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 @@ OperationState result(srcLocation, name); - // Generic operations have a resizable operation list. - result.setOperandListToResizable(); - // Parse the operand list. SmallVector 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 --- a/mlir/lib/TableGen/Operator.cpp +++ b/mlir/lib/TableGen/Operator.cpp @@ -169,10 +169,6 @@ 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 --- a/mlir/lib/Transforms/Utils/Utils.cpp +++ b/mlir/lib/Transforms/Utils/Utils.cpp @@ -175,7 +175,6 @@ // 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 --- a/mlir/test/mlir-tblgen/op-operand.td +++ b/mlir/test/mlir-tblgen/op-operand.td @@ -58,23 +58,3 @@ // 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:$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 -// 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 --- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp +++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp @@ -809,8 +809,6 @@ // 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 @@ // 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 @@ << 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 @@ 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 --- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp +++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp @@ -706,10 +706,6 @@ 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 --- 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 operands = llvm::None, ArrayRef 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 @@ 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 @@ 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);