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.

Addressing CR and adding documentation

Harbormaster completed remote builds in B220742: Diff 507008.Mar 21 2023, 9:42 AM

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.

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.

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

mgehre-amd added a subscriber: mgehre-amd.Mar 22 2023, 2:44 AM