[Mlir-commits] [mlir] bf6c46d - [MLIR] NFC Clean up doc comments on memref replacement utility

[Uday Bondhugula]

Author: Uday Bondhugula
Date: 2021-07-31T15:14:52+05:30
New Revision: bf6c46d9173b973fa30ebb03e4204f87a1af6b2d

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

LOG: [MLIR] NFC Clean up doc comments on memref replacement utility

NFC. Clean up stale doc comments on memref replacement utility and some
variable renaming in it to avoid confusion.

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

Added: 
    

Modified: 
    mlir/include/mlir/Transforms/Utils.h
    mlir/lib/Transforms/Utils/Utils.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Transforms/Utils.h b/mlir/include/mlir/Transforms/Utils.h
index 8bbde7c15b344..bac651909a169 100644
--- a/mlir/include/mlir/Transforms/Utils.h
+++ b/mlir/include/mlir/Transforms/Utils.h
@@ -45,13 +45,14 @@ class AllocOp;
 /// correspond to memref's indices, and its symbolic inputs if any should be
 /// provided in `symbolOperands`.
 ///
-/// `domInstFilter`, if non-null, restricts the replacement to only those
-/// operations that are dominated by the former; similarly, `postDomInstFilter`
+/// `domOpFilter`, if non-null, restricts the replacement to only those
+/// operations that are dominated by the former; similarly, `postDomOpFilter`
 /// restricts replacement to only those operations that are postdominated by it.
 ///
 /// 'allowNonDereferencingOps', if set, allows replacement of non-dereferencing
-/// uses of a memref without any requirement for access index rewrites. The
-/// default value of this flag variable is false.
+/// uses of a memref without any requirement for access index rewrites as long
+/// as the user operation has the MemRefsNormalizable trait. The default value
+/// of this flag is false.
 ///
 /// 'replaceInDeallocOp', if set, lets DeallocOp, a non-dereferencing user, to
 /// also be a candidate for replacement. The default value of this flag is
@@ -73,9 +74,9 @@ class AllocOp;
 LogicalResult replaceAllMemRefUsesWith(
     Value oldMemRef, Value newMemRef, ArrayRef<Value> extraIndices = {},
     AffineMap indexRemap = AffineMap(), ArrayRef<Value> extraOperands = {},
-    ArrayRef<Value> symbolOperands = {}, Operation *domInstFilter = nullptr,
-    Operation *postDomInstFilter = nullptr,
-    bool allowNonDereferencingOps = false, bool replaceInDeallocOp = false);
+    ArrayRef<Value> symbolOperands = {}, Operation *domOpFilter = nullptr,
+    Operation *postDomOpFilter = nullptr, bool allowNonDereferencingOps = false,
+    bool replaceInDeallocOp = false);
 
 /// Performs the same replacement as the other version above but only for the
 /// dereferencing uses of `oldMemRef` in `op`, except in cases where

diff  --git a/mlir/lib/Transforms/Utils/Utils.cpp b/mlir/lib/Transforms/Utils/Utils.cpp
index ce514a887dfb9..fdff3160e95f6 100644
--- a/mlir/lib/Transforms/Utils/Utils.cpp
+++ b/mlir/lib/Transforms/Utils/Utils.cpp
@@ -23,6 +23,9 @@
 #include "mlir/Support/MathExtras.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/TypeSwitch.h"
+
+#define DEBUG_TYPE "transforms-utils"
+
 using namespace mlir;
 
 // Perform the replacement in `op`.
