[mlir][SideEffects] Replace HasNoSideEffect with the memory effect interfaces.

HasNoSideEffect can now be implemented using the MemoryEffectInterface, removing the need to check multiple things for the same information. This also removes an easy foot-gun for users as 'Operation::hasNoSideEffect' would ignore operations that dynamically, or recursively, have no side effects. This also leads to an immediate improvement in some of the existing users, such as DCE, now that they have access to more information.

Differential Revision: https://reviews.llvm.org/D76036

Author: River707

mlir/docs/Traits.md

@@ -208,12 +208,6 @@ foo.region_op {
 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<typename TerminatorOpType>` :

mlir/docs/Tutorials/Toy/Ch-2.md

@@ -206,9 +206,7 @@ class ConstantOp : public mlir::Op<ConstantOp,
                      /// 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 @@ operation.
 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 @@ values. The results correspond to a set of types for the values produced by the
 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 @@ for users of the dialect and can even be used to auto-generate Markdown
 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 @@ as part of `ConstantOp::verify`. This blob can assume that all of the other
 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 @@ of C++ parameters, as well as an optional code block that can be used to specify
 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

mlir/include/mlir/IR/Operation.h

@@ -407,13 +407,6 @@ class Operation final
     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.

mlir/include/mlir/IR/OperationSupport.h

@@ -68,19 +68,15 @@ enum class OperationProperty {
   /// 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

mlir/include/mlir/Interfaces/SideEffects.h

@@ -163,15 +163,6 @@ template <typename EffectT> class EffectInstance {
 //===----------------------------------------------------------------------===//
 
 namespace OpTrait {
-/// This trait indicates that an operation never has side effects.
-template <typename ConcreteType>
-class HasNoSideEffect : public TraitBase<ConcreteType, HasNoSideEffect> {
-public:
-  static AbstractOperation::OperationProperties getTraitProperties() {
-    return static_cast<AbstractOperation::OperationProperties>(
-        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 @@ struct Write : public Effect::Base<Write> {};
 
 //===----------------------------------------------------------------------===//
 // 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

mlir/include/mlir/Interfaces/SideEffects.td

@@ -98,6 +98,13 @@ class EffectOpInterfaceBase<string name, string baseEffect>
       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<OpTrait::HasRecursiveSideEffects>();
+    }
   }];
 
   // The base effect name of this interface.
@@ -108,11 +115,8 @@ class EffectOpInterfaceBase<string name, string baseEffect>
 // effect interfaces to define their effects.
 class SideEffect<EffectOpInterfaceBase interface, string effectName,
                  string resourceName> : 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 @@ class SideEffectsTraitBase<EffectOpInterfaceBase parentInterface,
   /// 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<SideEffect> effects = staticEffects;
 }
@@ -193,7 +200,7 @@ def MemWrite : MemWrite<"">;
 //===----------------------------------------------------------------------===//
 
 // Op has no side effect.
-def NoSideEffect : NativeOpTrait<"HasNoSideEffect">;
+def NoSideEffect : MemoryEffects<[]>;
 // Op has recursively computed side effects.
 def RecursiveSideEffects : NativeOpTrait<"HasRecursiveSideEffects">;
 

mlir/include/mlir/TableGen/SideEffects.h

@@ -27,10 +27,7 @@ class SideEffect : public Operator::VariableDecorator {
   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 @@ class SideEffectTrait : public InterfaceOpTrait {
   // 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);
 };
 

mlir/include/mlir/Transforms/FoldUtils.h

@@ -106,6 +106,9 @@ class OperationFolder {
     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

mlir/lib/Analysis/Utils.cpp

@@ -974,11 +974,12 @@ void mlir::getSequentialLoops(AffineForOp forOp,
 bool mlir::isLoopParallel(AffineForOp forOp) {
   // Collect all load and store ops in loop nest rooted at 'forOp'.
   SmallVector<Operation *, 8> loadAndStoreOpInsts;
-  auto walkResult = forOp.walk([&](Operation *opInst) {
+  auto walkResult = forOp.walk([&](Operation *opInst) -> WalkResult {
     if (isa<AffineLoadOp>(opInst) || isa<AffineStoreOp>(opInst))
       loadAndStoreOpInsts.push_back(opInst);
     else if (!isa<AffineForOp>(opInst) && !isa<AffineTerminatorOp>(opInst) &&
-             !isa<AffineIfOp>(opInst) && !opInst->hasNoSideEffect())
+             !isa<AffineIfOp>(opInst) &&
+             !MemoryEffectOpInterface::hasNoEffect(opInst))
       return WalkResult::interrupt();
 
     return WalkResult::advance();

mlir/lib/Conversion/LoopsToGPU/LoopsToGPU.cpp

@@ -797,7 +797,9 @@ ParallelToGpuLaunchLowering::matchAndRewrite(ParallelOp parallelOp,
       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();