[mlir] Add signed int to TOSA supported types

[mamrami]

Summary

After the discussion:
https://discourse.llvm.org/t/why-does-tosa-use-signless-integer-types/67505
it was decided to add the signed int types.

[mamrami]

edit commit message

[mamrami]

rebase

[rsuderman]

mlir/include/mlir/Dialect/Tosa/IR/TosaTypesBase.td

80

Keep Int then SInt lowering ordering.

[eric-k256]

As noted in the thread, I want to make sure that we're moving to remove i8 support in the near future, not supporting i8/ui8/si8. As a starting point to the move of i8 -> si8, this change is fine (mod Rob's comment), but it is starting down a path where multiple projects will be affected.

[mamrami]

Addressing CR and adding documentation

[mamrami]

[rsuderman]

In D146380#4207422, @eric-k256 wrote:

As noted in the thread, I want to make sure that we're moving to remove i8 support in the near future, not supporting i8/ui8/si8. As a starting point to the move of i8 -> si8, this change is fine (mod Rob's comment), but it is starting down a path where multiple projects will be affected.

I am torn on this issue as really leaving i8/ui8 for our supported types should be fine. Anyone wanting to support the TOSA dialect can convert their si8s to i8s to guarantee compatibility. The main issue is that we are defaulting to signless and unsigned types which feels inconsistent. It would be good to have buy ins from more projects before we land this to guarantee reasonable compatibility, and a plan to drop support for signless types in a reasonable timeline. I believe the work required in tflite-to-tosa should be fairly reasonable, and IREE would just need to update its tosa tests and sign stripping.

[eric-k256]

Given Rob's comments, I think that the path forward is to get comments from a wider set of people about making the switch to signed/unsigned from signless/signed so we aren't surprising any TOSA dialect users when we get to the point of removing signless support. An RFC on discourse seems to be the right way to do that, along with some time to collect responses. All of the lit tests are going to need to change, which is going to be a big set of changes that need to land. Most of it is mechanical, but still a lot of overhead.

As a possible route to unblocking you while that happens, a look through IREE's sign-stripping code may be a good example to use.

[rsuderman]

In D146380#4211389, @eric-k256 wrote:

Given Rob's comments, I think that the path forward is to get comments from a wider set of people about making the switch to signed/unsigned from signless/signed so we aren't surprising any TOSA dialect users when we get to the point of removing signless support. An RFC on discourse seems to be the right way to do that, along with some time to collect responses. All of the lit tests are going to need to change, which is going to be a big set of changes that need to land. Most of it is mechanical, but still a lot of overhead.

As a possible route to unblocking you while that happens, a look through IREE's sign-stripping code may be a good example to use.

You can feel free to grab the implementation from below and tweak as needed:
https://github.com/openxla/iree/blob/main/compiler/src/iree/compiler/Dialect/Flow/Transforms/StripSignedness.cpp

mlir/include/mlir/Dialect/Tosa/IR/TosaTypesBase.td

 //===----------------------------------------------------------------------===//
 // Non-Quantized Signed Integer Types.
 // Used to express accumulator results or compare results.
 //===----------------------------------------------------------------------===//
 
 def Tosa_UInt8 : UI<8>;
 def Tosa_UInt16 : UI<16>;
 
+// TOSA spec supports unsinged and signed integers. Signed integers were
+// implemented as Tosa_Int. This will be fixed gradually, since I<#> types
+// are signless as defined by MLIR. The signed/unsigned types are SI<#>/UI<#>.
+// TODO: Remove signless types once all TOSA generators migrate to
+// signed/unsigned types.
 def Tosa_Int8 : I<8>;
 def Tosa_Int16 : I<16>;
 def Tosa_Int32 : I<32>;
 def Tosa_Int48 : I<48>;
 def Tosa_Int64 : I<64>;
 
