Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][vector] Restore assert and fix typos #68581

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

banach-space
Copy link
Contributor

Follow-up for #68400 - restoring an assert that was accidentally removed and fixed a typo in a diagnostic.

Follow-up for llvm#68400 - restoring an assert that was accidentally removed
and fixed a typo in a diagnostic.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 9, 2023

@llvm/pr-subscribers-mlir-vector

@llvm/pr-subscribers-mlir

Changes

Follow-up for #68400 - restoring an assert that was accidentally removed and fixed a typo in a diagnostic.


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Vector/Transforms/LowerVectorContract.cpp (+5-2)
diff --git a/mlir/lib/Dialect/Vector/Transforms/LowerVectorContract.cpp b/mlir/lib/Dialect/Vector/Transforms/LowerVectorContract.cpp
index 311e589547b89bc..5463a7bd8f4c840 100644
--- a/mlir/lib/Dialect/Vector/Transforms/LowerVectorContract.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/LowerVectorContract.cpp
@@ -432,6 +432,9 @@ struct UnrolledOuterProductGenerator
       return failure();
 
     int reductionSize = lhsType.getDimSize(reductionDim);
+    assert(reductionSize > 0 &&
+           "Reduction dim must be a known static size to allow unrolling");
+
     // Incremental support for masking.
     if (mask && !maybeMask.has_value())
       return failure();
@@ -997,7 +1000,7 @@ FailureOr<Value> ContractionOpLowering::lowerParallel(PatternRewriter &rewriter,
       });
     if (lhsType.getScalableDims()[lhsIndex])
       return rewriter.notifyMatchFailure(op, [&](Diagnostic &diag) {
-        diag << "Unrolloing scalable dimension (lhsIndex=" << lhsIndex
+        diag << "Unrolling scalable dimension (lhsIndex=" << lhsIndex
              << ") is not supported yet";
       });
     dimSize = lhsType.getDimSize(lhsIndex);
@@ -1005,7 +1008,7 @@ FailureOr<Value> ContractionOpLowering::lowerParallel(PatternRewriter &rewriter,
     iterIndex = iMap[1].getDimPosition(rhsIndex);
     if (rhsType.getScalableDims()[rhsIndex])
       return rewriter.notifyMatchFailure(op, [&](Diagnostic &diag) {
-        diag << "Unrolloing scalable dimension (lhsIndex=" << lhsIndex
+        diag << "Unrolling scalable dimension (rhsIndex=" << rhsIndex
              << ") is not supported yet";
       });
     dimSize = rhsType.getDimSize(rhsIndex);

Copy link
Collaborator

@c-rhodes c-rhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for fixes

Copy link
Member

@MacDue MacDue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@banach-space banach-space merged commit e01c867 into llvm:main Oct 9, 2023
6 checks passed
@banach-space banach-space deleted the andrzej/address_follow_up branch March 16, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants