[mlir][vector][nfc] Replace failure() with notifyMatchFailure()

[banach-space]

Updates some instances of plain return failure(); in VectorToSCF.cpp
with return notifyMatchFailure(); and a description (usually copied
from the nearby comment).

There's many more "plain" return failure(); left, but I ATM I only
have the cycles for the ones updated here.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

Updates some instances of plain return failure(); in VectorToSCF.cpp
with return notifyMatchFailure(); and a description (usually copied
from the nearby comment).

There's many more "plain" return failure(); left, but I ATM I only
have the cycles for the ones updated here.

---

Full diff: https://github.com/llvm/llvm-project/pull/129278.diff

1 Files Affected:

• (modified) mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp (+20-17)

diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index 22bf27d229ce5..52c99ec664044 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -549,22 +549,25 @@ struct Strategy<TransferWriteOp> {
 };
 
 template <typename OpTy>
-LogicalResult checkPrepareXferOp(OpTy xferOp,
-                                 VectorTransferToSCFOptions options) {
+static LogicalResult checkPrepareXferOp(OpTy xferOp, PatternRewriter &rewriter,
+                                        VectorTransferToSCFOptions options) {
   if (xferOp->hasAttr(kPassLabel))
-    return failure();
+    return rewriter.notifyMatchFailure(xferOp, "kPassLabel is present!");
   if (xferOp.getVectorType().getRank() <= options.targetRank)
-    return failure();
-  // Currently the unpacking of the leading dimension into the memref is not
-  // supported for scalable dimensions.
+    return rewriter.notifyMatchFailure(
+        xferOp, "xferOp vector rank <= transformation target rank");
   if (xferOp.getVectorType().getScalableDims().front())
-    return failure();
+    return rewriter.notifyMatchFailure(
+        xferOp, "Unpacking of the leading dimension into the memref is not yet "
+                "supported for scalable dims");
   if (isTensorOp(xferOp) && !options.lowerTensors)
-    return failure();
-  // Transfer ops that modify the element type are not supported atm.
+    return rewriter.notifyMatchFailure(
+        xferOp, "Unpacking for tensors has been disabled.");
   if (xferOp.getVectorType().getElementType() !=
       xferOp.getShapedType().getElementType())
-    return failure();
+    return rewriter.notifyMatchFailure(
+        xferOp, "Mismatching source and destination element types.");
+
   return success();
 }
 
@@ -597,8 +600,9 @@ struct PrepareTransferReadConversion
 
   LogicalResult matchAndRewrite(TransferReadOp xferOp,
                                 PatternRewriter &rewriter) const override {
-    if (checkPrepareXferOp(xferOp, options).failed())
-      return failure();
+    if (checkPrepareXferOp(xferOp, rewriter, options).failed())
+      return rewriter.notifyMatchFailure(
+          xferOp, "checkPrepareXferOp conditions not met!");
 
     auto buffers = allocBuffers(rewriter, xferOp);
     auto *newXfer = rewriter.clone(*xferOp.getOperation());
@@ -646,8 +650,9 @@ struct PrepareTransferWriteConversion
 
   LogicalResult matchAndRewrite(TransferWriteOp xferOp,
                                 PatternRewriter &rewriter) const override {
-    if (checkPrepareXferOp(xferOp, options).failed())
-      return failure();
+    if (checkPrepareXferOp(xferOp, rewriter, options).failed())
+      return rewriter.notifyMatchFailure(
+          xferOp, "checkPrepareXferOp conditions not met!");
 
     Location loc = xferOp.getLoc();
     auto buffers = allocBuffers(rewriter, xferOp);
@@ -1294,16 +1299,14 @@ struct UnrollTransferReadConversion
           xferOp, "vector rank is less or equal to target rank");
     if (failed(checkLowerTensors(xferOp, rewriter)))
       return failure();
-    // Transfer ops that modify the element type are not supported atm.
     if (xferOp.getVectorType().getElementType() !=
         xferOp.getShapedType().getElementType())
       return rewriter.notifyMatchFailure(
           xferOp, "not yet supported: element type mismatch");
     auto xferVecType = xferOp.getVectorType();
     if (xferVecType.getScalableDims()[0]) {
-      // Cannot unroll a scalable dimension at compile time.
       return rewriter.notifyMatchFailure(
-          xferOp, "scalable dimensions cannot be unrolled");
+          xferOp, "scalable dimensions cannot be unrolled at compile time");
     }
 
     auto insertOp = getInsertOp(xferOp);

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[javedabsar1]

LGTM with some nitpick comments/suggestions.

[javedabsar1]

Should we retain this comment if it gives additional info?

[banach-space]

Oh, I just moved the comment inside rewriter.notifyMatchFailure (with some tweaks):

// BEFORE
// Transfer ops that modify the element type are not supported atm.
  if (xferOp.getVectorType().getElementType() !=
      xferOp.getShapedType().getElementType())
    return failure();

// AFTER
  if (xferOp.getVectorType().getElementType() !=
      xferOp.getShapedType().getElementType())
    return rewriter.notifyMatchFailure(
        xferOp, "Mismatching source and destination element types.");

[javedabsar1]

(progressing lowering in progress)
maybe (vector-to-scf lowering in progress)?

[banach-space]

@javedabsar1 , thanks for reviewing! I've addressed your comments, so will land this. Let me know if you feel that there's something outstanding.