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,113 +336,93 @@ //===----------------------------------------------------------------------===// 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() { + 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 getOperands() { + return {getTrailingObjects(), 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 -/// 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 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 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(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 + }; + + /// Resize the storage to the given size. Returns the array containing the new + /// operands. + MutableArrayRef 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( - getTrailingObjects()); + /// 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 fit within the opaque " + "representation"); + return *reinterpret_cast(&representation); } - /// 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(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. + uint64_t representation; /// This stuff is used by the TrailingObjects template. friend llvm::TrailingObjects; 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,106 @@ // OperandStorage //===----------------------------------------------------------------------===// +detail::OperandStorage::OperandStorage(Operation *owner, ValueRange values) + : representation(0) { + auto &inlineStorage = getInlineStorage(); + inlineStorage.numOperands = inlineStorage.capacity = values.size(); + auto *operandPtrBegin = getTrailingObjects(); + 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 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 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 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(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(newCapacity)); + auto *newStorage = ::new (newStorageMem) TrailingOperandStorage(); + newStorage->numOperands = newSize; + newStorage->capacity = newCapacity; + + // Move the current operands to the new storage. + MutableArrayRef 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(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 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( - 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 --- 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);