Skip to content

[mlir][tosa] Fix RFFT2D verifier for width=1 #130279

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 1 commit into from
Mar 7, 2025

Conversation

RoboTux
Copy link
Contributor

@RoboTux RoboTux commented Mar 7, 2025

Current formula assumes width is a multiple of 2 but TOSA only requires
a power of 2, which 1 is.

Current formula assumes width is a multiple of 2 but TOSA only requires
a power of 2, which 1 is.
@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2025

@llvm/pr-subscribers-mlir-tosa

@llvm/pr-subscribers-mlir

Author: Thomas Preud'homme (RoboTux)

Changes

Current formula assumes width is a multiple of 2 but TOSA only requires
a power of 2, which 1 is.


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

2 Files Affected:

  • (modified) mlir/lib/Dialect/Tosa/IR/TosaOps.cpp (+1-1)
  • (modified) mlir/test/Dialect/Tosa/ops.mlir (+7)
diff --git a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
index ea4414fc1890e..f8588aa0ace0f 100644
--- a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
+++ b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
@@ -888,7 +888,7 @@ LogicalResult tosa::RFFT2dOp::verify() {
   // Output width dimension expected to be input_width / 2 + 1
   const int64_t outputWidth = outputType.getDimSize(2);
   if (!ShapedType::isDynamic(width) && !ShapedType::isDynamic(outputWidth) &&
-      (outputWidth - 1) * 2 != width)
+      (outputWidth != (width / 2) + 1))
     return emitOpError(
                "expected output width to be equal to input_width / 2 + 1, got ")
            << outputWidth;
diff --git a/mlir/test/Dialect/Tosa/ops.mlir b/mlir/test/Dialect/Tosa/ops.mlir
index 4c2cda8d9c027..600afe2abbff2 100644
--- a/mlir/test/Dialect/Tosa/ops.mlir
+++ b/mlir/test/Dialect/Tosa/ops.mlir
@@ -175,6 +175,13 @@ func.func @test_rfft2d(%arg0: tensor<13x8x16xf32>) -> (tensor<13x8x9xf32>, tenso
   return %0, %1 : tensor<13x8x9xf32>, tensor<13x8x9xf32>
 }
 
+// -----
+// CHECK-LABEL: rfft2d_width1
+func.func @test_rfft2d_width1(%arg0: tensor<1x1x1xf32>) -> (tensor<1x1x1xf32>, tensor<1x1x1xf32>) {
+  %0, %1 = tosa.rfft2d %arg0 : (tensor<1x1x1xf32>) -> (tensor<1x1x1xf32>, tensor<1x1x1xf32>)
+  return %0, %1 : tensor<1x1x1xf32>, tensor<1x1x1xf32>
+}
+
 // -----
 // CHECK-LABEL: rfft2d_with_local_bound
 func.func @test_rfft2d_with_local_bound(%arg0: tensor<13x8x16xf32>) -> (tensor<13x8x9xf32>, tensor<13x8x9xf32>) {

@lhutton1 lhutton1 changed the title [TOSA] Fix RFFT2D verifier for width=1 [mlir][tosa] Fix RFFT2D verifier for width=1 Mar 7, 2025
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 for the fix @RoboTux!

@RoboTux RoboTux merged commit 9d191f1 into llvm:main Mar 7, 2025
12 of 13 checks passed
@RoboTux RoboTux deleted the fix_rfft2d_verif branch March 12, 2025 13:27
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
Current formula assumes width is a multiple of 2 but TOSA only requires
a power of 2, which 1 is.
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.

3 participants