Hi!
Thanks for your answer.
I’d prefer not to add/replace i8 with si8, as I believe that would break any consumer of the TOSA dialect, as they may not be able to accept signed ints.
I see what you mean, but this explains why not to replace i8
with si8
. By adding si8
we’re not supposed to break anything, just like ui8
was added ⚙ D108427 [mlir][tosa] Support UInt8 inputs and outputs for tosa.rescale.
It comes down to a discussion of whether TOSA->(linalg/etc.) is the proper place to do the signed->signless conversion, or whether it should be done frontend->TOSA. Your proposal would take us to the second option
I didn’t mean TOSA will be signless, so I don’t see why my proposal will do the signed->signless conversion in frontend->TOSA. I support keeping the signedness in TOSA, but doing it as defined in MLIR - ui8
for unsigned and si8
for signed. Currently in TOSA it is ui8
and i8
.
If I understand it right, the i8
in TOSA should have been si8
but is i8
by mistake. I assume I can use the UnrealizedConversionCast
, but I am wondering if and why this overhead is really required. Assuming it’s really a mistake, I think it’s better to fix it, without having to carry it with us.
It seems that now we use i8
with multiple meanings, for TOSA dialect it means signed int, but for feeding ops from other dialects it is considered signless.
My proposal is:
- Use
ui8
and si8
in TOSA dialect. This way it will fully match the spec where there is unsigned/signed int. And it will use the types with there natural meaning as defined by MLIR.
- Use
UnrealizedConversionCast
for TOSA consumers that do not support unsigned/signed int.
Is your design eventually going to the existing backends, or do you have your own backend in mind? If your own, does it expect signed or signless?
My code will eventually run on custom hardware, using llvm compiler.
We can also set an online meeting if you think it will make it easier to discuss.
And of course I will be glad to implement/help with any change I am proposing.
Thanks a lot,
Maya