Skip to content

[mlir][tosa] Fix indexing in TosaToTensor #140906

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

Merged
merged 3 commits into from
May 22, 2025

Conversation

shay-kl
Copy link
Contributor

@shay-kl shay-kl commented May 21, 2025

Changed the indexing used in the extractOp from one that is intended for 0d tensors to one that is intended for 1d tensors.

@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-tosa

Author: Shay Kleiman (shay-kl)

Changes

Changed the indexing used in the extractOp from one that is intended for 0d tensors to one that is intended for 1d tensors.


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

1 Files Affected:

  • (modified) mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp (+2-1)
diff --git a/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp b/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp
index 5f23a33049f87..4f565607e2167 100644
--- a/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp
+++ b/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp
@@ -362,7 +362,8 @@ class PadConverter : public OpConversionPattern<tosa::PadOp> {
     // Setup the default constantAttr.
 
     Value padConstant = rewriter.createOrFold<tensor::ExtractOp>(
-        loc, padOp.getPadConst(), ValueRange({}));
+      loc, padOp.getPadConst(),
+      ValueRange({rewriter.create<arith::ConstantIndexOp>(loc, 0)}));
 
     if (!padConstant) {
       return rewriter.notifyMatchFailure(

Copy link

github-actions bot commented May 21, 2025

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

Changed the indexing used in the extractOp from one that is intended for
0d tensors to one that is intended for 1d tensors.
@shay-kl shay-kl force-pushed the shay-kl/fix-tensor-padop-indexing branch from 1c3eb32 to 216a5f2 Compare May 21, 2025 15:20
@shay-kl
Copy link
Contributor Author

shay-kl commented May 22, 2025

The issue seems to be from 0c34d7a @lhutton1

Copy link
Contributor

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

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

Thanks @shay-kl, LGTM. I think the related change is actually: #125297

@shay-kl shay-kl force-pushed the shay-kl/fix-tensor-padop-indexing branch from f755b61 to 8cc4f3b Compare May 22, 2025 11:01
@shay-kl
Copy link
Contributor Author

shay-kl commented May 22, 2025

I don't have permissions, can you please land it?

@shay-kl shay-kl force-pushed the shay-kl/fix-tensor-padop-indexing branch from 8cc4f3b to 41151d6 Compare May 22, 2025 11:14
@amirBish amirBish merged commit 6375a85 into llvm:main May 22, 2025
11 checks passed
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
Changed the indexing used in the extractOp from one that is intended for
0d tensors to one that is intended for 1d tensors.

---------

Co-authored-by: Shay Kleiman <[email protected]>
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 6, 2025
Changed the indexing used in the extractOp from one that is intended for
0d tensors to one that is intended for 1d tensors.

---------

Co-authored-by: Shay Kleiman <[email protected]>
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.

5 participants