-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[mlir][tosa] Add folder for multiply like reduce_prod operation #128067
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
Conversation
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-tosa Author: Tai Ly (Tai78641) ChangesThis commit uses mulBinaryFolder for reduce_prod operations that have a constant 1D input of two values. Full diff: https://github.com/llvm/llvm-project/pull/128067.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/Tosa/IR/TosaCanonicalizations.cpp b/mlir/lib/Dialect/Tosa/IR/TosaCanonicalizations.cpp
index 9bfc2aae1d6a5..45bcacff19caa 100644
--- a/mlir/lib/Dialect/Tosa/IR/TosaCanonicalizations.cpp
+++ b/mlir/lib/Dialect/Tosa/IR/TosaCanonicalizations.cpp
@@ -963,10 +963,34 @@ REDUCE_FOLDER(ReduceAllOp)
REDUCE_FOLDER(ReduceAnyOp)
REDUCE_FOLDER(ReduceMaxOp)
REDUCE_FOLDER(ReduceMinOp)
-REDUCE_FOLDER(ReduceProdOp)
REDUCE_FOLDER(ReduceSumOp)
#undef REDUCE_FOLDER
+OpFoldResult ReduceProdOp::fold(FoldAdaptor adaptor) {
+ ShapedType inputTy = llvm::cast<ShapedType>(getInput().getType());
+ if (!inputTy.hasRank())
+ return {};
+ if (inputTy == getType() && (inputTy.getRank() == 0 || inputTy.getDimSize(getAxis()) == 1))
+ return getInput();
+
+ // Fold multiply like reduce_prod operators using mulBinaryFolder
+ if (inputTy.getRank() == 1 && inputTy.getDimSize(0) == 2) {
+ const auto resultTy = llvm::dyn_cast<RankedTensorType>(getType());
+ if (!resultTy)
+ return {};
+
+ const auto elements = llvm::dyn_cast_if_present<DenseElementsAttr>(adaptor.getInput());
+ if (!elements)
+ return {};
+
+ const auto lhsAttr = DenseElementsAttr::get(resultTy, {elements.getValues<Attribute>()[0]});
+ const auto rhsAttr = DenseElementsAttr::get(resultTy, {elements.getValues<Attribute>()[1]});
+ return mulBinaryFolder(lhsAttr, rhsAttr, resultTy, 0);
+ }
+
+ return {};
+}
+
OpFoldResult ReshapeOp::fold(FoldAdaptor adaptor) {
auto inputTy = llvm::dyn_cast<RankedTensorType>(getInput1().getType());
auto outputTy = llvm::dyn_cast<RankedTensorType>(getType());
diff --git a/mlir/test/Dialect/Tosa/canonicalize.mlir b/mlir/test/Dialect/Tosa/canonicalize.mlir
index 0e177a076ee7a..316f22f88fc69 100644
--- a/mlir/test/Dialect/Tosa/canonicalize.mlir
+++ b/mlir/test/Dialect/Tosa/canonicalize.mlir
@@ -1012,3 +1012,14 @@ func.func nested @do_not_fold_reciprocal_int() -> tensor<3x600x1200xi32> {
%2 = "tosa.reciprocal"(%1): (tensor<3x600x1200xi32>) -> tensor<3x600x1200xi32>
return %2 : tensor<3x600x1200xi32>
}
+
+// -----
+
+// CHECK-LABEL: @fold_reduce_prod_is_mul
+func.func @fold_reduce_prod_is_mul() -> tensor<1xi32> {
+ // CHECK-DAG: %[[VAL_0:.*]] = "tosa.const"() <{value = dense<77> : tensor<1xi32>}> : () -> tensor<1xi32>
+ // CHECK: return %[[VAL_0]] : tensor<1xi32>
+ %0 = "tosa.const"() <{value = dense<[1, 77]> : tensor<2xi32>}> : () -> tensor<2xi32>
+ %1 = "tosa.reduce_prod"(%0) <{axis = 0 : i32}> : (tensor<2xi32>) -> tensor<1xi32>
+ return %1 : tensor<1xi32>
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
a4b507e to
62b2409
Compare
| (inputTy.getRank() == 0 || inputTy.getDimSize(getAxis()) == 1)) | ||
| return getInput(); | ||
|
|
||
| // Fold multiply like reduce_prod operators using mulBinaryFolder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite limited special-cased. What is the motivation behind this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed it's quite special cased, it was observed while legalizing a model with dynamic shapes
FranklandJack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the change ID from the commit message?
| OpFoldResult ReduceProdOp::fold(FoldAdaptor adaptor) { | ||
| ShapedType inputTy = llvm::cast<ShapedType>(getInput().getType()); | ||
| if (!inputTy.hasRank()) | ||
| return {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a newline after the return here to aid readability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| #undef REDUCE_FOLDER | ||
|
|
||
| OpFoldResult ReduceProdOp::fold(FoldAdaptor adaptor) { | ||
| ShapedType inputTy = llvm::cast<ShapedType>(getInput().getType()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use auto here since the type is already explicit on the RHS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| return getInput(); | ||
|
|
||
| // Fold multiply like reduce_prod operators using mulBinaryFolder | ||
| if (inputTy.getRank() == 1 && inputTy.getDimSize(0) == 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we invert the condition here and do an early exit to avoid the indention and big conditional block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| return %2 : tensor<3x600x1200xi32> | ||
| } | ||
|
|
||
| // ----- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add some negative tests here? From the above logic it seems like the following cases should fail:
- input tensor of rank > 1
- input tensor of rank 1 with dim[0] > 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
62b2409 to
835c8db
Compare
98ff8a5 to
b2026c2
Compare
This commit uses mulBinaryFolder for reduce_prod operations that have a constant 1D input of two values. Change-Id: Icb234282c70898189083231506ed38a3ab40efb2 Signed-off-by: Luke Hutton <[email protected]>
b2026c2 to
a8fd688
Compare
|
No longer needed |
This commit uses mulBinaryFolder for reduce_prod operations that have a constant 1D input of two values.