diff --git a/mlir/docs/Traits.md b/mlir/docs/Traits.md --- a/mlir/docs/Traits.md +++ b/mlir/docs/Traits.md @@ -208,12 +208,6 @@ This trait is an important structural property of the IR, and enables operations to have [passes](WritingAPass.md) scheduled under them. -### NoSideEffect - -* `OpTrait::HasNoSideEffect` -- `NoSideEffect` - -This trait signifies that the operation is pure and has no visible side effects. - ### Single Block with Implicit Terminator * `OpTrait::SingleBlockImplicitTerminator` : diff --git a/mlir/docs/Tutorials/Toy/Ch-2.md b/mlir/docs/Tutorials/Toy/Ch-2.md --- a/mlir/docs/Tutorials/Toy/Ch-2.md +++ b/mlir/docs/Tutorials/Toy/Ch-2.md @@ -206,9 +206,7 @@ /// The ConstantOp takes no inputs. mlir::OpTrait::ZeroOperands, /// The ConstantOp returns a single result. - mlir::OpTrait::OneResult, - /// The ConstantOp is pure and has no visible side-effects. - mlir::OpTrait::HasNoSideEffect> { + mlir::OpTrait::OneResult> { public: /// Inherit the constructors from the base Op class. @@ -335,15 +333,13 @@ We define a toy operation by inheriting from our base 'Toy_Op' class above. Here we provide the mnemonic and a list of traits for the operation. The [mnemonic](../../OpDefinitions.md#operation-name) here matches the one given in -`ConstantOp::getOperationName` without the dialect prefix; `toy.`. The constant -operation here is also marked as 'NoSideEffect'. This is an ODS trait, and -matches one-to-one with the trait we providing when defining `ConstantOp`: -`mlir::OpTrait::HasNoSideEffect`. Missing here from our C++ definition are the -`ZeroOperands` and `OneResult` traits; these will be automatically inferred -based upon the `arguments` and `results` fields we define later. +`ConstantOp::getOperationName` without the dialect prefix; `toy.`. Missing here +from our C++ definition are the `ZeroOperands` and `OneResult` traits; these +will be automatically inferred based upon the `arguments` and `results` fields +we define later. ```tablegen -def ConstantOp : Toy_Op<"constant", [NoSideEffect]> { +def ConstantOp : Toy_Op<"constant"> { } ``` @@ -369,7 +365,7 @@ operation: ```tablegen -def ConstantOp : Toy_Op<"constant", [NoSideEffect]> { +def ConstantOp : Toy_Op<"constant"> { // The constant operation takes an attribute as the only input. // `F64ElementsAttr` corresponds to a 64-bit floating-point ElementsAttr. let arguments = (ins F64ElementsAttr:$value); @@ -394,7 +390,7 @@ documents. ```tablegen -def ConstantOp : Toy_Op<"constant", [NoSideEffect]> { +def ConstantOp : Toy_Op<"constant"> { // Provide a summary and description for this operation. This can be used to // auto-generate documentation of the operations within our dialect. let summary = "constant operation"; @@ -432,7 +428,7 @@ invariants of the operation have already been verified: ```tablegen -def ConstantOp : Toy_Op<"constant", [NoSideEffect]> { +def ConstantOp : Toy_Op<"constant"> { // Provide a summary and description for this operation. This can be used to // auto-generate documentation of the operations within our dialect. let summary = "constant operation"; @@ -472,7 +468,7 @@ the implementation inline. ```tablegen -def ConstantOp : Toy_Op<"constant", [NoSideEffect]> { +def ConstantOp : Toy_Op<"constant"> { ... // Add custom build methods for the constant operation. These methods populate 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 @@ -407,13 +407,6 @@ return false; } - /// Returns whether the operation has side-effects. - bool hasNoSideEffect() { - if (auto *absOp = getAbstractOperation()) - return absOp->hasProperty(OperationProperty::NoSideEffect); - return false; - } - /// Represents the status of whether an operation is a terminator. We /// represent an 'unknown' status because we want to support unregistered /// terminators. 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 @@ -68,19 +68,15 @@ /// results. Commutative = 0x1, - /// This bit is set for operations that have no side effects: that means that - /// they do not read or write memory, or access any hidden state. - NoSideEffect = 0x2, - /// This bit is set for an operation if it is a terminator: that means /// an operation at the end of a block. - Terminator = 0x4, + Terminator = 0x2, /// This bit is set for operations that are completely isolated from above. /// This is used for operations whose regions are explicit capture only, i.e. /// they are never allowed to implicitly reference values defined above the /// parent operation. - IsolatedFromAbove = 0x8, + IsolatedFromAbove = 0x4, }; /// This is a "type erased" representation of a registered operation. This diff --git a/mlir/include/mlir/Interfaces/SideEffects.h b/mlir/include/mlir/Interfaces/SideEffects.h --- a/mlir/include/mlir/Interfaces/SideEffects.h +++ b/mlir/include/mlir/Interfaces/SideEffects.h @@ -163,15 +163,6 @@ //===----------------------------------------------------------------------===// namespace OpTrait { -/// This trait indicates that an operation never has side effects. -template -class HasNoSideEffect : public TraitBase { -public: - static AbstractOperation::OperationProperties getTraitProperties() { - return static_cast( - OperationProperty::NoSideEffect); - } -}; /// This trait indicates that the side effects of an operation includes the /// effects of operations nested within its regions. If the operation has no /// derived effects interfaces, the operation itself can be assumed to have no @@ -221,10 +212,24 @@ //===----------------------------------------------------------------------===// // SideEffect Interfaces +//===----------------------------------------------------------------------===// /// Include the definitions of the side effect interfaces. #include "mlir/Interfaces/SideEffectInterfaces.h.inc" +//===----------------------------------------------------------------------===// +// SideEffect Utilities +//===----------------------------------------------------------------------===// + +/// Return true if the given operation is unused, and has no side effects on +/// memory that prevent erasing. +bool isOpTriviallyDead(Operation *op); + +/// Return true if the given operation would be dead if unused, and has no side +/// effects on memory that would prevent erasing. This is equivalent to checking +/// `isOpTriviallyDead` if `op` was unused. +bool wouldOpBeTriviallyDead(Operation *op); + } // end namespace mlir #endif // MLIR_INTERFACES_SIDEEFFECTS_H diff --git a/mlir/include/mlir/Interfaces/SideEffects.td b/mlir/include/mlir/Interfaces/SideEffects.td --- a/mlir/include/mlir/Interfaces/SideEffects.td +++ b/mlir/include/mlir/Interfaces/SideEffects.td @@ -98,6 +98,13 @@ getEffects(effects); return effects.empty(); } + + /// Returns if the given operation has no effects for this interface. + static bool hasNoEffect(Operation *op) { + if (auto interface = dyn_cast<}] # name # [{>(op)) + return interface.hasNoEffect(); + return op->hasTrait(); + } }]; // The base effect name of this interface. @@ -108,11 +115,8 @@ // effect interfaces to define their effects. class SideEffect : OpVariableDecorator { - /// The parent interface that the effect belongs to. - string interfaceTrait = interface.trait; - /// The name of the base effects class. - string baseEffect = interface.baseEffectName; + string baseEffectName = interface.baseEffectName; /// The derived effect that is being applied. string effect = effectName; @@ -128,6 +132,9 @@ /// The name of the interface trait to use. let trait = parentInterface.trait; + /// The name of the base effects class. + string baseEffectName = parentInterface.baseEffectName; + /// The derived effects being applied. list effects = staticEffects; } @@ -193,7 +200,7 @@ //===----------------------------------------------------------------------===// // Op has no side effect. -def NoSideEffect : NativeOpTrait<"HasNoSideEffect">; +def NoSideEffect : MemoryEffects<[]>; // Op has recursively computed side effects. def RecursiveSideEffects : NativeOpTrait<"HasRecursiveSideEffects">; diff --git a/mlir/include/mlir/TableGen/SideEffects.h b/mlir/include/mlir/TableGen/SideEffects.h --- a/mlir/include/mlir/TableGen/SideEffects.h +++ b/mlir/include/mlir/TableGen/SideEffects.h @@ -27,10 +27,7 @@ StringRef getName() const; // Return the name of the base C++ effect. - StringRef getBaseName() const; - - // Return the name of the parent interface trait. - StringRef getInterfaceTrait() const; + StringRef getBaseEffectName() const; // Return the name of the resource class. StringRef getResource() const; @@ -46,6 +43,9 @@ // Return the effects that are attached to the side effect interface. Operator::var_decorator_range getEffects() const; + // Return the name of the base C++ effect. + StringRef getBaseEffectName() const; + static bool classof(const OpTrait *t); }; diff --git a/mlir/include/mlir/Transforms/FoldUtils.h b/mlir/include/mlir/Transforms/FoldUtils.h --- a/mlir/include/mlir/Transforms/FoldUtils.h +++ b/mlir/include/mlir/Transforms/FoldUtils.h @@ -106,6 +106,9 @@ return op; } + /// Clear out any constants cached inside of the folder. + void clear(); + private: /// This map keeps track of uniqued constants by dialect, attribute, and type. /// A constant operation materializes an attribute with a type. Dialects may diff --git a/mlir/lib/Analysis/Utils.cpp b/mlir/lib/Analysis/Utils.cpp --- a/mlir/lib/Analysis/Utils.cpp +++ b/mlir/lib/Analysis/Utils.cpp @@ -974,11 +974,12 @@ bool mlir::isLoopParallel(AffineForOp forOp) { // Collect all load and store ops in loop nest rooted at 'forOp'. SmallVector loadAndStoreOpInsts; - auto walkResult = forOp.walk([&](Operation *opInst) { + auto walkResult = forOp.walk([&](Operation *opInst) -> WalkResult { if (isa(opInst) || isa(opInst)) loadAndStoreOpInsts.push_back(opInst); else if (!isa(opInst) && !isa(opInst) && - !isa(opInst) && !opInst->hasNoSideEffect()) + !isa(opInst) && + !MemoryEffectOpInterface::hasNoEffect(opInst)) return WalkResult::interrupt(); return WalkResult::advance(); diff --git a/mlir/lib/Conversion/LoopsToGPU/LoopsToGPU.cpp b/mlir/lib/Conversion/LoopsToGPU/LoopsToGPU.cpp --- a/mlir/lib/Conversion/LoopsToGPU/LoopsToGPU.cpp +++ b/mlir/lib/Conversion/LoopsToGPU/LoopsToGPU.cpp @@ -797,7 +797,9 @@ Operation *clone = rewriter.clone(*op, cloningMap); cloningMap.map(op->getResults(), clone->getResults()); // Check for side effects. - seenSideeffects |= !clone->hasNoSideEffect(); + // TODO: Handle region side effects properly. + seenSideeffects |= !MemoryEffectOpInterface::hasNoEffect(clone) || + clone->getNumRegions() != 0; // If we are no longer in the innermost scope, sideeffects are disallowed. if (seenSideeffects && leftNestingScope) return matchFailure(); diff --git a/mlir/lib/Dialect/Linalg/Transforms/Tiling.cpp b/mlir/lib/Dialect/Linalg/Transforms/Tiling.cpp --- a/mlir/lib/Dialect/Linalg/Transforms/Tiling.cpp +++ b/mlir/lib/Dialect/Linalg/Transforms/Tiling.cpp @@ -491,9 +491,7 @@ op.erase(); }); f.walk([](LinalgOp op) { - if (!op.getOperation()->hasNoSideEffect()) - return; - if (op.getOperation()->use_empty()) + if (isOpTriviallyDead(op)) op.erase(); }); } diff --git a/mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp b/mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp --- a/mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp +++ b/mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp @@ -132,13 +132,13 @@ void mlir::loop::naivelyFuseParallelOps(Region ®ion) { OpBuilder b(region); // Consider every single block and attempt to fuse adjacent loops. - for (auto &block : region.getBlocks()) { + for (auto &block : region) { SmallVector, 1> ploopChains{{}}; // Not using `walk()` to traverse only top-level parallel loops and also // make sure that there are no side-effecting ops between the parallel // loops. bool noSideEffects = true; - for (auto &op : block.getOperations()) { + for (auto &op : block) { if (auto ploop = dyn_cast(op)) { if (noSideEffects) { ploopChains.back().push_back(ploop); @@ -148,7 +148,9 @@ } continue; } - noSideEffects &= op.hasNoSideEffect(); + // TODO: Handle region side effects properly. + noSideEffects &= + MemoryEffectOpInterface::hasNoEffect(&op) && op.getNumRegions() == 0; } for (ArrayRef ploops : ploopChains) { for (int i = 0, e = ploops.size(); i + 1 < e; ++i) diff --git a/mlir/lib/Interfaces/SideEffects.cpp b/mlir/lib/Interfaces/SideEffects.cpp --- a/mlir/lib/Interfaces/SideEffects.cpp +++ b/mlir/lib/Interfaces/SideEffects.cpp @@ -25,3 +25,70 @@ return isa(effect) || isa(effect) || isa(effect) || isa(effect); } + +//===----------------------------------------------------------------------===// +// SideEffect Utilities +//===----------------------------------------------------------------------===// + +bool mlir::isOpTriviallyDead(Operation *op) { + return op->use_empty() && wouldOpBeTriviallyDead(op); +} + +/// Internal implementation of `mlir::wouldOpBeTriviallyDead` that also +/// considers terminator operations as dead if they have no side effects. This +/// allows for marking region operations as trivially dead without always being +/// conservative of terminators. +static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) { + // The set of operations to consider when checking for side effects. + SmallVector effectingOps(1, rootOp); + while (!effectingOps.empty()) { + Operation *op = effectingOps.pop_back_val(); + + // If the operation has recursive effects, push all of the nested operations + // on to the stack to consider. + bool hasRecursiveEffects = op->hasTrait(); + if (hasRecursiveEffects) { + for (Region ®ion : op->getRegions()) { + for (auto &block : region) { + for (auto &nestedOp : block) + effectingOps.push_back(&nestedOp); + } + } + } + + // If the op has memory effects, try to characterize them to see if the op + // is trivially dead here. + if (auto effectInterface = dyn_cast(op)) { + // Check to see if this op either has no effects, or only allocates/reads + // memory. + SmallVector effects; + effectInterface.getEffects(effects); + if (!llvm::all_of(effects, [](const auto &it) { + return isa(it.getEffect()) || + isa(it.getEffect()); + })) { + return false; + } + continue; + + // Otherwise, if the op has recursive side effects we can treat the + // operation itself as having no effects. + } else if (hasRecursiveEffects) { + continue; + } + + // If there were no effect interfaces, we treat this op as conservatively + // having effects. + return false; + } + + // If we get here, none of the operations had effects that prevented marking + // 'op' as dead. + return true; +} + +bool mlir::wouldOpBeTriviallyDead(Operation *op) { + if (!op->isKnownNonTerminator()) + return false; + return wouldOpBeTriviallyDeadImpl(op); +} diff --git a/mlir/lib/TableGen/SideEffects.cpp b/mlir/lib/TableGen/SideEffects.cpp --- a/mlir/lib/TableGen/SideEffects.cpp +++ b/mlir/lib/TableGen/SideEffects.cpp @@ -20,12 +20,8 @@ return def->getValueAsString("effect"); } -StringRef SideEffect::getBaseName() const { - return def->getValueAsString("baseEffect"); -} - -StringRef SideEffect::getInterfaceTrait() const { - return def->getValueAsString("interfaceTrait"); +StringRef SideEffect::getBaseEffectName() const { + return def->getValueAsString("baseEffectName"); } StringRef SideEffect::getResource() const { @@ -46,6 +42,10 @@ return {listInit->begin(), listInit->end()}; } +StringRef SideEffectTrait::getBaseEffectName() const { + return def->getValueAsString("baseEffectName"); +} + bool SideEffectTrait::classof(const OpTrait *t) { return t->getDef().isSubClassOf("SideEffectsTraitBase"); } diff --git a/mlir/lib/Transforms/CSE.cpp b/mlir/lib/Transforms/CSE.cpp --- a/mlir/lib/Transforms/CSE.cpp +++ b/mlir/lib/Transforms/CSE.cpp @@ -127,6 +127,13 @@ if (op->isKnownTerminator()) return failure(); + // If the operation is already trivially dead just add it to the erase list. + if (isOpTriviallyDead(op)) { + opsToErase.push_back(op); + ++numDCE; + return success(); + } + // Don't simplify operations with nested blocks. We don't currently model // equality comparisons correctly among other things. It is also unclear // whether we would want to CSE such operations. @@ -135,16 +142,9 @@ // TODO(riverriddle) We currently only eliminate non side-effecting // operations. - if (!op->hasNoSideEffect()) + if (!MemoryEffectOpInterface::hasNoEffect(op)) return failure(); - // If the operation is already trivially dead just add it to the erase list. - if (op->use_empty()) { - opsToErase.push_back(op); - ++numDCE; - return success(); - } - // Look for an existing definition for the operation. if (auto *existing = knownValues.lookup(op)) { // If we find one then replace all uses of the current operation with the diff --git a/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp b/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp --- a/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp +++ b/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp @@ -49,16 +49,18 @@ if (auto memInterface = dyn_cast(op)) { if (!memInterface.hasNoEffect()) return false; - } else if (!op->hasNoSideEffect() && - !op->hasTrait()) { + // If the operation doesn't have side effects and it doesn't recursively + // have side effects, it can always be hoisted. + if (!op->hasTrait()) + return true; + + // Otherwise, if the operation doesn't provide the memory effect interface + // and it doesn't have recursive side effects we treat it conservatively as + // side-effecting. + } else if (!op->hasTrait()) { return false; } - // If the operation doesn't have side effects and it doesn't recursively - // have side effects, it can always be hoisted. - if (!op->hasTrait()) - return true; - // Recurse into the regions for this op and check whether the contained ops // can be hoisted. for (auto ®ion : op->getRegions()) { diff --git a/mlir/lib/Transforms/Utils/FoldUtils.cpp b/mlir/lib/Transforms/Utils/FoldUtils.cpp --- a/mlir/lib/Transforms/Utils/FoldUtils.cpp +++ b/mlir/lib/Transforms/Utils/FoldUtils.cpp @@ -126,6 +126,12 @@ referencedDialects.erase(it); } +/// Clear out any constants cached inside of the folder. +void OperationFolder::clear() { + foldScopes.clear(); + referencedDialects.clear(); +} + /// Tries to perform folding on the given `op`. If successful, populates /// `results` with the results of the folding. LogicalResult OperationFolder::tryToFold( diff --git a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp --- a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp +++ b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp @@ -10,9 +10,8 @@ // //===----------------------------------------------------------------------===// -#include "mlir/Dialect/StandardOps/IR/Ops.h" -#include "mlir/IR/Builders.h" #include "mlir/IR/PatternMatch.h" +#include "mlir/Interfaces/SideEffects.h" #include "mlir/Transforms/FoldUtils.h" #include "mlir/Transforms/RegionUtils.h" #include "llvm/ADT/DenseMap.h" @@ -162,11 +161,8 @@ if (op == nullptr) continue; - // If the operation has no side effects, and no users, then it is - // trivially dead - remove it. - if (op->isKnownNonTerminator() && op->hasNoSideEffect() && - op->use_empty()) { - // Be careful to update bookkeeping. + // If the operation is trivially dead - remove it. + if (isOpTriviallyDead(op)) { notifyOperationRemoved(op); op->erase(); continue; @@ -204,7 +200,10 @@ // After applying patterns, make sure that the CFG of each of the regions is // kept up to date. - changed |= succeeded(simplifyRegions(regions)); + if (succeeded(simplifyRegions(regions))) { + folder.clear(); + changed = true; + } } while (changed && ++i < maxIterations); // Whether the rewrite converges, i.e. wasn't changed in the last iteration. return !changed; diff --git a/mlir/lib/Transforms/Utils/LoopUtils.cpp b/mlir/lib/Transforms/Utils/LoopUtils.cpp --- a/mlir/lib/Transforms/Utils/LoopUtils.cpp +++ b/mlir/lib/Transforms/Utils/LoopUtils.cpp @@ -887,7 +887,7 @@ } // Skip if op has side effects. // TODO(ntv): loads to immutable memory regions are ok. - if (!op.hasNoSideEffect()) { + if (!MemoryEffectOpInterface::hasNoEffect(&op)) { status = failure(); continue; } diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp --- a/mlir/lib/Transforms/Utils/RegionUtils.cpp +++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp @@ -12,6 +12,7 @@ #include "mlir/IR/RegionGraphTraits.h" #include "mlir/IR/Value.h" #include "mlir/Interfaces/ControlFlowInterfaces.h" +#include "mlir/Interfaces/SideEffects.h" #include "llvm/ADT/DepthFirstIterator.h" #include "llvm/ADT/PostOrderIterator.h" @@ -196,9 +197,8 @@ if (!op->isKnownNonTerminator()) return true; // If the op has a side effect, we treat it as live. - if (!op->hasNoSideEffect()) - return true; - return false; + // TODO: Properly handle region side effects. + return !MemoryEffectOpInterface::hasNoEffect(op) || op->getNumRegions() != 0; } static void propagateLiveness(Region ®ion, LiveMap &liveMap); diff --git a/mlir/test/Transforms/canonicalize.mlir b/mlir/test/Transforms/canonicalize.mlir --- a/mlir/test/Transforms/canonicalize.mlir +++ b/mlir/test/Transforms/canonicalize.mlir @@ -445,8 +445,6 @@ } } } - // CHECK-NEXT: %c0 = constant 0 : index - // CHECK-NEXT: %c1 = constant 1 : index // CHECK-NEXT: affine.for %arg7 = 0 to %arg2 { // CHECK-NEXT: affine.for %arg8 = 0 to %arg0 { // CHECK-NEXT: affine.for %arg9 = %arg0 to %arg0 { @@ -468,9 +466,7 @@ } } } - // CHECK: loop.for %{{.*}} = %c0 to %[[M]] step %c1 { - // CHECK: loop.for %arg8 = %c0 to %[[N]] step %c1 { - // CHECK: loop.for %arg9 = %c0 to %[[K]] step %c1 { + // CHECK-NEXT: return return } diff --git a/mlir/test/mlir-tblgen/op-decl.td b/mlir/test/mlir-tblgen/op-decl.td --- a/mlir/test/mlir-tblgen/op-decl.td +++ b/mlir/test/mlir-tblgen/op-decl.td @@ -10,9 +10,9 @@ class NS_Op traits> : Op; -// NoSideEffect trait is included twice to ensure it gets uniqued during +// IsolatedFromAbove trait is included twice to ensure it gets uniqued during // emission. -def NS_AOp : NS_Op<"a_op", [NoSideEffect, NoSideEffect]> { +def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> { let arguments = (ins I32:$a, Variadic:$b, @@ -55,7 +55,8 @@ // CHECK: ArrayRef tblgen_operands; // CHECK: }; -// CHECK: class AOp : public Op::Impl, OpTrait::ZeroSuccessor, OpTrait::AtLeastNOperands<1>::Impl, OpTrait::HasNoSideEffect +// CHECK: class AOp : public Op::Impl, OpTrait::ZeroSuccessor, OpTrait::AtLeastNOperands<1>::Impl, OpTrait::IsIsolatedFromAbove +// CHECK-NOT: OpTrait::IsIsolatedFromAbove // CHECK: public: // CHECK: using Op::Op; // CHECK: using OperandAdaptor = AOpOperandAdaptor; 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 @@ -1184,15 +1184,21 @@ unsigned index, unsigned kind) { for (auto decorator : decorators) if (SideEffect *effect = dyn_cast(&decorator)) - interfaceEffects[effect->getInterfaceTrait()].push_back( + interfaceEffects[effect->getBaseEffectName()].push_back( EffectLocation{*effect, index, kind}); }; // Collect effects that were specified via: /// Traits. - for (const auto &trait : op.getTraits()) - if (const auto *opTrait = dyn_cast(&trait)) - resolveDecorators(opTrait->getEffects(), /*index=*/0, EffectKind::Static); + for (const auto &trait : op.getTraits()) { + const auto *opTrait = dyn_cast(&trait); + if (!opTrait) + continue; + auto &effects = interfaceEffects[opTrait->getBaseEffectName()]; + for (auto decorator : opTrait->getEffects()) + effects.push_back(EffectLocation{cast(decorator), + /*index=*/0, EffectKind::Static}); + } /// Operands. for (unsigned i = 0, operandIt = 0, e = op.getNumArgs(); i != e; ++i) { if (op.getArg(i).is()) { @@ -1205,11 +1211,10 @@ resolveDecorators(op.getResultDecorators(i), i, EffectKind::Result); for (auto &it : interfaceEffects) { - StringRef baseEffect = it.second.front().effect.getBaseName(); auto effectsParam = llvm::formatv( "SmallVectorImpl> &effects", - baseEffect) + it.first()) .str(); // Generate the 'getEffects' method.