[Mlir-commits] [mlir] b10c662 - [mlir][SideEffects] Replace the old SideEffects dialect interface with the newly added op interfaces/traits.
River Riddle
llvmlistbot at llvm.org
Mon Mar 9 16:07:48 PDT 2020
Author: River Riddle
Date: 2020-03-09T16:02:21-07:00
New Revision: b10c662514510c0b7d22768681db2b19798b150d
URL: https://github.com/llvm/llvm-project/commit/b10c662514510c0b7d22768681db2b19798b150d
DIFF: https://github.com/llvm/llvm-project/commit/b10c662514510c0b7d22768681db2b19798b150d.diff
LOG: [mlir][SideEffects] Replace the old SideEffects dialect interface with the newly added op interfaces/traits.
Summary:
The old interface was a temporary stopgap to allow for implementing simple LICM that took side effects of region operations into account. Now that MLIR has proper support for specifying memory effects, this interface can be deleted.
Differential Revision: https://reviews.llvm.org/D74441
Added:
Modified:
mlir/include/mlir/Dialect/AffineOps/AffineOps.td
mlir/include/mlir/Dialect/LoopOps/LoopOps.td
mlir/lib/Dialect/AffineOps/AffineOps.cpp
mlir/lib/Dialect/LoopOps/LoopOps.cpp
mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
mlir/test/Transforms/loop-invariant-code-motion.mlir
Removed:
mlir/include/mlir/Transforms/SideEffectsInterface.h
################################################################################
diff --git a/mlir/include/mlir/Dialect/AffineOps/AffineOps.td b/mlir/include/mlir/Dialect/AffineOps/AffineOps.td
index efbe02b82ad7..150d2a52b640 100644
--- a/mlir/include/mlir/Dialect/AffineOps/AffineOps.td
+++ b/mlir/include/mlir/Dialect/AffineOps/AffineOps.td
@@ -41,7 +41,7 @@ def ImplicitAffineTerminator
: SingleBlockImplicitTerminator<"AffineTerminatorOp">;
def AffineForOp : Affine_Op<"for",
- [ImplicitAffineTerminator,
+ [ImplicitAffineTerminator, RecursiveSideEffects,
DeclareOpInterfaceMethods<LoopLikeOpInterface>]> {
let summary = "for operation";
let description = [{
@@ -177,7 +177,8 @@ def AffineForOp : Affine_Op<"for",
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
index 28b2e8c99392..1ae328ea9c9d 100644
--- a/mlir/include/mlir/Dialect/LoopOps/LoopOps.td
+++ b/mlir/include/mlir/Dialect/LoopOps/LoopOps.td
@@ -37,7 +37,8 @@ class Loop_Op<string mnemonic, list<OpTrait> traits = []> :
def ForOp : Loop_Op<"for",
[DeclareOpInterfaceMethods<LoopLikeOpInterface>,
- 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 ForOp : Loop_Op<"for",
}
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
index 517b7ae44b72..000000000000
--- 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<SideEffectsDialectInterface> {
-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<SideEffectsDialectInterface> {
-public:
- using SideEffecting = SideEffectsDialectInterface::SideEffecting;
- explicit SideEffectsInterface(MLIRContext *ctx)
- : DialectInterfaceCollection<SideEffectsDialectInterface>(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
index 755a42bf1ae6..7da6e25ce344 100644
--- 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 @@ struct AffineInlinerInterface : public DialectInlinerInterface {
/// 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<AffineIfOp>(op)) {
- return Recursive;
- }
- return SideEffectsDialectInterface::isSideEffecting(op);
- };
-};
-
} // end anonymous namespace
//===----------------------------------------------------------------------===//
@@ -88,7 +74,7 @@ AffineOpsDialect::AffineOpsDialect(MLIRContext *context)
#define GET_OP_LIST
#include "mlir/Dialect/AffineOps/AffineOps.cpp.inc"
>();
- addInterfaces<AffineInlinerInterface, AffineSideEffectsInterface>();
+ addInterfaces<AffineInlinerInterface>();
}
/// 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
index 9c28eec27eba..4d1533aba859 100644
--- a/mlir/lib/Dialect/LoopOps/LoopOps.cpp
+++ b/mlir/lib/Dialect/LoopOps/LoopOps.cpp
@@ -20,29 +20,10 @@
#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<IfOp>(op) || isa<ForOp>(op)) {
- return Recursive;
- }
- return SideEffectsDialectInterface::isSideEffecting(op);
- };
-};
-
-} // namespace
-
//===----------------------------------------------------------------------===//
// LoopOpsDialect
//===----------------------------------------------------------------------===//
@@ -53,7 +34,6 @@ LoopOpsDialect::LoopOpsDialect(MLIRContext *context)
#define GET_OP_LIST
#include "mlir/Dialect/LoopOps/LoopOps.cpp.inc"
>();
- addInterfaces<LoopSideEffectsInterface>();
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp b/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
index 9ed95a9d7a9e..ccac42f25cf3 100644
--- 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 @@ using namespace mlir;
namespace {
-using SideEffecting = SideEffectsInterface::SideEffecting;
-
/// Loop invariant code motion (LICM) pass.
struct LoopInvariantCodeMotion : public OperationPass<LoopInvariantCodeMotion> {
public:
@@ -41,40 +38,39 @@ struct LoopInvariantCodeMotion : public OperationPass<LoopInvariantCodeMotion> {
// - 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<bool(Value)> definedOutside,
- SideEffecting sideEffecting,
- SideEffectsInterface &interface) {
+ function_ref<bool(Value)> 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<MemoryEffectOpInterface>(op)) {
+ if (!memInterface.hasNoEffect())
return false;
+ } else if (!op->hasNoSideEffect() &&
+ !op->hasTrait<OpTrait::HasRecursiveSideEffects>()) {
+ 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<OpTrait::HasRecursiveSideEffects>())
+ 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 @@ static LogicalResult moveLoopInvariantCode(LoopLikeOpInterface looplike,
// 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 @@ static LogicalResult moveLoopInvariantCode(LoopLikeOpInterface looplike,
} // 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<LoopLikeOpInterface>(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
index 1c39d56a28e9..5873532fc254 100644
--- a/mlir/test/Transforms/loop-invariant-code-motion.mlir
+++ b/mlir/test/Transforms/loop-invariant-code-motion.mlir
@@ -105,6 +105,8 @@ func @invariant_affine_if() {
// 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: }
More information about the Mlir-commits
mailing list