@@ -207,8 +210,8 @@ LogicalResult mlir::replaceAllMemRefUsesWith(Value oldMemRef, Value newMemRef,
 LogicalResult mlir::replaceAllMemRefUsesWith(
     Value oldMemRef, Value newMemRef, ArrayRef<Value> extraIndices,
     AffineMap indexRemap, ArrayRef<Value> extraOperands,
-    ArrayRef<Value> symbolOperands, Operation *domInstFilter,
-    Operation *postDomInstFilter, bool allowNonDereferencingOps,
+    ArrayRef<Value> symbolOperands, Operation *domOpFilter,
+    Operation *postDomOpFilter, bool allowNonDereferencingOps,
     bool replaceInDeallocOp) {
   unsigned newMemRefRank = newMemRef.getType().cast<MemRefType>().getRank();
   (void)newMemRefRank; // unused in opt mode
@@ -230,25 +233,25 @@ LogicalResult mlir::replaceAllMemRefUsesWith(
 
   std::unique_ptr<DominanceInfo> domInfo;
   std::unique_ptr<PostDominanceInfo> postDomInfo;
-  if (domInstFilter)
-    domInfo = std::make_unique<DominanceInfo>(
-        domInstFilter->getParentOfType<FuncOp>());
+  if (domOpFilter)
+    domInfo =
+        std::make_unique<DominanceInfo>(domOpFilter->getParentOfType<FuncOp>());
 
-  if (postDomInstFilter)
+  if (postDomOpFilter)
     postDomInfo = std::make_unique<PostDominanceInfo>(
-        postDomInstFilter->getParentOfType<FuncOp>());
+        postDomOpFilter->getParentOfType<FuncOp>());
 
   // Walk all uses of old memref; collect ops to perform replacement. We use a
   // DenseSet since an operation could potentially have multiple uses of a
   // memref (although rare), and the replacement later is going to erase ops.
   DenseSet<Operation *> opsToReplace;
   for (auto *op : oldMemRef.getUsers()) {
-    // Skip this use if it's not dominated by domInstFilter.
-    if (domInstFilter && !domInfo->dominates(domInstFilter, op))
+    // Skip this use if it's not dominated by domOpFilter.
+    if (domOpFilter && !domInfo->dominates(domOpFilter, op))
       continue;
 
-    // Skip this use if it's not post-dominated by postDomInstFilter.
-    if (postDomInstFilter && !postDomInfo->postDominates(postDomInstFilter, op))
+    // Skip this use if it's not post-dominated by postDomOpFilter.
+    if (postDomOpFilter && !postDomInfo->postDominates(postDomOpFilter, op))
       continue;
 
     // Skip dealloc's - no replacement is necessary, and a memref replacement
@@ -260,13 +263,20 @@ LogicalResult mlir::replaceAllMemRefUsesWith(
     // for the memref to be used in a non-dereferencing way outside of the
     // region where this replacement is happening.
     if (!isa<AffineMapAccessInterface>(*op)) {
-      if (!allowNonDereferencingOps)
+      if (!allowNonDereferencingOps) {
+        LLVM_DEBUG(llvm::dbgs()
+                   << "Memref replacement failed: non-deferencing memref op: \n"
+                   << *op << '\n');
         return failure();
-      // Currently we support the following non-dereferencing ops to be a
-      // candidate for replacement: Dealloc, CallOp and ReturnOp.
-      // TODO: Add support for other kinds of ops.
-      if (!op->hasTrait<OpTrait::MemRefsNormalizable>())
+      }
+      // Non-dereferencing ops with the MemRefsNormalizable trait are
+      // supported for replacement.
+      if (!op->hasTrait<OpTrait::MemRefsNormalizable>()) {
+        LLVM_DEBUG(llvm::dbgs() << "Memref replacement failed: use without a "
+                                   "memrefs normalizable trait: \n"
+                                << *op << '\n');
         return failure();
+      }
     }
 
     // We'll first collect and then replace --- since replacement erases the op
@@ -661,8 +671,8 @@ LogicalResult mlir::normalizeMemRef(memref::AllocOp *allocOp) {
                                       /*indexRemap=*/layoutMap,
                                       /*extraOperands=*/{},
                                       /*symbolOperands=*/symbolOperands,
-                                      /*domInstFilter=*/nullptr,
-                                      /*postDomInstFilter=*/nullptr,
+                                      /*domOpFilter=*/nullptr,
+                                      /*postDomOpFilter=*/nullptr,
                                       /*allowDereferencingOps=*/true))) {
     // If it failed (due to escapes for example), bail out.
     newAlloc.erase();