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] Fix invalid IR in vector.print lowering #74410

Merged

Conversation

matthias-springer
Copy link
Member

@matthias-springer matthias-springer commented Dec 5, 2023

DecomposePrintOpConversion used to generate invalid op such as:

error: 'arith.extsi' op operand type 'vector<10xi32>' and result type 'vector<10xi32>' are cast incompatible
  vector.print %v9 : vector<10xi32>

This commit fixes tests such as mlir/test/Integration/Dialect/Vector/CPU/test-reductions-i32.mlir when verifying the IR after each pattern application (#74270).

`DecomposePrintOpConversion` used to generate invalid op such as:
```
error: 'arith.extsi' op operand type 'vector<10xi32>' and result type 'vector<10xi32>' are cast incompatible
  vector.print %v9 : vector<10xi32>
```

This commit fixes tests such as `mlir/test/Integration/Dialect/Vector/CPU/test-reductions-i32.mlir` when verifying the IR after each pattern (llvm#74270).
@llvmbot
Copy link
Member

llvmbot commented Dec 5, 2023

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

DecomposePrintOpConversion used to generate invalid op such as:

error: 'arith.extsi' op operand type 'vector&lt;10xi32&gt;' and result type 'vector&lt;10xi32&gt;' are cast incompatible
  vector.print %v9 : vector&lt;10xi32&gt;

This commit fixes tests such as mlir/test/Integration/Dialect/Vector/CPU/test-reductions-i32.mlir when verifying the IR after each pattern application (#74270).


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

1 Files Affected:

  • (modified) mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp (+8-6)
diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index 33a77d7576ba7..2ee314e9fedfe 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -726,12 +726,14 @@ struct DecomposePrintOpConversion : public VectorToSCFPattern<vector::PrintOp> {
       auto targetVectorType = vectorType.cloneWith({}, legalIntTy);
       value = rewriter.create<vector::BitCastOp>(loc, signlessSourceVectorType,
                                                  value);
-      if (width == 1 || intTy.isUnsigned())
-        value = rewriter.create<arith::ExtUIOp>(loc, signlessTargetVectorType,
-                                                value);
-      else
-        value = rewriter.create<arith::ExtSIOp>(loc, signlessTargetVectorType,
-                                                value);
+      if (value.getType() != signlessTargetVectorType) {
+        if (width == 1 || intTy.isUnsigned())
+          value = rewriter.create<arith::ExtUIOp>(loc, signlessTargetVectorType,
+                                                  value);
+        else
+          value = rewriter.create<arith::ExtSIOp>(loc, signlessTargetVectorType,
+                                                  value);
+      }
       value = rewriter.create<vector::BitCastOp>(loc, targetVectorType, value);
       vectorType = targetVectorType;
     }

@joker-eph
Copy link
Collaborator

Thanks Matthias for all the 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, thank you!

@matthias-springer matthias-springer merged commit 8f9aac4 into llvm:main Dec 6, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants