[mlir][SideEffects] Replace the old SideEffects dialect interface with the newly added op interfaces/traits.

[rriddle]

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.

Depends On D74440

[jpienaar]

Looks good thanks

[rriddle]

Rebase

mlir/include/mlir/Dialect/AffineOps/AffineOps.td

class Affine_Op<string mnemonic, list<OpTrait> traits = []> :
   let parser = [{ return ::parse$cppClass(parser, result); }];
 }
 
 // Require regions to have affine terminator.
 def ImplicitAffineTerminator
     : SingleBlockImplicitTerminator<"AffineTerminatorOp">;
 
 def AffineForOp : Affine_Op<"for",
-    [ImplicitAffineTerminator,
+    [ImplicitAffineTerminator, RecursiveSideEffects,
      DeclareOpInterfaceMethods<LoopLikeOpInterface>]> {
   let summary = "for operation";
   let description = [{
     The "affine.for" operation represents an affine loop nest, defining an SSA
     value for its induction variable. It has one region capturing the loop body.
     The induction variable is represented as a argument of this region. This SSA
     value always has type index, which is the size of the machine word. The
     stride, represented by step, is a positive constant integer which defaults
let extraClassDeclaration = [{
     /// lists (same operands in the same order).
     bool matchingBoundOperandList();
   }];
 
   let hasCanonicalizer = 1;
   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
     executing two regions of code. The operands to an if operation are an
     IntegerSet condition and a set of symbol/dimension operands to the
     condition set. The operation produces no results. For example:
 
        affine.if #set(%i)  {

mlir/include/mlir/Dialect/LoopOps/LoopOps.td

class Loop_Op<string mnemonic, list<OpTrait> traits = []> :
   // functions.
   let printer = [{ return ::print(p, *this); }];
   let verifier = [{ return ::verify(*this); }];
   let parser = [{ return ::parse$cppClass(parser, result); }];
 }
 
 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
     operands that represent the lower bound, upper bound and step respectively.
     The operation defines an SSA value for its induction variable. It has one
     region capturing the loop body. The induction variable is represented as an
     argument of this region. This SSA value always has type index, which is the
     size of the machine word. The step is a value of type index, required to be
let extraClassDeclaration = [{
     /// Get Number of loop-carried values
     unsigned getNumIterOperands() {
       return getOperation()->getNumOperands() - getNumControlOperands();
     }
   }];
 }
 
 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
     conditionally executing two regions of code. The operand to an if operation
     is a boolean value. For example:
 
     ```mlir
        loop.if %b  {

mlir/include/mlir/Transforms/SideEffectsInterface.h

-//===- 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_

mlir/lib/Dialect/AffineOps/AffineOps.cpp

 #include "mlir/Dialect/AffineOps/AffineValueMap.h"
 #include "mlir/Dialect/StandardOps/IR/Ops.h"
 #include "mlir/IR/Function.h"
 #include "mlir/IR/IntegerSet.h"
 #include "mlir/IR/Matchers.h"
 #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"
 
 using namespace mlir;
 using llvm::dbgs;
 
 #define DEBUG_TYPE "affine-analysis"
bool isLegalToInline(Operation *op, Region *region,
     // above.
     // TODO: We should be able to inline into other regions than functions.
     return isa<FuncOp>(region->getParentOp());
   }
 
   /// 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
 
 //===----------------------------------------------------------------------===//
 // AffineOpsDialect
 //===----------------------------------------------------------------------===//
 
 AffineOpsDialect::AffineOpsDialect(MLIRContext *context)
     : Dialect(getDialectNamespace(), context) {
   addOperations<AffineApplyOp, AffineDmaStartOp, AffineDmaWaitOp, AffineLoadOp,
                 AffineStoreOp,
 #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
 /// the desired resultant type.
 Operation *AffineOpsDialect::materializeConstant(OpBuilder &builder,
                                                  Attribute value, Type type,
                                                  Location loc) {
   return builder.create<ConstantOp>(loc, type, value);

mlir/lib/Dialect/LoopOps/LoopOps.cpp

 #include "mlir/IR/Matchers.h"
 #include "mlir/IR/Module.h"
 #include "mlir/IR/OpImplementation.h"
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/IR/StandardTypes.h"
 #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
 //===----------------------------------------------------------------------===//
 
 LoopOpsDialect::LoopOpsDialect(MLIRContext *context)
     : Dialect(getDialectNamespace(), context) {
   addOperations<
 #define GET_OP_LIST
 #include "mlir/Dialect/LoopOps/LoopOps.cpp.inc"
       >();
-  addInterfaces<LoopSideEffectsInterface>();
 }
 
 //===----------------------------------------------------------------------===//
 // ForOp
 //===----------------------------------------------------------------------===//
 
 void ForOp::build(Builder *builder, OperationState &result, Value lb, Value ub,
                   Value step, ValueRange iterArgs) {

mlir/lib/Transforms/LoopInvariantCodeMotion.cpp

 //===----------------------------------------------------------------------===//
 
 #include "mlir/Transforms/Passes.h"
 
 #include "mlir/IR/Builders.h"
 #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"
 
 #define DEBUG_TYPE "licm"
 
 using namespace mlir;
 
 namespace {
 
-using SideEffecting = SideEffectsInterface::SideEffecting;
-
 /// Loop invariant code motion (LICM) pass.
 struct LoopInvariantCodeMotion : public OperationPass<LoopInvariantCodeMotion> {
 public:
   void runOnOperation() override;
 };
 
 // Checks whether the given op can be hoisted by checking that
 // - the op and any of its contained operations do not depend on SSA values
 //   defined inside of the loop (by means of calling definedOutside).
 // - 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)
-      return false;
+  if (auto memInterface = dyn_cast<MemoryEffectOpInterface>(op)) {
+    if (!memInterface.hasNoEffect())
+      return false;
+  } else if (!op->hasNoSideEffect() &&
+             !op->hasTrait<OpTrait::HasRecursiveSideEffects>()) {
   }
+
+  // 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 &region : 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
   // and this is easiest.
   SmallPtrSet<Operation *, 8> willBeMovedSet;
   SmallVector<Operation *, 8> opsToMove;
 
   // Helper to check whether an operation is loop invariant wrt. SSA properties.
   auto isDefinedOutsideOfBody = [&](Value value) {
     auto definingOp = value.getDefiningOp();
     return (definingOp && !!willBeMovedSet.count(definingOp)) ||
            looplike.isDefinedOutsideOfLoop(value);
   };
 
   // Do not use walk here, as we do not want to go into nested regions and hoist
   // operations from there. These regions might have semantics unknown to this
   // 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);
       }
     }
   }
 
   // For all instructions that we found to be invariant, move outside of the
   // loop.
   auto result = looplike.moveOutOfLoop(opsToMove);
   LLVM_DEBUG(looplike.print(llvm::dbgs() << "Modified loop\n"));
   return result;
 }
 
 } // 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)))
   });
 }
 
 // Include the generated code for the loop-like interface here, as it otherwise
 // has no compilation unit. This works as loop-invariant code motion is the
 // only user of that interface.
 #include "mlir/Transforms/LoopLikeInterface.cpp.inc"
 
 std::unique_ptr<Pass> mlir::createLoopInvariantCodeMotionPass() {
   return std::make_unique<LoopInvariantCodeMotion>();
 }
 
 static PassRegistration<LoopInvariantCodeMotion>
     pass("loop-invariant-code-motion",
          "Hoist loop invariant instructions outside of the loop");

mlir/test/Transforms/loop-invariant-code-motion.mlir

affine.for %arg1 = 0 to 10 {
           %cf9 = addf %cf8, %cf8 : f32
       }
     }
   }
 
   // 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: }
 
   return
 }
 
 func @invariant_affine_if2() {