[Mlir-commits] [mlir] fe7c0d9 - [mlir][IR] Remove the concept of `OperationProperties`

River Riddle llvmlistbot at llvm.org
Tue Feb 9 12:00:32 PST 2021


Author: River Riddle
Date: 2021-02-09T12:00:15-08:00
New Revision: fe7c0d90b2940232cc6ab396253091bcf1ca4f2f

URL: https://github.com/llvm/llvm-project/commit/fe7c0d90b2940232cc6ab396253091bcf1ca4f2f
DIFF: https://github.com/llvm/llvm-project/commit/fe7c0d90b2940232cc6ab396253091bcf1ca4f2f.diff

LOG: [mlir][IR] Remove the concept of `OperationProperties`

These properties were useful for a few things before traits had a better integration story, but don't really carry their weight well these days. Most of these properties are already checked via traits in most of the code. It is better to align the system around traits, and improve the performance/cost of traits in general.

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

Added: 
    

Modified: 
    mlir/include/mlir/IR/OpDefinition.h
    mlir/include/mlir/IR/Operation.h
    mlir/include/mlir/IR/OperationSupport.h
    mlir/lib/CAPI/IR/IR.cpp
    mlir/lib/Dialect/Affine/IR/AffineOps.cpp
    mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp
    mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp
    mlir/lib/Dialect/StandardOps/Transforms/FuncBufferize.cpp
    mlir/lib/EDSC/Builders.cpp
    mlir/lib/IR/AsmPrinter.cpp
    mlir/lib/IR/Block.cpp
    mlir/lib/IR/MLIRContext.cpp
    mlir/lib/IR/Operation.cpp
    mlir/lib/IR/Verifier.cpp
    mlir/lib/Interfaces/SideEffectInterfaces.cpp
    mlir/lib/Parser/Parser.cpp
    mlir/lib/Pass/Pass.cpp
    mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
    mlir/lib/Transforms/CSE.cpp
    mlir/lib/Transforms/Inliner.cpp
    mlir/lib/Transforms/SCCP.cpp
    mlir/lib/Transforms/Utils/FoldUtils.cpp
    mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
    mlir/lib/Transforms/Utils/RegionUtils.cpp
    mlir/test/lib/Dialect/Shape/TestShapeFunctions.cpp
    mlir/test/lib/Dialect/Test/TestPatterns.cpp
    mlir/test/lib/Transforms/TestDynamicPipeline.cpp
    mlir/test/lib/Transforms/TestOpaqueLoc.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index 79024174375b..40f037e8a7ef 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -671,10 +671,6 @@ class VariadicResults
 template <typename ConcreteType>
 class IsTerminator : public TraitBase<ConcreteType, IsTerminator> {
 public:
-  static AbstractOperation::OperationProperties getTraitProperties() {
-    return static_cast<AbstractOperation::OperationProperties>(
-        OperationProperty::Terminator);
-  }
   static LogicalResult verifyTrait(Operation *op) {
     return impl::verifyIsTerminator(op);
   }
@@ -1001,13 +997,7 @@ class ResultsAreSignlessIntegerLike
 
 /// This class adds property that the operation is commutative.
 template <typename ConcreteType>
-class IsCommutative : public TraitBase<ConcreteType, IsCommutative> {
-public:
-  static AbstractOperation::OperationProperties getTraitProperties() {
-    return static_cast<AbstractOperation::OperationProperties>(
-        OperationProperty::Commutative);
-  }
-};
+class IsCommutative : public TraitBase<ConcreteType, IsCommutative> {};
 
 /// This class adds property that the operation is an involution.
 /// This means a unary to unary operation "f" that satisfies f(f(x)) = x
@@ -1110,10 +1100,6 @@ template <typename ConcreteType>
 class IsIsolatedFromAbove
     : public TraitBase<ConcreteType, IsIsolatedFromAbove> {
 public:
-  static AbstractOperation::OperationProperties getTraitProperties() {
-    return static_cast<AbstractOperation::OperationProperties>(
-        OperationProperty::IsolatedFromAbove);
-  }
   static LogicalResult verifyTrait(Operation *op) {
     for (auto &region : op->getRegions())
       if (!region.isIsolatedFromAbove(op->getLoc()))
@@ -1425,34 +1411,6 @@ foldTraits(Operation *op, ArrayRef<Attribute> operands,
   return failure();
 }
 
-//===----------------------------------------------------------------------===//
-// Trait Properties
-
-/// Trait to check if T provides a `getTraitProperties` method.
-template <typename T, typename... Args>
-using has_get_trait_properties = decltype(T::getTraitProperties());
-template <typename T>
-using detect_has_get_trait_properties =
-    llvm::is_detected<has_get_trait_properties, T>;
-
-/// The internal implementation of `getTraitProperties` below that returns the
-/// OR of invoking `getTraitProperties` on all of the provided trait types `Ts`.
-template <typename... Ts>
-static AbstractOperation::OperationProperties
-getTraitPropertiesImpl(std::tuple<Ts...> *) {
-  AbstractOperation::OperationProperties result = 0;
-  (void)std::initializer_list<int>{(result |= Ts::getTraitProperties(), 0)...};
-  return result;
-}
-
-/// Given a tuple type containing a set of traits that contain a
-/// `getTraitProperties` method, return the OR of all of the results of invoking
-/// those methods.
-template <typename TraitTupleT>
-static AbstractOperation::OperationProperties getTraitProperties() {
-  return getTraitPropertiesImpl((TraitTupleT *)nullptr);
-}
-
 //===----------------------------------------------------------------------===//
 // Trait Verification
 
@@ -1574,14 +1532,6 @@ class Op : public OpState, public Traits<ConcreteType>... {
       typename detail::FilterTypes<op_definition_impl::detect_has_verify_trait,
                                    Traits<ConcreteType>...>::type;
 
-  /// Returns the properties of this operation by combining the properties
-  /// defined by the traits.
-  static AbstractOperation::OperationProperties getOperationProperties() {
-    return op_definition_impl::getTraitProperties<typename detail::FilterTypes<
-        op_definition_impl::detect_has_get_trait_properties,
-        Traits<ConcreteType>...>::type>();
-  }
-
   /// Returns an interface map containing the interfaces registered to this
   /// operation.
   static detail::InterfaceMap getInterfaceMap() {

diff  --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index 70cd55dbbb13..63a94d971da7 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -455,47 +455,6 @@ class alignas(8) Operation final
   // Accessors for various properties of operations
   //===--------------------------------------------------------------------===//
 
-  /// Returns whether the operation is commutative.
-  bool isCommutative() {
-    if (auto *absOp = getAbstractOperation())
-      return absOp->hasProperty(OperationProperty::Commutative);
-    return false;
-  }
-
-  /// Represents the status of whether an operation is a terminator. We
-  /// represent an 'unknown' status because we want to support unregistered
-  /// terminators.
-  enum class TerminatorStatus { Terminator, NonTerminator, Unknown };
-
-  /// Returns the status of whether this operation is a terminator or not.
-  TerminatorStatus getTerminatorStatus() {
-    if (auto *absOp = getAbstractOperation()) {
-      return absOp->hasProperty(OperationProperty::Terminator)
-                 ? TerminatorStatus::Terminator
-                 : TerminatorStatus::NonTerminator;
-    }
-    return TerminatorStatus::Unknown;
-  }
-
-  /// Returns true if the operation is known to be a terminator.
-  bool isKnownTerminator() {
-    return getTerminatorStatus() == TerminatorStatus::Terminator;
-  }
-
-  /// Returns true if the operation is known to *not* be a terminator.
-  bool isKnownNonTerminator() {
-    return getTerminatorStatus() == TerminatorStatus::NonTerminator;
-  }
-
-  /// Returns true if the operation is known to be completely isolated from
-  /// enclosing regions, i.e., no internal regions reference values defined
-  /// above this operation.
-  bool isKnownIsolatedFromAbove() {
-    if (auto *absOp = getAbstractOperation())
-      return absOp->hasProperty(OperationProperty::IsolatedFromAbove);
-    return false;
-  }
-
   /// Attempt to fold this operation with the specified constant operand values
   /// - the elements in "operands" will correspond directly to the operands of
   /// the operation, but may be null if non-constant. If folding is successful,
@@ -506,8 +465,16 @@ class alignas(8) Operation final
   /// Returns true if the operation was registered with a particular trait, e.g.
   /// hasTrait<OperandsAreSignlessIntegerLike>().
   template <template <typename T> class Trait> bool hasTrait() {
-    auto *absOp = getAbstractOperation();
-    return absOp ? absOp->hasTrait<Trait>() : false;
+    const AbstractOperation *abstractOp = getAbstractOperation();
+    return abstractOp ? abstractOp->hasTrait<Trait>() : false;
+  }
+
+  /// Returns true if the operation is *might* have the provided trait. This
+  /// means that either the operation is unregistered, or it was registered with
+  /// the provide trait.
+  template <template <typename T> class Trait> bool mightHaveTrait() {
+    const AbstractOperation *abstractOp = getAbstractOperation();
+    return abstractOp ? abstractOp->hasTrait<Trait>() : true;
   }
 
   //===--------------------------------------------------------------------===//

diff  --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 818cda01bff5..b6283fc071c8 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -55,33 +55,12 @@ class OwningRewritePatternList;
 // AbstractOperation
 //===----------------------------------------------------------------------===//
 
-enum class OperationProperty {
-  /// This bit is set for an operation if it is a commutative
-  /// operation: that is an operator where order of operands does not
-  /// change the result of the operation.  For example, in a binary
-  /// commutative operation, "a op b" and "b op a" produce the same
-  /// results.
-  Commutative = 0x1,
-
-  /// This bit is set for an operation if it is a terminator: that means
-  /// an operation at the end of a block.
-  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 = 0x4,
-};
-
 /// This is a "type erased" representation of a registered operation.  This
 /// should only be used by things like the AsmPrinter and other things that need
 /// to be parameterized by generic operation hooks.  Most user code should use
 /// the concrete operation types.
 class AbstractOperation {
 public:
-  using OperationProperties = uint32_t;
-
   using GetCanonicalizationPatternsFn = void (*)(OwningRewritePatternList &,
                                                  MLIRContext *);
   using FoldHookFn = LogicalResult (*)(Operation *, ArrayRef<Attribute>,
@@ -146,11 +125,6 @@ class AbstractOperation {
     return getCanonicalizationPatternsFn(results, context);
   }
 
-  /// Returns whether the operation has a particular property.
-  bool hasProperty(OperationProperty property) const {
-    return opProperties & static_cast<OperationProperties>(property);
-  }
-
   /// Returns an instance of the concept object for the given interface if it
   /// was registered to this operation, null otherwise. This should not be used
   /// directly.
@@ -171,33 +145,28 @@ class AbstractOperation {
   /// This constructor is used by Dialect objects when they register the list of
   /// operations they contain.
   template <typename T> static void insert(Dialect &dialect) {
-    insert(T::getOperationName(), dialect, T::getOperationProperties(),
-           TypeID::get<T>(), T::getParseAssemblyFn(), T::getPrintAssemblyFn(),
+    insert(T::getOperationName(), dialect, TypeID::get<T>(),
+           T::getParseAssemblyFn(), T::getPrintAssemblyFn(),
            T::getVerifyInvariantsFn(), T::getFoldHookFn(),
            T::getGetCanonicalizationPatternsFn(), T::getInterfaceMap(),
            T::getHasTraitFn());
   }
 
 private:
-  static void insert(StringRef name, Dialect &dialect,
-                     OperationProperties opProperties, TypeID typeID,
+  static void insert(StringRef name, Dialect &dialect, TypeID typeID,
                      ParseAssemblyFn parseAssembly,
                      PrintAssemblyFn printAssembly,
                      VerifyInvariantsFn verifyInvariants, FoldHookFn foldHook,
                      GetCanonicalizationPatternsFn getCanonicalizationPatterns,
                      detail::InterfaceMap &&interfaceMap, HasTraitFn hasTrait);
 
-  AbstractOperation(StringRef name, Dialect &dialect,
-                    OperationProperties opProperties, TypeID typeID,
+  AbstractOperation(StringRef name, Dialect &dialect, TypeID typeID,
                     ParseAssemblyFn parseAssembly,
                     PrintAssemblyFn printAssembly,
                     VerifyInvariantsFn verifyInvariants, FoldHookFn foldHook,
                     GetCanonicalizationPatternsFn getCanonicalizationPatterns,
                     detail::InterfaceMap &&interfaceMap, HasTraitFn hasTrait);
 
-  /// The properties of the operation.
-  const OperationProperties opProperties;
-
   /// A map of interfaces that were registered to this operation.
   detail::InterfaceMap interfaceMap;
 

diff  --git a/mlir/lib/CAPI/IR/IR.cpp b/mlir/lib/CAPI/IR/IR.cpp
index 87c09944c77c..ba4a985aef45 100644
--- a/mlir/lib/CAPI/IR/IR.cpp
+++ b/mlir/lib/CAPI/IR/IR.cpp
@@ -485,7 +485,7 @@ MlirOperation mlirBlockGetTerminator(MlirBlock block) {
   if (cppBlock->empty())
     return wrap(static_cast<Operation *>(nullptr));
   Operation &back = cppBlock->back();
-  if (!back.isKnownTerminator())
+  if (!back.hasTrait<OpTrait::IsTerminator>())
     return wrap(static_cast<Operation *>(nullptr));
   return wrap(&back);
 }

diff  --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 7cb6bcced292..f1a276f1da98 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -387,7 +387,8 @@ bool mlir::isValidSymbol(Value value, Region *region) {
   if (!defOp) {
     // A block argument that is not a top-level value is a valid symbol if it
     // dominates region's parent op.
-    if (region && !region->getParentOp()->isKnownIsolatedFromAbove())
+    Operation *regionOp = region ? region->getParentOp() : nullptr;
+    if (regionOp && !regionOp->hasTrait<OpTrait::IsIsolatedFromAbove>())
       if (auto *parentOpRegion = region->getParentOp()->getParentRegion())
         return isValidSymbol(value, parentOpRegion);
     return false;
@@ -407,7 +408,8 @@ bool mlir::isValidSymbol(Value value, Region *region) {
     return isDimOpValidSymbol(dimOp, region);
 
   // Check for values dominating `region`'s parent op.
-  if (region && !region->getParentOp()->isKnownIsolatedFromAbove())
+  Operation *regionOp = region ? region->getParentOp() : nullptr;
+  if (regionOp && !regionOp->hasTrait<OpTrait::IsIsolatedFromAbove>())
     if (auto *parentRegion = region->getParentOp()->getParentRegion())
       return isValidSymbol(value, parentRegion);
 

diff  --git a/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp b/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp
index 1131e6ecec22..c8cf3dd9475a 100644
--- a/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp
@@ -185,7 +185,8 @@ AffineDataCopyGeneration::runOnBlock(Block *block,
   // Generate the copy for the final block range.
   if (curBegin != block->end()) {
     // Can't be a terminator because it would have been skipped above.
-    assert(!curBegin->isKnownTerminator() && "can't be a terminator");
+    assert(!curBegin->hasTrait<OpTrait::IsTerminator>() &&
+           "can't be a terminator");
     // Exclude the affine.yield - hence, the std::prev.
     affineDataCopyGenerate(/*begin=*/curBegin, /*end=*/std::prev(block->end()),
                            copyOptions, /*filterMemRef=*/llvm::None, copyNests);

diff  --git a/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp b/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp
index 37e0b9373d42..fe4395f26859 100644
--- a/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp
+++ b/mlir/lib/Dialect/GPU/Transforms/AsyncRegionRewriter.cpp
@@ -34,7 +34,9 @@ class GpuAsyncRegionPass : public GpuAsyncRegionPassBase<GpuAsyncRegionPass> {
 };
 } // namespace
 
-static bool isTerminator(Operation *op) { return !op->isKnownNonTerminator(); }
+static bool isTerminator(Operation *op) {
+  return op->mightHaveTrait<OpTrait::IsTerminator>();
+}
 static bool hasSideEffects(Operation *op) {
   return !MemoryEffectOpInterface::hasNoEffect(op);
 }

diff  --git a/mlir/lib/Dialect/StandardOps/Transforms/FuncBufferize.cpp b/mlir/lib/Dialect/StandardOps/Transforms/FuncBufferize.cpp
index 1aace4517f71..c63150f3ab87 100644
--- a/mlir/lib/Dialect/StandardOps/Transforms/FuncBufferize.cpp
+++ b/mlir/lib/Dialect/StandardOps/Transforms/FuncBufferize.cpp
@@ -51,7 +51,7 @@ struct FuncBufferizePass : public FuncBufferizeBase<FuncBufferizePass> {
     // implement the trait or interface, mark them as illegal no matter what.
     target.markUnknownOpDynamicallyLegal([&](Operation *op) {
       // If it is not a terminator, ignore it.
-      if (op->isKnownNonTerminator())
+      if (!op->mightHaveTrait<OpTrait::IsTerminator>())
         return true;
       // If it is not the last operation in the block, also ignore it. We do
       // this to handle unknown operations, as well.

diff  --git a/mlir/lib/EDSC/Builders.cpp b/mlir/lib/EDSC/Builders.cpp
index 21a6b922d91f..5f40149040cc 100644
--- a/mlir/lib/EDSC/Builders.cpp
+++ b/mlir/lib/EDSC/Builders.cpp
@@ -85,7 +85,7 @@ void mlir::edsc::appendToBlock(Block *block,
   OpBuilder &builder = ScopedContext::getBuilderRef();
 
   OpBuilder::InsertionGuard guard(builder);
-  if (block->empty() || block->back().isKnownNonTerminator())
+  if (block->empty() || !block->back().mightHaveTrait<OpTrait::IsTerminator>())
     builder.setInsertionPointToEnd(block);
   else
     builder.setInsertionPoint(&block->back());

diff  --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index 4f4778dcf408..437d353655ff 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -878,7 +878,7 @@ void SSANameState::shadowRegionArgs(Region &region, ValueRange namesToUse) {
   assert(!region.empty() && "cannot shadow arguments of an empty region");
   assert(region.getNumArguments() == namesToUse.size() &&
          "incorrect number of names passed in");
-  assert(region.getParentOp()->isKnownIsolatedFromAbove() &&
+  assert(region.getParentOp()->hasTrait<OpTrait::IsIsolatedFromAbove>() &&
          "only KnownIsolatedFromAbove ops can shadow names");
 
   SmallVector<char, 16> nameStr;
@@ -2646,22 +2646,22 @@ void Operation::print(raw_ostream &os, OpPrintingFlags flags) {
   }
 
   // Find the operation to number from based upon the provided flags.
-  Operation *printedOp = this;
+  Operation *op = this;
   bool shouldUseLocalScope = flags.shouldUseLocalScope();
   do {
     // If we are printing local scope, stop at the first operation that is
     // isolated from above.
-    if (shouldUseLocalScope && printedOp->isKnownIsolatedFromAbove())
+    if (shouldUseLocalScope && op->hasTrait<OpTrait::IsIsolatedFromAbove>())
       break;
 
     // Otherwise, traverse up to the next parent.
-    Operation *parentOp = printedOp->getParentOp();
+    Operation *parentOp = op->getParentOp();
     if (!parentOp)
       break;
-    printedOp = parentOp;
+    op = parentOp;
   } while (true);
 
-  AsmState state(printedOp);
+  AsmState state(op);
   print(os, state, flags);
 }
 void Operation::print(raw_ostream &os, AsmState &state, OpPrintingFlags flags) {

diff  --git a/mlir/lib/IR/Block.cpp b/mlir/lib/IR/Block.cpp
index 79e7daa12a7c..74209bd0884b 100644
--- a/mlir/lib/IR/Block.cpp
+++ b/mlir/lib/IR/Block.cpp
@@ -214,7 +214,7 @@ BlockArgument Block::insertArgument(args_iterator it, Type type) {
 /// Get the terminator operation of this block. This function asserts that
 /// the block has a valid terminator operation.
 Operation *Block::getTerminator() {
-  assert(!empty() && !back().isKnownNonTerminator());
+  assert(!empty() && back().mightHaveTrait<OpTrait::IsTerminator>());
   return &back();
 }
 

diff  --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 8d13a9c4af32..f56eb75390a9 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -659,15 +659,14 @@ const AbstractOperation *AbstractOperation::lookup(StringRef opName,
 }
 
 void AbstractOperation::insert(
-    StringRef name, Dialect &dialect, OperationProperties opProperties,
-    TypeID typeID, ParseAssemblyFn parseAssembly, PrintAssemblyFn printAssembly,
+    StringRef name, Dialect &dialect, TypeID typeID,
+    ParseAssemblyFn parseAssembly, PrintAssemblyFn printAssembly,
     VerifyInvariantsFn verifyInvariants, FoldHookFn foldHook,
     GetCanonicalizationPatternsFn getCanonicalizationPatterns,
     detail::InterfaceMap &&interfaceMap, HasTraitFn hasTrait) {
-  AbstractOperation opInfo(name, dialect, opProperties, typeID, parseAssembly,
-                           printAssembly, verifyInvariants, foldHook,
-                           getCanonicalizationPatterns, std::move(interfaceMap),
-                           hasTrait);
+  AbstractOperation opInfo(
+      name, dialect, typeID, parseAssembly, printAssembly, verifyInvariants,
+      foldHook, getCanonicalizationPatterns, std::move(interfaceMap), hasTrait);
 
   auto &impl = dialect.getContext()->getImpl();
   assert(impl.multiThreadedExecutionContext == 0 &&
@@ -681,14 +680,14 @@ void AbstractOperation::insert(
 }
 
 AbstractOperation::AbstractOperation(
-    StringRef name, Dialect &dialect, OperationProperties opProperties,
-    TypeID typeID, ParseAssemblyFn parseAssembly, PrintAssemblyFn printAssembly,
+    StringRef name, Dialect &dialect, TypeID typeID,
+    ParseAssemblyFn parseAssembly, PrintAssemblyFn printAssembly,
     VerifyInvariantsFn verifyInvariants, FoldHookFn foldHook,
     GetCanonicalizationPatternsFn getCanonicalizationPatterns,
     detail::InterfaceMap &&interfaceMap, HasTraitFn hasTrait)
     : name(Identifier::get(name, dialect.getContext())), dialect(dialect),
-      typeID(typeID), opProperties(opProperties),
-      interfaceMap(std::move(interfaceMap)), foldHookFn(foldHook),
+      typeID(typeID), interfaceMap(std::move(interfaceMap)),
+      foldHookFn(foldHook),
       getCanonicalizationPatternsFn(getCanonicalizationPatterns),
       hasTraitFn(hasTrait), parseAssemblyFn(parseAssembly),
       printAssemblyFn(printAssembly), verifyInvariantsFn(verifyInvariants) {}

diff  --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index be312689cebb..f1c40b03fdfc 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -139,7 +139,7 @@ Operation *Operation::create(Location location, OperationName name,
       ::new (rawMem) Operation(location, name, resultTypes, numSuccessors,
                                numRegions, attributes, needsOperandStorage);
 
-  assert((numSuccessors == 0 || !op->isKnownNonTerminator()) &&
+  assert((numSuccessors == 0 || op->mightHaveTrait<OpTrait::IsTerminator>()) &&
          "unexpected successors in a non-terminator operation");
 
   // Initialize the results.
@@ -1286,7 +1286,7 @@ void impl::ensureRegionTerminator(
     builder.createBlock(&region);
 
   Block &block = region.back();
-  if (!block.empty() && block.back().isKnownTerminator())
+  if (!block.empty() && block.back().hasTrait<OpTrait::IsTerminator>())
     return;
 
   builder.setInsertionPointToEnd(&block);

diff  --git a/mlir/lib/IR/Verifier.cpp b/mlir/lib/IR/Verifier.cpp
index 036919bd84b9..9b047e7e0e25 100644
--- a/mlir/lib/IR/Verifier.cpp
+++ b/mlir/lib/IR/Verifier.cpp
@@ -145,9 +145,10 @@ LogicalResult OperationVerifier::verifyBlock(Block &block) {
   }
 
   // Verify the terminator.
-  if (failed(verifyOperation(block.back())))
+  Operation &terminator = block.back();
+  if (failed(verifyOperation(terminator)))
     return failure();
-  if (block.back().isKnownNonTerminator())
+  if (!terminator.mightHaveTrait<OpTrait::IsTerminator>())
     return block.back().emitError("block with no terminator");
 
   // Verify that this block is not branching to a block of a 
diff erent

diff  --git a/mlir/lib/Interfaces/SideEffectInterfaces.cpp b/mlir/lib/Interfaces/SideEffectInterfaces.cpp
index 89b2b34e7604..c02bc0c9f588 100644
--- a/mlir/lib/Interfaces/SideEffectInterfaces.cpp
+++ b/mlir/lib/Interfaces/SideEffectInterfaces.cpp
@@ -91,7 +91,7 @@ static bool wouldOpBeTriviallyDeadImpl(Operation *rootOp) {
 }
 
 bool mlir::wouldOpBeTriviallyDead(Operation *op) {
-  if (!op->isKnownNonTerminator())
+  if (op->mightHaveTrait<OpTrait::IsTerminator>())
     return false;
   return wouldOpBeTriviallyDeadImpl(op);
 }

diff  --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index 58ed9004c56c..19a00295a163 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -808,7 +808,7 @@ Operation *OperationParser::parseGenericOperation() {
   if (getToken().is(Token::l_square)) {
     // Check if the operation is a known terminator.
     const AbstractOperation *abstractOp = result.name.getAbstractOperation();
-    if (abstractOp && !abstractOp->hasProperty(OperationProperty::Terminator))
+    if (abstractOp && !abstractOp->hasTrait<OpTrait::IsTerminator>())
       return emitError("successors in non-terminator"), nullptr;
 
     SmallVector<Block *, 2> successors;
@@ -1448,7 +1448,7 @@ class CustomOpAsmParser : public OpAsmParser {
 
     // Try to parse the region.
     assert((!enableNameShadowing ||
-            opDefinition->hasProperty(OperationProperty::IsolatedFromAbove)) &&
+            opDefinition->hasTrait<OpTrait::IsIsolatedFromAbove>()) &&
            "name shadowing is only allowed on isolated regions");
     if (parser.parseRegion(region, regionArguments, enableNameShadowing))
       return failure();

diff  --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index a78adde2e941..d9f2e7f23508 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -357,11 +357,10 @@ void OpPassManager::initialize(MLIRContext *context,
 LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
                                      AnalysisManager am, bool verifyPasses,
                                      unsigned parentInitGeneration) {
-  if (!op->getName().getAbstractOperation())
+  if (!op->isRegistered())
     return op->emitOpError()
            << "trying to schedule a pass on an unregistered operation";
-  if (!op->getName().getAbstractOperation()->hasProperty(
-          OperationProperty::IsolatedFromAbove))
+  if (!op->hasTrait<OpTrait::IsIsolatedFromAbove>())
     return op->emitOpError() << "trying to schedule a pass on an operation not "
                                 "marked as 'IsolatedFromAbove'";
 

diff  --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index f9b714738ffe..4924d4bed6d8 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1155,7 +1155,8 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
 
 LogicalResult ModuleTranslation::checkSupportedModuleOps(Operation *m) {
   for (Operation &o : getModuleBody(m).getOperations())
-    if (!isa<LLVM::LLVMFuncOp, LLVM::GlobalOp>(&o) && !o.isKnownTerminator())
+    if (!isa<LLVM::LLVMFuncOp, LLVM::GlobalOp>(&o) &&
+        !o.hasTrait<OpTrait::IsTerminator>())
       return o.emitOpError("unsupported module-level operation");
   return success();
 }

diff  --git a/mlir/lib/Transforms/CSE.cpp b/mlir/lib/Transforms/CSE.cpp
index f9d6ee76a041..57f39041b11f 100644
--- a/mlir/lib/Transforms/CSE.cpp
+++ b/mlir/lib/Transforms/CSE.cpp
@@ -90,7 +90,7 @@ struct CSE : public CSEBase<CSE> {
 /// Attempt to eliminate a redundant operation.
 LogicalResult CSE::simplifyOperation(ScopedMapTy &knownValues, Operation *op) {
   // Don't simplify terminator operations.
-  if (op->isKnownTerminator())
+  if (op->hasTrait<OpTrait::IsTerminator>())
     return failure();
 
   // If the operation is already trivially dead just add it to the erase list.
@@ -145,7 +145,7 @@ void CSE::simplifyBlock(ScopedMapTy &knownValues, DominanceInfo &domInfo,
     // If this operation is isolated above, we can't process nested regions with
     // the given 'knownValues' map. This would cause the insertion of implicit
     // captures in explicit capture only regions.
-    if (!inst.isRegistered() || inst.isKnownIsolatedFromAbove()) {
+    if (inst.mightHaveTrait<OpTrait::IsIsolatedFromAbove>()) {
       ScopedMapTy nestedKnownValues;
       for (auto &region : inst.getRegions())
         simplifyRegion(nestedKnownValues, domInfo, region);

diff  --git a/mlir/lib/Transforms/Inliner.cpp b/mlir/lib/Transforms/Inliner.cpp
index 4beb7e7360a5..d79af21cb4ac 100644
--- a/mlir/lib/Transforms/Inliner.cpp
+++ b/mlir/lib/Transforms/Inliner.cpp
@@ -414,7 +414,7 @@ struct Inliner : public InlinerInterface {
 static bool shouldInline(ResolvedCall &resolvedCall) {
   // Don't allow inlining terminator calls. We currently don't support this
   // case.
-  if (resolvedCall.call->isKnownTerminator())
+  if (resolvedCall.call->hasTrait<OpTrait::IsTerminator>())
     return false;
 
   // Don't allow inlining if the target is an ancestor of the call. This
@@ -654,7 +654,7 @@ LogicalResult InlinerPass::optimizeSCC(CallGraph &cg, CGUseList &useList,
     // We also won't apply simplifications to nodes that can't have passes
     // scheduled on them.
     auto *region = node->getCallableRegion();
-    if (!region->getParentOp()->isKnownIsolatedFromAbove())
+    if (!region->getParentOp()->hasTrait<OpTrait::IsIsolatedFromAbove>())
       continue;
     nodesToVisit.push_back(node);
   }

diff  --git a/mlir/lib/Transforms/SCCP.cpp b/mlir/lib/Transforms/SCCP.cpp
index ec550d9d7dd2..a09403510da2 100644
--- a/mlir/lib/Transforms/SCCP.cpp
+++ b/mlir/lib/Transforms/SCCP.cpp
@@ -487,7 +487,7 @@ void SCCPSolver::visitOperation(Operation *op) {
   }
 
   // If this is a terminator operation, process any control flow lattice state.
-  if (op->isKnownTerminator())
+  if (op->hasTrait<OpTrait::IsTerminator>())
     visitTerminatorOperation(op, operandConstants);
 
   // Process call operations. The call visitor processes result values, so we

diff  --git a/mlir/lib/Transforms/Utils/FoldUtils.cpp b/mlir/lib/Transforms/Utils/FoldUtils.cpp
index 52eee2cb5d2d..024ae1892861 100644
--- a/mlir/lib/Transforms/Utils/FoldUtils.cpp
+++ b/mlir/lib/Transforms/Utils/FoldUtils.cpp
@@ -30,7 +30,7 @@ getInsertionRegion(DialectInterfaceCollection<DialectFoldInterface> &interfaces,
     //  * The parent is unregistered, or is known to be isolated from above.
     //  * The parent is a top-level operation.
     auto *parentOp = region->getParentOp();
-    if (!parentOp->isRegistered() || parentOp->isKnownIsolatedFromAbove() ||
+    if (parentOp->mightHaveTrait<OpTrait::IsIsolatedFromAbove>() ||
         !parentOp->getBlock())
       return region;
 
@@ -182,7 +182,7 @@ LogicalResult OperationFolder::tryToFold(
   SmallVector<OpFoldResult, 8> foldResults;
 
   // If this is a commutative operation, move constants to be trailing operands.
-  if (op->getNumOperands() >= 2 && op->isCommutative()) {
+  if (op->getNumOperands() >= 2 && op->hasTrait<OpTrait::IsCommutative>()) {
     std::stable_partition(
         op->getOpOperands().begin(), op->getOpOperands().end(),
         [&](OpOperand &O) { return !matchPattern(O.get(), m_Constant()); });

diff  --git a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
index c1d286690c48..9ed3b3514db6 100644
--- a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
+++ b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
@@ -247,7 +247,7 @@ mlir::applyPatternsAndFoldGreedily(MutableArrayRef<Region> regions,
   // prevent performing canonicalizations on operations defined at or above
   // the region containing 'op'.
   auto regionIsIsolated = [](Region &region) {
-    return region.getParentOp()->isKnownIsolatedFromAbove();
+    return region.getParentOp()->hasTrait<OpTrait::IsIsolatedFromAbove>();
   };
   (void)regionIsIsolated;
   assert(llvm::all_of(regions, regionIsIsolated) &&

diff  --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index dd2de96e0118..125d32b782c9 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -173,7 +173,7 @@ static bool isUseSpeciallyKnownDead(OpOperand &use, LiveMap &liveMap) {
   // And similarly, because each successor operand is really an operand to a phi
   // node, rather than to the terminator op itself, a terminator op can't e.g.
   // "print" the value of a successor operand.
-  if (owner->isKnownTerminator()) {
+  if (owner->hasTrait<OpTrait::IsTerminator>()) {
     if (BranchOpInterface branchInterface = dyn_cast<BranchOpInterface>(owner))
       if (auto arg = branchInterface.getSuccessorBlockArgument(operandIndex))
         return !liveMap.wasProvenLive(*arg);
@@ -194,7 +194,7 @@ static void processValue(Value value, LiveMap &liveMap) {
 
 static bool isOpIntrinsicallyLive(Operation *op) {
   // This pass doesn't modify the CFG, so terminators are never deleted.
-  if (!op->isKnownNonTerminator())
+  if (op->mightHaveTrait<OpTrait::IsTerminator>())
     return true;
   // If the op has a side effect, we treat it as live.
   // TODO: Properly handle region side effects.
@@ -234,7 +234,7 @@ static void propagateLiveness(Operation *op, LiveMap &liveMap) {
     propagateLiveness(region, liveMap);
 
   // Process terminator operations.
-  if (op->isKnownTerminator())
+  if (op->hasTrait<OpTrait::IsTerminator>())
     return propagateTerminatorLiveness(op, liveMap);
 
   // Process the op itself.

diff  --git a/mlir/test/lib/Dialect/Shape/TestShapeFunctions.cpp b/mlir/test/lib/Dialect/Shape/TestShapeFunctions.cpp
index a8851832d881..54ef66e48980 100644
--- a/mlir/test/lib/Dialect/Shape/TestShapeFunctions.cpp
+++ b/mlir/test/lib/Dialect/Shape/TestShapeFunctions.cpp
@@ -29,7 +29,7 @@ void ReportShapeFnPass::runOnOperation() {
   // Report the shape function available to refine the op.
   auto shapeFnId = Identifier::get("shape.function", &getContext());
   auto remarkShapeFn = [&](shape::FunctionLibraryOp shapeFnLib, Operation *op) {
-    if (op->isKnownTerminator())
+    if (op->hasTrait<OpTrait::IsTerminator>())
       return true;
     if (auto typeInterface = dyn_cast<InferTypeOpInterface>(op)) {
       op->emitRemark() << "implements InferType op interface";

diff  --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index 740d3712df65..277f6ccdc283 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -1017,7 +1017,7 @@ struct TestSelectiveOpReplacementPattern : public OpRewritePattern<TestCastOp> {
 
     // Replace non-terminator uses with the first operand.
     rewriter.replaceOpWithIf(op, operands[0], [](OpOperand &operand) {
-      return operand.getOwner()->isKnownTerminator();
+      return operand.getOwner()->hasTrait<OpTrait::IsTerminator>();
     });
     // Replace everything else with the second operand if the operation isn't
     // dead.

diff  --git a/mlir/test/lib/Transforms/TestDynamicPipeline.cpp b/mlir/test/lib/Transforms/TestDynamicPipeline.cpp
index d1295c96d51b..98e30b9a6d23 100644
--- a/mlir/test/lib/Transforms/TestDynamicPipeline.cpp
+++ b/mlir/test/lib/Transforms/TestDynamicPipeline.cpp
@@ -72,7 +72,7 @@ class TestDynamicPipelinePass
     if (runOnNestedOp) {
       llvm::errs() << "Run on nested op\n";
       currentOp->walk([&](Operation *op) {
-        if (op == currentOp || !op->isKnownIsolatedFromAbove() ||
+        if (op == currentOp || !op->hasTrait<OpTrait::IsIsolatedFromAbove>() ||
             op->getName() != currentOp->getName())
           return;
         llvm::errs() << "Run on " << *op << "\n";

diff  --git a/mlir/test/lib/Transforms/TestOpaqueLoc.cpp b/mlir/test/lib/Transforms/TestOpaqueLoc.cpp
index 1d16ab7c7175..817e1527d76b 100644
--- a/mlir/test/lib/Transforms/TestOpaqueLoc.cpp
+++ b/mlir/test/lib/Transforms/TestOpaqueLoc.cpp
@@ -44,7 +44,7 @@ struct TestOpaqueLoc
       op->setLoc(
           OpaqueLoc::get<MyLocation *>(myLocs.back().get(), &getContext()));
 
-      if (isa<FuncOp>(op) || op->isKnownTerminator())
+      if (isa<FuncOp>(op) || op->hasTrait<OpTrait::IsTerminator>())
         return;
 
       OpBuilder builder(op);


        


More information about the Mlir-commits mailing list