Skip to content

Conversation

@Jerry-Ge
Copy link
Member

@Jerry-Ge Jerry-Ge commented Feb 20, 2025

  • Fix ability to expand ranks with dynamic shape support
  • Simplify the code

@llvmbot
Copy link
Member

llvmbot commented Feb 20, 2025

@llvm/pr-subscribers-mlir-tosa

@llvm/pr-subscribers-mlir

Author: Jerry-Ge (Jerry-Ge)

Changes

Fix ability to expand ranks with dynamic shape support


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Tosa/Utils/ConversionUtils.cpp (+2-2)
diff --git a/mlir/lib/Dialect/Tosa/Utils/ConversionUtils.cpp b/mlir/lib/Dialect/Tosa/Utils/ConversionUtils.cpp
index d1a8732dac212..9ade05c8f095b 100644
--- a/mlir/lib/Dialect/Tosa/Utils/ConversionUtils.cpp
+++ b/mlir/lib/Dialect/Tosa/Utils/ConversionUtils.cpp
@@ -88,9 +88,9 @@ computeReshapeOutput(ArrayRef<int64_t> higherRankShape,
     higherRankDim = higherRankShape[i];
     lowerRankDim = lowerRankShape[j];
 
-    if (lowerRankDim == 1 && higherRankDim > 1)
+    if (lowerRankDim == 1 && higherRankDim != 1)
       reshapeOutputShape[i] = 1;
-    else if ((lowerRankDim > 1 && higherRankDim == 1) ||
+    else if ((lowerRankDim != 1 && higherRankDim == 1) ||
              (lowerRankDim == higherRankDim))
       reshapeOutputShape[i] = lowerRankDim;
     else if (higherRankDim != lowerRankDim)

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.

Is it possible to test this change at all?

@Jerry-Ge
Copy link
Member Author

Is it possible to test this change at all?

good question, i don't have a clear answer. CC the original author: @sjarus

@Jerry-Ge Jerry-Ge force-pushed the expand_ranks branch 2 times, most recently from 9f7d82f to 16462f4 Compare February 24, 2025 22:01
- The use of != 1 accommodates the use of kDynamicDim
- Simplified the for loop to iterate only on lowerRank and access the
  higherRank dim by using the rankDiff

Signed-off-by: Suraj Sudhir <[email protected]>
Change-Id: I0f223f335667b2e32c43d4370f0a4b11b0617694
@Jerry-Ge Jerry-Ge merged commit 0a7809c into llvm:main Feb 25, 2025
9 of 10 checks passed
@Jerry-Ge Jerry-Ge deleted the expand_ranks branch March 20, 2025 21:41
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