[Mlir-commits] [mlir] b28e3db - Merge OpFolderDialectInterface with DialectFoldInterface (NFC)

[Mehdi Amini]

Author: Mehdi Amini
Date: 2020-08-13T00:39:22Z
New Revision: b28e3db88d053b94250562f837758dc8a92a0a0f

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

LOG: Merge OpFolderDialectInterface with DialectFoldInterface (NFC)

Reviewed By: rriddle

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

Added: 
    

Modified: 
    mlir/docs/Canonicalization.md
    mlir/include/mlir/Interfaces/FoldInterfaces.h
    mlir/include/mlir/Transforms/FoldUtils.h
    mlir/lib/Transforms/Utils/FoldUtils.cpp
    mlir/test/lib/Dialect/Test/TestDialect.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/docs/Canonicalization.md b/mlir/docs/Canonicalization.md
index 8434337ee486..3e1c9d11ecab 100644
--- a/mlir/docs/Canonicalization.md
+++ b/mlir/docs/Canonicalization.md
@@ -51,7 +51,7 @@ These transformations are applied to all levels of IR:
 *   `constant-like` operations are uniqued and hoisted into the entry block of
     the first parent barrier region. This is a region that is either isolated
     from above, e.g. the entry block of a function, or one marked as a barrier
-    via the `shouldMaterializeInto` method on the `OpFolderDialectInterface`.
+    via the `shouldMaterializeInto` method on the `DialectFoldInterface`.
 
 ## Defining Canonicalizations
 
@@ -170,6 +170,10 @@ generate new `Value`s. There are no specific restrictions on the form of the
 `Attribute` value returned, but it is important to ensure that the `Attribute`
 representation of a specific `Type` is consistent.
 
+When the `fold` hook on an operation is not successful, the dialect can
+provide a fallback by implementing the `DialectFoldInterface` and overriding
+the fold hook.
+
 #### Generating Constants from Attributes
 
 When a `fold` method returns an `Attribute` as the result, it signifies that

diff  --git a/mlir/include/mlir/Interfaces/FoldInterfaces.h b/mlir/include/mlir/Interfaces/FoldInterfaces.h
index e1f17872e6eb..1399d9a56415 100644
--- a/mlir/include/mlir/Interfaces/FoldInterfaces.h
+++ b/mlir/include/mlir/Interfaces/FoldInterfaces.h
@@ -15,9 +15,10 @@
 namespace mlir {
 class Attribute;
 class OpFoldResult;
+class Region;
 
-/// Define a fold interface to allow for dialects to opt-in specific
-/// folding for operations they define.
+/// Define a fold interface to allow for dialects to control specific aspects
+/// of the folding behavior for operations they define.
 class DialectFoldInterface
     : public DialectInterface::Base<DialectFoldInterface> {
 public:
@@ -33,6 +34,13 @@ class DialectFoldInterface
                              SmallVectorImpl<OpFoldResult> &results) const {
     return failure();
   }
+
+  /// Registered hook to check if the given region, which is attached to an
+  /// operation that is *not* isolated from above, should be used when
+  /// materializing constants. The folder will generally materialize constants
+  /// into the top-level isolated region, this allows for materializing into a
+  /// lower level ancestor region if it is more profitable/correct.
+  virtual bool shouldMaterializeInto(Region *region) const { return false; }
 };
 
 } // end namespace mlir

diff  --git a/mlir/include/mlir/Transforms/FoldUtils.h b/mlir/include/mlir/Transforms/FoldUtils.h
index d427f0b2406d..ad406cb18085 100644
--- a/mlir/include/mlir/Transforms/FoldUtils.h
+++ b/mlir/include/mlir/Transforms/FoldUtils.h
@@ -17,29 +17,12 @@
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/Dialect.h"
 #include "mlir/IR/DialectInterface.h"
+#include "mlir/Interfaces/FoldInterfaces.h"
 
 namespace mlir {
 class Operation;
 class Value;
 
-//===--------------------------------------------------------------------===//
-// Operation Folding Interface
-//===--------------------------------------------------------------------===//
-
-/// This class defines a dialect interface used to assist the operation folder.
-/// It provides hooks for materializing and folding operations.
-class OpFolderDialectInterface
-    : public DialectInterface::Base<OpFolderDialectInterface> {
-public:
-  OpFolderDialectInterface(Dialect *dialect) : Base(dialect) {}
-
-  /// Registered hook to check if the given region, which is attached to an
-  /// operation that is *not* isolated from above, should be used when
-  /// materializing constants. The folder will generally materialize constants
-  /// into the top-level isolated region, this allows for materializing into a
-  /// lower level ancestor region if it is more profitable/correct.
-  virtual bool shouldMaterializeInto(Region *region) const { return false; }
-};
 
 //===--------------------------------------------------------------------===//
 // OperationFolder
@@ -153,7 +136,7 @@ class OperationFolder {
   DenseMap<Operation *, SmallVector<Dialect *, 2>> referencedDialects;
 
   /// A collection of dialect folder interfaces.
-  DialectInterfaceCollection<OpFolderDialectInterface> interfaces;
+  DialectInterfaceCollection<DialectFoldInterface> interfaces;
 };
 
 } // end namespace mlir

diff  --git a/mlir/lib/Transforms/Utils/FoldUtils.cpp b/mlir/lib/Transforms/Utils/FoldUtils.cpp
index a28e65d53b1a..074f71c92fff 100644
--- a/mlir/lib/Transforms/Utils/FoldUtils.cpp
+++ b/mlir/lib/Transforms/Utils/FoldUtils.cpp
@@ -22,9 +22,9 @@ using namespace mlir;
 
 /// Given an operation, find the parent region that folded constants should be
 /// inserted into.
-static Region *getInsertionRegion(
-    DialectInterfaceCollection<OpFolderDialectInterface> &interfaces,
-    Block *insertionBlock) {
+static Region *
+getInsertionRegion(DialectInterfaceCollection<DialectFoldInterface> &interfaces,
+                   Block *insertionBlock) {
   while (Region *region = insertionBlock->getParent()) {
     // Insert in this region for any of the following scenarios:
     //  * The parent is unregistered, or is known to be isolated from above.

diff  --git a/mlir/test/lib/Dialect/Test/TestDialect.cpp b/mlir/test/lib/Dialect/Test/TestDialect.cpp
index c9cfdc5ff415..c873a009f151 100644
--- a/mlir/test/lib/Dialect/Test/TestDialect.cpp
+++ b/mlir/test/lib/Dialect/Test/TestDialect.cpp
@@ -52,8 +52,8 @@ struct TestOpAsmInterface : public OpAsmDialectInterface {
   }
 };
 
-struct TestOpFolderDialectInterface : public OpFolderDialectInterface {
-  using OpFolderDialectInterface::OpFolderDialectInterface;
+struct TestDialectFoldInterface : public DialectFoldInterface {
+  using DialectFoldInterface::DialectFoldInterface;
 
   /// Registered hook to check if the given region, which is attached to an
   /// operation that is *not* isolated from above, should be used when
@@ -135,7 +135,7 @@ void TestDialect::initialize() {
 #define GET_OP_LIST
 #include "TestOps.cpp.inc"
       >();
-  addInterfaces<TestOpAsmInterface, TestOpFolderDialectInterface,
+  addInterfaces<TestOpAsmInterface, TestDialectFoldInterface,
                 TestInlinerInterface>();
   addTypes<TestType, TestRecursiveType>();
   allowUnknownOperations();