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][tosa] Fix conv op build functions #126321

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

Tai78641
Copy link
Contributor

@Tai78641 Tai78641 commented Feb 7, 2025

This patch fixes several issues:

  • buildConvOpWithQuantInfo:
    call buildConvOpResultTypeInfo to get final output type
  • buildTransConvOpWithQuantInfo:
    add input_zp and weight_zp operands
    remove input_zp/weight_zp attributes
  • createZeroPointTensor:
    add getElementTypeOrSelf to get element type just in case
    remove bad auto-merge lines

Change-Id: Idbf88f500ce57a865da4b7be7b7b8bf2ba194b24

This patch fixes several issues:
  - buildConvOpWithQuantInfo:
       call buildConvOpResultTypeInfo to get final output type
  - buildTransConvOpWithQuantInfo:
       add input_zp and weight_zp operands
       remove input_zp/weight_zp attributes
  - createZeroPointTensor:
       add getElementTypeOrSelf to get element type just in case
       remove bad auto-merge lines

Signed-off-by: Tai Ly <[email protected]>
Change-Id: Idbf88f500ce57a865da4b7be7b7b8bf2ba194b24
@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2025

@llvm/pr-subscribers-mlir-tosa

@llvm/pr-subscribers-mlir

Author: Tai Ly (Tai78641)

Changes

This patch fixes several issues:

  • buildConvOpWithQuantInfo:
    call buildConvOpResultTypeInfo to get final output type
  • buildTransConvOpWithQuantInfo:
    add input_zp and weight_zp operands
    remove input_zp/weight_zp attributes
  • createZeroPointTensor:
    add getElementTypeOrSelf to get element type just in case
    remove bad auto-merge lines

Change-Id: Idbf88f500ce57a865da4b7be7b7b8bf2ba194b24


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Tosa/IR/TosaOps.cpp (+17-20)
diff --git a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
index 955021abdd67b12..fd166cc1322cef9 100644
--- a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
+++ b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
@@ -510,7 +510,13 @@ static void buildConvOpWithQuantInfo(OpBuilder &builder, OperationState &result,
   result.addAttribute("stride", stride);
   result.addAttribute("dilation", dilation);
   result.addAttribute("acc_type", accType);
-  result.addTypes(outputType);
+  Type finalOutputType = outputType;
+  auto quantAttr = buildConvOpQuantizationAttr(builder, input, weight);
+  if (quantAttr) {
+    finalOutputType =
+        buildConvOpResultTypeInfo(builder, outputType, input, weight);
+  }
+  result.addTypes(finalOutputType);
 }
 
 /// Handles tosa.transpose_conv2d which has outpad and output shape
@@ -519,25 +525,19 @@ static void buildTransConvOpWithQuantInfo(
     OpBuilder &builder, OperationState &result, Type outputType, Value input,
     Value weight, Value bias, DenseI64ArrayAttr outpad,
     DenseI64ArrayAttr stride, DenseI64ArrayAttr outputShape, TypeAttr accType) {
-  result.addOperands({input, weight, bias});
+  auto zps = createZPsAsConst(builder, input, weight);
+  result.addOperands({input, weight, bias, zps.first, zps.second});
   result.addAttribute("out_pad", outpad);
   result.addAttribute("stride", stride);
   result.addAttribute("out_shape", outputShape);
   result.addAttribute("acc_type", accType);
-  auto quantAttr = ::buildConvOpQuantizationAttr(builder, input, weight);
-
+  Type finalOutputType = outputType;
+  auto quantAttr = buildConvOpQuantizationAttr(builder, input, weight);
   if (quantAttr) {
-    result.addAttribute("input_zp",
-                        builder.getI32IntegerAttr(
-                            static_cast<int32_t>(quantAttr.getInputZp())));
-    result.addAttribute("weight_zp",
-                        builder.getI32IntegerAttr(
-                            static_cast<int32_t>(quantAttr.getWeightZp())));
-    result.addTypes(
-        buildConvOpResultTypeInfo(builder, outputType, input, weight));
-  } else {
-    result.addTypes(outputType);
+    finalOutputType =
+        buildConvOpResultTypeInfo(builder, outputType, input, weight);
   }
+  result.addTypes(finalOutputType);
 }
 
 /// The tosa.fully_connected op has its own builder as it does not have
@@ -2492,18 +2492,15 @@ LogicalResult mlir::tosa::getZeroPoint(ElementsAttr zpAttr, int64_t &zp) {
   return failure();
 }
 
-// Create a rank-0 const tensor for zero point of the source tensor.
+// Create a rank-1 const tensor for zero point of the source tensor.
 std::optional<Value> mlir::tosa::createZeroPointTensor(OpBuilder &builder,
                                                        Location loc,
                                                        Type srcElemType,
                                                        int64_t zp) {
-  if (auto quantType =
-          llvm::dyn_cast<mlir::quant::UniformQuantizedType>(srcElemType))
-    srcElemType = quantType.getStorageType();
-
-  auto zpType = mlir::RankedTensorType::get({1}, srcElemType);
+  srcElemType = getElementTypeOrSelf(srcElemType);
   if (auto quantType = llvm::dyn_cast<mlir::quant::QuantizedType>(srcElemType))
     srcElemType = quantType.getStorageType();
+  auto zpType = mlir::RankedTensorType::get({1}, srcElemType);
   if (llvm::isa<FloatType>(srcElemType)) {
     auto zpAttr = DenseElementsAttr::get(
         zpType, builder.getFloatAttr(srcElemType, static_cast<double>(zp)));

@Jerry-Ge Jerry-Ge merged commit 62ae876 into llvm:main Feb 10, 2025
11 checks passed
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
This patch fixes several issues:
  - buildConvOpWithQuantInfo: 
       call buildConvOpResultTypeInfo to get final output type
  - buildTransConvOpWithQuantInfo:
       add input_zp and weight_zp operands
       remove input_zp/weight_zp attributes
  - createZeroPointTensor:
       add getElementTypeOrSelf to get element type just in case
       remove bad auto-merge lines


Change-Id: Idbf88f500ce57a865da4b7be7b7b8bf2ba194b24

Signed-off-by: Tai Ly <[email protected]>
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
This patch fixes several issues:
  - buildConvOpWithQuantInfo: 
       call buildConvOpResultTypeInfo to get final output type
  - buildTransConvOpWithQuantInfo:
       add input_zp and weight_zp operands
       remove input_zp/weight_zp attributes
  - createZeroPointTensor:
       add getElementTypeOrSelf to get element type just in case
       remove bad auto-merge lines


Change-Id: Idbf88f500ce57a865da4b7be7b7b8bf2ba194b24

Signed-off-by: Tai Ly <[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.

4 participants