Integer dot product translation

[Quetzonarch]

This proposed change is a fix for translation of IR from OCL-like source when dot function is called with integer arguments (ints or vectors of chars or shorts) to properly support cl_khr_integer_dot_product extension. Previously dot would always be translated to its version with floating point arguments. With this fix the call to dot function should be translated into a proper OpCode, based on the arguments used.

[CLAassistant]

All committers have signed the CLA.

[MrSidims]

Approved for testing. @Quetzonarch could you please rebase your PR to exclude "Fix support of SPV_INTEL_vector_compute extension" from it as it was merged?

[StuartDBrady]

In my view, this optimization is beyond the scope of the SPIR-V/LLVM Translator, and should instead be implemented in spirv-opt (in SPIR-V Tools) and/or as part of a standalone optimizer for LLVM IR making use of OpenCL builtins. Such an optimizer could make use of the SPIR-V/LLVM Translator library interface, if needed, but it does not belong in this repository. It certainly shouldn't be enabled by default, given that 1) this is an optimization beyond the scope of the translator and 2) the translator needs to support consumers of SPIR-V that do not support the IDP extension.

[StuartDBrady]

At first glance, the description led me to believe this was an optimization which was using pattern matching to detect cases where a floating point dot product has been used with floating point vectors that have been converted from integer vectors, i.e. optimizing OpDot(OpConvertStoF(some_int)) → OpSDot(some_int).

I now see that this is not what was intended, but that this is a correctness fix for translation of IR produced from source using the cl_khr_integer_dot_product extension. I haven't looked very closely at the tests, but the code looks fine as far as I can see. However, I'd request that the commit message and description be updated to make it clearer that this is adding support for cl_khr_integer_dot_product, rather than adding an optimization.

[svenvh]

Like @StuartDBrady I initially also got misled by the description. As I understand now, this is not an optimization but actually fixing a bug in the translator: it currently translates calls such as dot(uchar4, uchar4) into OpDot %i8, which is wrong because OpDot only operates on floating point types. Clarifying in the description that the current behaviour is wrong would be great, as Stuart mentioned.

I think we should work towards accepting this PR and am keen to help; we've left this for too long!

@Quetzonarch I guess first you should try to rebase onto latest main and then you'll probably hit some issues. I have left some comments that should help you solve some problems that I expect will appear after rebasing, plus some other improvements.

[Quetzonarch]

Thank you for the comments and suggestions regarding the change. Yes, the point of this is to fix incorrect translations of IR produced from source using the cl_khr_integer_dot_product extension, sorry for the misleading description. I've begun implementing suggested changes. I'll try to push the update as soon as possible.

[Quetzonarch]

I've rebased the branch onto the latest main and applied suggested changes. Everything seemed to work alright, but some parts might need some additional checking (for example my use of mutateCallInstSPIRV function, as I wasn't too familiar with that one).

[svenvh]

Thanks for the update @Quetzonarch! I've left a few more comments and the CI has found some issues; other than that it looks good to me.

[Quetzonarch]

I've seen that CI had found some issues, so I've applied small changes to the last commit (slight code formatting in OCLToSPIRV and missing spir_func words in tests).