diff --git a/mlir/include/mlir/Dialect/AffineOps/AffineOps.td b/mlir/include/mlir/Dialect/AffineOps/AffineOps.td --- a/mlir/include/mlir/Dialect/AffineOps/AffineOps.td +++ b/mlir/include/mlir/Dialect/AffineOps/AffineOps.td @@ -41,7 +41,7 @@ : SingleBlockImplicitTerminator<"AffineTerminatorOp">; def AffineForOp : Affine_Op<"for", - [ImplicitAffineTerminator, + [ImplicitAffineTerminator, RecursiveSideEffects, DeclareOpInterfaceMethods]> { let summary = "for operation"; let description = [{ @@ -177,7 +177,8 @@ let hasFolder = 1; } -def AffineIfOp : Affine_Op<"if", [ImplicitAffineTerminator]> { +def AffineIfOp : Affine_Op<"if", + [ImplicitAffineTerminator, RecursiveSideEffects]> { let summary = "if-then-else operation"; let description = [{ The "if" operation represents an if-then-else construct for conditionally diff --git a/mlir/include/mlir/Dialect/LoopOps/LoopOps.td b/mlir/include/mlir/Dialect/LoopOps/LoopOps.td --- a/mlir/include/mlir/Dialect/LoopOps/LoopOps.td +++ b/mlir/include/mlir/Dialect/LoopOps/LoopOps.td @@ -37,7 +37,8 @@ def ForOp : Loop_Op<"for", [DeclareOpInterfaceMethods, - SingleBlockImplicitTerminator<"YieldOp">]> { + SingleBlockImplicitTerminator<"YieldOp">, + RecursiveSideEffects]> { let summary = "for operation"; let description = [{ The "loop.for" operation represents a loop taking 3 SSA value as @@ -170,7 +171,7 @@ } def IfOp : Loop_Op<"if", - [SingleBlockImplicitTerminator<"YieldOp">]> { + [SingleBlockImplicitTerminator<"YieldOp">, RecursiveSideEffects]> { let summary = "if-then-else operation"; let description = [{ The "loop.if" operation represents an if-then-else construct for diff --git a/mlir/include/mlir/Transforms/SideEffectsInterface.h b/mlir/include/mlir/Transforms/SideEffectsInterface.h deleted file mode 100644 --- a/mlir/include/mlir/Transforms/SideEffectsInterface.h +++ /dev/null @@ -1,64 +0,0 @@ -//===- SideEffectsInterface.h - dialect interface modeling side effects ---===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -// This file specifies a dialect interface to model side-effects. -// -//===----------------------------------------------------------------------===// - -#ifndef MLIR_TRANSFORMS_SIDEEFFECTSINTERFACE_H_ -#define MLIR_TRANSFORMS_SIDEEFFECTSINTERFACE_H_ - -#include "mlir/IR/DialectInterface.h" -#include "mlir/IR/Operation.h" - -namespace mlir { - -/// Specifies an interface for basic side-effect modelling that is used by the -/// loop-invariant code motion pass. -/// -/// TODO: This interface should be replaced by a more general solution. -class SideEffectsDialectInterface - : public DialectInterface::Base { -public: - SideEffectsDialectInterface(Dialect *dialect) : Base(dialect) {} - - enum SideEffecting { - Never, /* the operation has no side-effects */ - Recursive, /* the operation has side-effects if a contained operation has */ - Always /* the operation has side-effects */ - }; - - /// Checks whether the given operation has side-effects. - virtual SideEffecting isSideEffecting(Operation *op) const { - if (op->hasNoSideEffect()) - return Never; - return Always; - }; -}; - -class SideEffectsInterface - : public DialectInterfaceCollection { -public: - using SideEffecting = SideEffectsDialectInterface::SideEffecting; - explicit SideEffectsInterface(MLIRContext *ctx) - : DialectInterfaceCollection(ctx) {} - - SideEffecting isSideEffecting(Operation *op) const { - // First check generic trait. - if (op->hasNoSideEffect()) - return SideEffecting::Never; - if (auto handler = getInterfaceFor(op)) - return handler->isSideEffecting(op); - - return SideEffecting::Always; - } -}; - -} // namespace mlir - -#endif // MLIR_TRANSFORMS_SIDEEFFECTSINTERFACE_H_ diff --git a/mlir/lib/Dialect/AffineOps/AffineOps.cpp b/mlir/lib/Dialect/AffineOps/AffineOps.cpp --- a/mlir/lib/Dialect/AffineOps/AffineOps.cpp +++ b/mlir/lib/Dialect/AffineOps/AffineOps.cpp @@ -15,7 +15,6 @@ #include "mlir/IR/OpImplementation.h" #include "mlir/IR/PatternMatch.h" #include "mlir/Transforms/InliningUtils.h" -#include "mlir/Transforms/SideEffectsInterface.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallBitVector.h" #include "llvm/Support/Debug.h" @@ -62,19 +61,6 @@ /// Affine regions should be analyzed recursively. bool shouldAnalyzeRecursively(Operation *op) const final { return true; } }; - -// TODO(mlir): Extend for other ops in this dialect. -struct AffineSideEffectsInterface : public SideEffectsDialectInterface { - using SideEffectsDialectInterface::SideEffectsDialectInterface; - - SideEffecting isSideEffecting(Operation *op) const override { - if (isa(op)) { - return Recursive; - } - return SideEffectsDialectInterface::isSideEffecting(op); - }; -}; - } // end anonymous namespace //===----------------------------------------------------------------------===// @@ -88,7 +74,7 @@ #define GET_OP_LIST #include "mlir/Dialect/AffineOps/AffineOps.cpp.inc" >(); - addInterfaces(); + addInterfaces(); } /// Materialize a single constant operation from a given attribute value with diff --git a/mlir/lib/Dialect/LoopOps/LoopOps.cpp b/mlir/lib/Dialect/LoopOps/LoopOps.cpp --- a/mlir/lib/Dialect/LoopOps/LoopOps.cpp +++ b/mlir/lib/Dialect/LoopOps/LoopOps.cpp @@ -20,30 +20,11 @@ #include "mlir/IR/Value.h" #include "mlir/Support/MathExtras.h" #include "mlir/Support/STLExtras.h" -#include "mlir/Transforms/SideEffectsInterface.h" using namespace mlir; using namespace mlir::loop; //===----------------------------------------------------------------------===// -// LoopOpsDialect Interfaces -//===----------------------------------------------------------------------===// -namespace { - -struct LoopSideEffectsInterface : public SideEffectsDialectInterface { - using SideEffectsDialectInterface::SideEffectsDialectInterface; - - SideEffecting isSideEffecting(Operation *op) const override { - if (isa(op) || isa(op)) { - return Recursive; - } - return SideEffectsDialectInterface::isSideEffecting(op); - }; -}; - -} // namespace - -//===----------------------------------------------------------------------===// // LoopOpsDialect //===----------------------------------------------------------------------===// @@ -53,7 +34,6 @@ #define GET_OP_LIST #include "mlir/Dialect/LoopOps/LoopOps.cpp.inc" >(); - addInterfaces(); } //===----------------------------------------------------------------------===// 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 @@ -16,7 +16,6 @@ #include "mlir/IR/Function.h" #include "mlir/Pass/Pass.h" #include "mlir/Transforms/LoopLikeInterface.h" -#include "mlir/Transforms/SideEffectsInterface.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" @@ -27,8 +26,6 @@ namespace { -using SideEffecting = SideEffectsInterface::SideEffecting; - /// Loop invariant code motion (LICM) pass. struct LoopInvariantCodeMotion : public OperationPass { public: @@ -41,40 +38,39 @@ // - the op has no side-effects. If sideEffecting is Never, sideeffects of this // op and its nested ops are ignored. static bool canBeHoisted(Operation *op, - function_ref definedOutside, - SideEffecting sideEffecting, - SideEffectsInterface &interface) { + function_ref definedOutside) { // Check that dependencies are defined outside of loop. if (!llvm::all_of(op->getOperands(), definedOutside)) return false; // Check whether this op is side-effect free. If we already know that there // can be no side-effects because the surrounding op has claimed so, we can // (and have to) skip this step. - auto thisOpIsSideEffecting = sideEffecting; - if (thisOpIsSideEffecting != SideEffecting::Never) { - thisOpIsSideEffecting = interface.isSideEffecting(op); - // If the op always has sideeffects, we cannot hoist. - if (thisOpIsSideEffecting == SideEffecting::Always) + if (auto memInterface = dyn_cast(op)) { + if (!memInterface.hasNoEffect()) return false; + } else if (!op->hasNoSideEffect() && + !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()) { for (auto &block : region.getBlocks()) { - for (auto &innerOp : block) { - if (innerOp.isKnownTerminator()) - continue; - if (!canBeHoisted(&innerOp, definedOutside, thisOpIsSideEffecting, - interface)) + for (auto &innerOp : block.without_terminator()) + if (!canBeHoisted(&innerOp, definedOutside)) return false; - } } } return true; } -static LogicalResult moveLoopInvariantCode(LoopLikeOpInterface looplike, - SideEffectsInterface &interface) { +static LogicalResult moveLoopInvariantCode(LoopLikeOpInterface looplike) { auto &loopBody = looplike.getLoopBody(); // We use two collections here as we need to preserve the order for insertion @@ -94,9 +90,7 @@ // rewriting. If the nested regions are loops, they will have been processed. for (auto &block : loopBody) { for (auto &op : block.without_terminator()) { - if (canBeHoisted(&op, isDefinedOutsideOfBody, - mlir::SideEffectsDialectInterface::Recursive, - interface)) { + if (canBeHoisted(&op, isDefinedOutsideOfBody)) { opsToMove.push_back(&op); willBeMovedSet.insert(&op); } @@ -113,16 +107,13 @@ } // end anonymous namespace void LoopInvariantCodeMotion::runOnOperation() { - SideEffectsInterface interface(&getContext()); // Walk through all loops in a function in innermost-loop-first order. This // way, we first LICM from the inner loop, and place the ops in // the outer loop, which in turn can be further LICM'ed. - getOperation()->walk([&](Operation *op) { - if (auto looplike = dyn_cast(op)) { - LLVM_DEBUG(op->print(llvm::dbgs() << "\nOriginal loop\n")); - if (failed(moveLoopInvariantCode(looplike, interface))) - signalPassFailure(); - } + getOperation()->walk([&](LoopLikeOpInterface loopLike) { + LLVM_DEBUG(loopLike.print(llvm::dbgs() << "\nOriginal loop\n")); + if (failed(moveLoopInvariantCode(loopLike))) + signalPassFailure(); }); } diff --git a/mlir/test/Transforms/loop-invariant-code-motion.mlir b/mlir/test/Transforms/loop-invariant-code-motion.mlir --- a/mlir/test/Transforms/loop-invariant-code-motion.mlir +++ b/mlir/test/Transforms/loop-invariant-code-motion.mlir @@ -105,6 +105,8 @@ // CHECK: %0 = alloc() : memref<10xf32> // CHECK-NEXT: %[[CST:.*]] = constant 8.000000e+00 : f32 // CHECK-NEXT: affine.for %[[ARG:.*]] = 0 to 10 { + // CHECK-NEXT: } + // CHECK-NEXT: affine.for %[[ARG:.*]] = 0 to 10 { // CHECK-NEXT: affine.if #set0(%[[ARG]], %[[ARG]]) { // CHECK-NEXT: addf %[[CST]], %[[CST]] : f32 // CHECK-NEXT: }