diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h --- a/mlir/include/mlir/IR/OpDefinition.h +++ b/mlir/include/mlir/IR/OpDefinition.h @@ -271,6 +271,7 @@ LogicalResult verifyResultSizeAttr(Operation *op, StringRef sizeAttrName); LogicalResult verifyNoRegionArguments(Operation *op); LogicalResult verifyElementwise(Operation *op); +LogicalResult verifyIsIsolatedFromAbove(Operation *op); } // namespace impl /// Helper class for implementing traits. Clients are not expected to interact @@ -1163,10 +1164,7 @@ : public TraitBase { public: static LogicalResult verifyTrait(Operation *op) { - for (auto ®ion : op->getRegions()) - if (!region.isIsolatedFromAbove(op->getLoc())) - return failure(); - return success(); + return impl::verifyIsIsolatedFromAbove(op); } }; @@ -1631,9 +1629,9 @@ llvm::is_detected; /// Trait to check if T provides a general 'fold' method. template - using has_fold = decltype( - std::declval().fold(std::declval>(), - std::declval &>())); + using has_fold = decltype(std::declval().fold( + std::declval>(), + std::declval &>())); template using detect_has_fold = llvm::is_detected; /// Trait to check if T provides a 'print' method. diff --git a/mlir/include/mlir/IR/Region.h b/mlir/include/mlir/IR/Region.h --- a/mlir/include/mlir/IR/Region.h +++ b/mlir/include/mlir/IR/Region.h @@ -164,13 +164,16 @@ /// Return iterators that walk operations of type 'T' nested directly within /// this region. - template op_iterator op_begin() { + template + op_iterator op_begin() { return detail::op_filter_iterator(op_begin(), op_end()); } - template op_iterator op_end() { + template + op_iterator op_end() { return detail::op_filter_iterator(op_end(), op_end()); } - template iterator_range> getOps() { + template + iterator_range> getOps() { auto endIt = op_end(); return {detail::op_filter_iterator(op_begin(), endIt), detail::op_filter_iterator(endIt, endIt)}; @@ -189,7 +192,8 @@ /// Find the first parent operation of the given type, or nullptr if there is /// no ancestor operation. - template ParentT getParentOfType() { + template + ParentT getParentOfType() { auto *region = this; do { if (auto parent = dyn_cast_or_null(region->container)) @@ -226,12 +230,6 @@ blocks.splice(blocks.end(), other.getBlocks()); } - /// Check that this does not use any value defined outside it. - /// Emit errors if `noteLoc` is provided; this location is used to point - /// to the operation containing the region, the actual error is reported at - /// the operation with an offending use. - bool isIsolatedFromAbove(Optional noteLoc = llvm::None); - /// Returns 'block' if 'block' lies in this region, or otherwise finds the /// ancestor of 'block' that lies in this region. Returns nullptr if the /// latter fails. 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 @@ -1104,6 +1104,54 @@ return success(); } +/// Check for any values used by operations regions attached to the +/// specified "IsIsolatedFromAbove" operation defined outside of it. +LogicalResult OpTrait::impl::verifyIsIsolatedFromAbove(Operation *isolatedOp) { + assert(isolatedOp->hasTrait() && + "Intended to check IsolatedFromAbove ops"); + + // List of regions to analyze. Each region is processed independently, with + // respect to the common `limit` region, so we can look at them in any order. + // Therefore, use a simple vector and push/pop back the current region. + SmallVector pendingRegions; + for (auto ®ion : isolatedOp->getRegions()) { + pendingRegions.push_back(®ion); + + // Traverse all operations in the region. + while (!pendingRegions.empty()) { + for (Operation &op : pendingRegions.pop_back_val()->getOps()) { + for (Value operand : op.getOperands()) { + // operand should be non-null here if the IR is well-formed. But + // we don't assert here as this function is called from the verifier + // and so could be called on invalid IR. + if (!operand) + return op.emitOpError("operation's operand is null"); + + // Check that any value that is used by an operation is defined in the + // same region as either an operation result. + auto *operandRegion = operand.getParentRegion(); + if (!region.isAncestor(operandRegion)) { + return op.emitOpError("using value defined outside the region") + .attachNote(isolatedOp->getLoc()) + << "required by region isolation constraints"; + } + } + + // Schedule any regions in the operation for further checking. Don't + // recurse into other IsolatedFromAbove ops, because they will check + // themselves. + if (op.getNumRegions() && + !op.hasTrait()) { + for (Region &subRegion : op.getRegions()) + pendingRegions.push_back(&subRegion); + } + } + } + } + + return success(); +} + bool OpTrait::hasElementwiseMappableTraits(Operation *op) { return op->hasTrait() && op->hasTrait() && op->hasTrait() && op->hasTrait(); diff --git a/mlir/lib/IR/Region.cpp b/mlir/lib/IR/Region.cpp --- a/mlir/lib/IR/Region.cpp +++ b/mlir/lib/IR/Region.cpp @@ -137,60 +137,6 @@ b.dropAllReferences(); } -/// Check if there are any values used by operations in `region` defined -/// outside its ancestor region `limit`. That is, given `A{B{C{}}}` with region -/// `C` and limit `B`, the values defined in `B` can be used but the values -/// defined in `A` cannot. Emit errors if `noteLoc` is provided; this location -/// is used to point to the operation containing the region, the actual error is -/// reported at the operation with an offending use. -static bool isIsolatedAbove(Region ®ion, Region &limit, - Optional noteLoc) { - assert(limit.isAncestor(®ion) && - "expected isolation limit to be an ancestor of the given region"); - - // List of regions to analyze. Each region is processed independently, with - // respect to the common `limit` region, so we can look at them in any order. - // Therefore, use a simple vector and push/pop back the current region. - SmallVector pendingRegions; - pendingRegions.push_back(®ion); - - // Traverse all operations in the region. - while (!pendingRegions.empty()) { - for (Operation &op : pendingRegions.pop_back_val()->getOps()) { - for (Value operand : op.getOperands()) { - // operand should be non-null here if the IR is well-formed. But - // we don't assert here as this function is called from the verifier - // and so could be called on invalid IR. - if (!operand) { - if (noteLoc) - op.emitOpError("block's operand not defined").attachNote(noteLoc); - return false; - } - - // Check that any value that is used by an operation is defined in the - // same region as either an operation result or a block argument. - if (operand.getParentRegion()->isProperAncestor(&limit)) { - if (noteLoc) { - op.emitOpError("using value defined outside the region") - .attachNote(noteLoc) - << "required by region isolation constraints"; - } - return false; - } - } - // Schedule any regions the operations contain for further checking. - pendingRegions.reserve(pendingRegions.size() + op.getNumRegions()); - for (Region &subRegion : op.getRegions()) - pendingRegions.push_back(&subRegion); - } - } - return true; -} - -bool Region::isIsolatedFromAbove(Optional noteLoc) { - return isIsolatedAbove(*this, *this, noteLoc); -} - Region *llvm::ilist_traits<::mlir::Block>::getParentRegion() { size_t Offset( size_t(&((Region *)nullptr->*Region::getSublistAccess(nullptr))));