I recently looked into the tosa multiplication, and I was wondering what the result for an overflowing i32 multipliation (with shift = 0) should be.
The tosa.mul spec says
result = value1 * value2; // low 32-bits of result for int32_t
However, in the Main Inference Profile, the table mentions for MUL that “Integer result overflows must be saturated”.
Can somebody clarify this for me?
Hi @TinaAMD. That is a good catch, the TOSA specification isn’t clear here.
The original language in MUL with the REQUIRE gives you unspecified behavior in the overflow case (any REQUIRE that fails results in the results being tosa_unpredictable).
Recently we added the Main inference conformance information that you link to, which requires saturation. I think with C++ having integer overflow unspecified, we may need to modify the language in Main inference profile to leave the integer overflow behavior unspecified.
Do you have a requirement for saturation or any specific behavior for your implementation, or would unspecified meet your needs?
Thank you for the quick response!
I don’t have a use case where the result is required to be one or the other, I think the unspecified behavior is a reasonable choice.
Would this mean that the Main Inference Profile section would be updated? I guess that tosa.add and tosa.sub have the same diverging behavior in the reference model (add, sub [same file]) compared to that section of the spec. I notiticed that in the Integer Behavior section overflows are mentioned and that they are said to have unpredictable results.
This behavior should also be fine for the Torch-to-TOSA lowering, as integer overflows seem to be unspecified in Torch as well.
Great! Yes, we will clarify the meaning in the Main Inference Profile section. The existing wording in that table is meant to apply to the CAST operator when converting from float to int, requiring saturation if overflow occurs. We missed that it can be read to apply to the results of an integer MUL (or ADD/SUB).
We should have new wording soon, but for now you can expect unspecified behavior as the result. I’ll try to post the update here when we’ve got it.