+def Tosa_SInt8 : SI<8>;
+def Tosa_SInt16 : SI<16>;
+def Tosa_SInt32 : SI<32>;
+def Tosa_SInt48 : SI<48>;
+def Tosa_SInt64 : SI<64>;
+
 def Tosa_SignedInt : AnyTypeOf<[Tosa_Int8,
                                 Tosa_Int16,
                                 Tosa_Int32,
                                 Tosa_Int48,
-                                Tosa_Int64]>;
+                                Tosa_Int64,
+                                Tosa_SInt8,
+                                Tosa_SInt16,
+                                Tosa_SInt32,
+                                Tosa_SInt48,
+                                Tosa_SInt64]>;
 
 def Tosa_Bool : I<1>;
 
 // No unsigned unquantized int types.
 def Tosa_Int : AnyTypeOf<[Tosa_Bool,
                           Tosa_UInt8,
                           Tosa_UInt16,
                           Tosa_SignedInt]>;
 
 def Tosa_Int32Or64 : AnyTypeOf<[Tosa_Int32,
-                            Tosa_Int64]>;
+                                Tosa_Int64,
+                                Tosa_SInt32,
+                                Tosa_SInt64]>;
 
 //===----------------------------------------------------------------------===//
 // Quantized Integer Types.
 // Datatype for network feature map or weight content.
 //===----------------------------------------------------------------------===//
 //===----------------------------------------------------------------------===//
 // Name    Symmetry   Grouping                Sign
 //===----------------------------------------------------------------------===//

mlir/test/Conversion/TosaToTensor/tosa-to-tensor.mlir

func.func @concat_non_axis_dyn(%arg0: tensor<5x?xf32>, %arg1: tensor<6x?xf32>) -> () {
   return
 }
 
 // -----
 
 // CHECK-LABEL: @concat_axis_dyn
 // CHECK-SAME: (%[[ARG0:[0-9a-zA-Z_]*]]:
 // CHECK-SAME:  %[[ARG1:[0-9a-zA-Z_]*]]:
-func.func @concat_axis_dyn(%arg0: tensor<?x3xf32>, %arg1: tensor<?x3xf32>) -> () {
+func.func @concat_axis_dyn(%arg0: tensor<?x3xsi8>, %arg1: tensor<?x3xsi8>) -> () {
   // CHECK: %[[AXIS:.+]] = arith.constant 0
   // CHECK: %[[STRIDE:.+]]   = arith.constant 1
   // CHECK: %[[OFFSET:.+]] = arith.constant 0 : index
   // CHECK: %[[IDX0:.+]] = arith.constant 0 : index
   // CHECK: %[[SIZE:.+]] = tensor.dim %[[ARG0]], %[[IDX0]]
   // CHECK: %[[IDX0_2:.+]] = arith.constant 0 : index
   // CHECK: %[[DYN:.+]] = tensor.dim %[[ARG0]], %[[IDX0_2]]
   // CHECK: %[[IDX1:.+]] = arith.constant 1 : index
-  // CHECK: %[[INIT:.+]] = tensor.empty(%[[DYN]]) : tensor<?x3xf32>
+  // CHECK: %[[INIT:.+]] = tensor.empty(%[[DYN]]) : tensor<?x3xsi8>
   // CHECK: %[[DYN1:.+]] = tensor.dim %[[ARG0]], %[[AXIS]]
   // CHECK: %[[INSERT0:.+]] = tensor.insert_slice %[[ARG0]] into %[[INIT]][0, 0] [%[[DYN1]], 3] [1, 1]
   // CHECK: %[[SUM:.+]]  = arith.addi %[[OFFSET]], %[[DYN1]]
   // CHECK: %[[DYN2:.+]] = tensor.dim %[[ARG1]], %[[AXIS]]
   // CHECK: %[[INSERT1:.+]] = tensor.insert_slice %[[ARG1]] into %[[INSERT0]][%[[SUM]], 0] [%[[DYN2]], 3] [1, 1]
-  %0 = "tosa.concat"(%arg0, %arg1) { axis = 0 : i64} : (tensor<?x3xf32>, tensor<?x3xf32>)  -> (tensor<?x3xf32>)
+  %0 = "tosa.concat"(%arg0, %arg1) { axis = 0 : i64} : (tensor<?x3xsi8>, tensor<?x3xsi8>)  -> (tensor<?x3xsi8>)
   return
 }