[Matrix] Confusing encoding of RowMajor / ColMajor layout

[masahi]

Hi, SPV_INTEL_joint_matrix.asciidoc#matrix-layout defines the numerical encoding of RowMajor / ColMajor matrix layout. But in

llvm/sycl/include/CL/__spirv/spirv_types.hpp

Lines 120 to 121 in f4a9ef1

	RowMajor = 0,
	ColumnMajor = 1,

, these values are swapped. So if I use matrix_layout::row_major in my sycl program, I get 0 as the encoding of the layout.

On the IGC side, there are two conflicting definitions of this encoding:
https://github.com/intel/intel-graphics-compiler/blob/4eb6e950f5a617dadf1055253e4120afa57b7433/IGC/AdaptorOCL/SPIRV/libSPIRV/SPIRVType.h#L717-L718
https://github.com/intel/intel-graphics-compiler/blob/a8afe42bd544defeacb0ef88f79c40432b5f82b4/IGC/Compiler/Optimizer/OpenCLPasses/JointMatrixFuncsResolutionPass.cpp#L82-L83

So can I get a clarification of this situation?

Off topic, but I'd also like to know when SPV_INTEL_joint_matrix will become available in the official SPIRV tool chain. I've been updating a home-grown SPIRV code generator (in TVM) to support this extension, and not being able to use the validator has been painful.

[MrSidims]

FYI @dkhaldi @yubingex007-a11y @mbelicki

@masahi thanks for the report! It seems like a bug in our implementation, though not sure whether it would possible to swap enum values without breaking ABI, so probably the spec will be aligned.

Regarding IGC: the first link is a fork of https://github.com/KhronosGroup/SPIRV-LLVM-Translator , basically what matters for the translation is that what value is passed through it, how to interpret this value is up to IGC BE (the second link) and there enum matches the one from SYCL headers. So even if it's still quite incorrect I don't expect seeing functional and performance issues on GPU related to this bug.

Same for Intel CPU, as far as I see the order is the same as in the headers and not as in SPIR-V spec.

@yubingex007-a11y @mbelicki please correct if I'm missing something.

Off topic, but I'd also like to know when SPV_INTEL_joint_matrix will become available in the official SPIRV tool chain.

I can't tell exactly when, but we acknowledge this problem (it's not unique for matrix extension, several others also have it) and are trying to solve it as soon as possible. There is also an Khronos internal discussion about cooperative/joint matrix extension, it's under IP so can't share the detail, but if you have access you can try to find this discussion yourself.

[MrSidims]

After some discussion we consider it as the specification bug - the intention always was to make the enumeration like in the implementation, not sure how different enumeration appeared in the spec. I've updated #5944 to solidify this (though this very PR probably won't ever be merged) and will update our internal next-iteration spec (hope it will be public soon, after this we plan to upstream the changes to SPIR-V Headers and hence Tools). The SPIR-V Translator will be updated as well (just naming of variables and enum members).

Thanks again for the report!

[MrSidims]

@masahi not 100% final, but a much improved spec can be found here: #8175 . It also addresses the issue.

[masahi]

Closing since #8175 was merged. Thanks! @MrSidims