[Mlir-commits] [mlir] [mlir][tosa] Fail lowering of `tosa.pad` -> `tensor.pad` with -1 padding (PR #232221)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-tosa

Author: Federico Bruzzone (FedericoBruzzone)

<details>
<summary>Changes</summary>

In `tosa.pad`, a padding of -1 means that the amount is not known. 
`TosaToTensor` lowered it as is to `tensor.pad low[%c-1, ...]`, which has negative padding.

`tensor.pad` requires non-negative padding now (see #<!-- -->231389):
1. _Attributes_ (constant amounts) are rejected by the verifier.
2. _SSA Values_ that eventually fold to negative are UB.

With this PR, we bail out of the lowering (`tosa.pad` -> `tensor.pad`) when the padding contains -1.

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


3 Files Affected:

- (modified) mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp (+7) 
- (modified) mlir/test/Conversion/TosaToTensor/tosa-to-tensor-invalid.mlir (+10) 
- (modified) mlir/test/Conversion/TosaToTensor/tosa-to-tensor.mlir (-17) 


``````````diff
diff --git a/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp b/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp
index 9bf9ca3ae7a89b..1c65abbb7af41a 100644
--- a/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp
+++ b/mlir/lib/Conversion/TosaToTensor/TosaToTensor.cpp
@@ -354,6 +354,13 @@ class PadConverter : public OpConversionPattern<tosa::PadOp> {
       paddingVals.push_back(static_cast<int64_t>(idx.getInt()));
     }
 
+    // A padding of -1 means that the amount is not known, so it cannot be
+    // lowered to a `tensor.pad`, which requires non-negative padding.
+    if (llvm::is_contained(paddingVals, -1)) {
+      return rewriter.notifyMatchFailure(
+          padOp, "padding must not contain -1 (unknown amount)");
+    }
+
     ShapedType inputTy = cast<ShapedType>(input.getType());
     int64_t rank = inputTy.getRank();
 
diff --git a/mlir/test/Conversion/TosaToTensor/tosa-to-tensor-invalid.mlir b/mlir/test/Conversion/TosaToTensor/tosa-to-tensor-invalid.mlir
index 5e33ce2a2fcced..8741ea07431ec1 100644
--- a/mlir/test/Conversion/TosaToTensor/tosa-to-tensor-invalid.mlir
+++ b/mlir/test/Conversion/TosaToTensor/tosa-to-tensor-invalid.mlir
@@ -8,3 +8,13 @@ func.func @slice_resultType_unranked(%arg0: tensor<?xf32>) -> (tensor<*xf32>) {
   %2 = tosa.slice %arg0, %start, %size : (tensor<?xf32>, !tosa.shape<1>, !tosa.shape<1>) -> tensor<*xf32>
   return %2 : tensor<*xf32>
 }
+
+// -----
+
+func.func @pad_unknown_padding(%arg0: tensor<1x2xf32>) -> tensor<?x9xf32> {
+  %padding = tosa.const_shape values(dense<[-1, 2, 3, 4]> : tensor<4xindex>) : () -> !tosa.shape<4>
+  %pad_const = "tosa.const"() <{values = dense<3.14> : tensor<1xf32>}> : () -> tensor<1xf32>
+  // expected-error at +1 {{failed to legalize operation 'tosa.pad'}}
+  %0 = "tosa.pad"(%arg0, %padding, %pad_const) : (tensor<1x2xf32>, !tosa.shape<4>, tensor<1xf32>) -> tensor<?x9xf32>
+  return %0 : tensor<?x9xf32>
+}
diff --git a/mlir/test/Conversion/TosaToTensor/tosa-to-tensor.mlir b/mlir/test/Conversion/TosaToTensor/tosa-to-tensor.mlir
index c4e98bf7bece9d..414832d6b5d9fa 100644
--- a/mlir/test/Conversion/TosaToTensor/tosa-to-tensor.mlir
+++ b/mlir/test/Conversion/TosaToTensor/tosa-to-tensor.mlir
@@ -568,23 +568,6 @@ func.func @pad_dyn_input(%arg0 : tensor<?x2xf32>) -> (tensor<?x9xf32>) {
 }
 // -----
 
-func.func @pad_dyn_padding(%arg0 : tensor<1x2xf32>) -> (tensor<?x9xf32>) {
-  %0 = tosa.const_shape values(dense<[-1, 2, 3, 4]> : tensor<4xindex>) : () -> !tosa.shape<4>
-  %pad_const = "tosa.const"() <{values = dense<3.14> : tensor<1xf32>}> : () -> tensor<1xf32>
-  // CHECK-DAG: [[INDEX1:%.+]] = arith.constant -1 : index
-  // CHECK-DAG: [[INDEX2:%.+]] = arith.constant 2 : index
-  // CHECK-DAG: [[INDEX3:%.+]] = arith.constant 3 : index
-  // CHECK-DAG: [[INDEX4:%.+]] = arith.constant 4 : index
-  // CHECK-DAG: [[CST:%.+]] = arith.constant 3.140000e+00 : f32
-  // CHECK: tensor.pad %[[ARG0]] low{{\[}}[[INDEX1]], [[INDEX3]]] high{{\[}}[[INDEX2]], [[INDEX4]]]  {
-  // CHECK:   tensor.yield [[CST]]
-  // CHECK: } : tensor<1x2xf32> to tensor<?x9xf32>
-  %1 = "tosa.pad"(%arg0, %0, %pad_const)  : (tensor<1x2xf32>, !tosa.shape<4>, tensor<1xf32>)  -> (tensor<?x9xf32>)
-  return %1 : tensor<?x9xf32>
-}
-
-// -----
-
 // CHECK-LABEL: @concat
 // CHECK-SAME: %[[ARG0:.+]]: tensor<5x1xf32>
 // CHECK-SAME: %[[ARG1:.+]]: tensor<6x1xf32>

``````````

</details>


https://github.com/llvm/llvm-project/pull/232221