Skip to content

Conversation

fda0
Copy link

@fda0 fda0 commented May 22, 2025

12 commits that were needed to cleanly apply 9b00b88 which fixes Cooperative Matrix prefetch.

MrSidims and others added 12 commits May 22, 2025 16:48
KhronosGroup#1835)

It specifies how to interpret 'Component Type' when components of a joint matrix are storages for values of different types, for example float for TF32, unsigned short for bfloat16.

At this point only tf32 type interpretation is added during SPIR-V generation. Adding it to bf16 is a breaking change and
requires adaptation across drivers.

Spec update:
intel/llvm#8175

Signed-off-by: Sidorov, Dmitry [email protected]
(cherry picked from commit b7c5218)
…onosGroup#1913)

Extension name will be preserved for a while for binary compatibility.

Signed-off-by: Sidorov, Dmitry <[email protected]>
(cherry picked from commit 68855f6)
…operative matrices (KhronosGroup#2213)

Previously added scalar/vector ConvertFToBF16INTEL, ConvertBF16ToFINTEL
and RoundFToTF32INTEL conversions are now enabled for cooperative matrix
type under SPV_INTEL_joint_matrix extension following the spec:
https://github.com/intel/llvm/blob/sycl/sycl/doc/design/spirv-extensions/SPV_INTEL_joint_matrix.asciidoc

Note, joint matrices are not allowed as input/output for these
conversions as it is being deprecated.

Signed-off-by: Sidorov, Dmitry <[email protected]>
(cherry picked from commit 1010efc)
KhronosGroup#2214)

This PR aims to introduce entities related to OpCooperativeMatrixApplyFunctionINTEL in llvm-spirv translator, according to https://github.com/intel/llvm/blob/sycl/sycl/doc/design/spirv-extensions/SPV_INTEL_joint_matrix.asciidoc.

Co-authored-by: Sidorov, Dmitry <[email protected]>
(cherry picked from commit 467edf9)
…n type (KhronosGroup#1995)

This target extension type is created here: https://github.com/intel/vc-intrinsics/blob/master/GenXIntrinsics/lib/GenXIntrinsics/GenXSPIRVWriterAdaptor.cpp#L245

As with other target extension types, reverse translation is not yet supported.

Signed-off-by: Sarnie, Nick <[email protected]>
Co-authored-by: Victor Mustya <[email protected]>
(cherry picked from commit 60746d5)
(cherry picked from commit ac6aa17dc92885d9ecbe65fb7d0fb6f75ace2b9e)
…Group#2088)

This patch adds joint_matrix reverse translation to
target extension type and starts rewriting all of the tests.

Some tests are being removed as outdated

Remaining tests to add after the patch:
1. tf32 test
2. element wise operations test

Signed-off-by: Sidorov, Dmitry <[email protected]>
(cherry picked from commit 465eb3c)
…onosGroup#1935)

The patch adds TypeJointMatrixINTELv2 which maps to new type OpCode
6184. Under new OpCode matrix type no longer has Layout parameter. The patch also moved 'scope' to optional matrix muladd instruction.

The changes are done only in the consumer part to prepare the switch and make E2E switch backward compatible by preparing consumers ahead of time.

Unfortunately there is no way to add a test foe this unless it's binary test, but it seems to be a bit unsafe to add this, so the patch was tested locally.

Spec change:
intel/llvm#8175

Signed-off-by: Sidorov, Dmitry <[email protected]>
(cherry picked from commit a6fcade)
…2143)

visit method of the sort relies on getNonLiteralOperands method of the
SPIRVType which is being inserted in the module. Without it dependent
types can be inserted in the module in incorrect order.

For example:
TypeCooperativeMatrixKHR %ID%
TypeStruct ... %ID%
is the correct order, but without the patch in some cases the translator
could generate the opposite order.

Signed-off-by: Sidorov, Dmitry <[email protected]>
(cherry picked from commit 436c497)
…ope parameters (KhronosGroup#2223)

Use should be: MatrixA, MatrixB or Accumulator.
Scope must be at max Invocation (others are not supported
by the translator).

Signed-off-by: Sidorov, Dmitry <[email protected]>
(cherry picked from commit f18e64d)
…2234)

- change Scope argument to one of two available options: ScopeWorkgroup/ScopeWorkgroup
- fix arguments order in calls to OpCooperativeMatrixLoadKHR()

(cherry picked from commit 4e1d3e0)
@fda0 fda0 changed the title [Backport to 16] Add ComponentTypeInterpretation for joint matrix type (#1835) [Backport to 16] Add multiple missing Cooperative Matrix commits May 22, 2025
@fda0
Copy link
Author

fda0 commented May 22, 2025

It would be best to avoid squashing for this PR.

@michalpaszkowski michalpaszkowski requested a review from MrSidims May 22, 2025 18:59
Copy link
Contributor

@MrSidims MrSidims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't harm to merge the PR as is, yet may be it's better to discuss few things in advance.

(unsigned)S};
if (auto *Use = MT->getUse())
Params.push_back(static_cast<SPIRVConstant *>(Use)->getZExtIntValue());
auto *CTI = MT->getComponentTypeInterpretation();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch contradicts current SPV_INTEL_joint_matrix specification, where type interpretation is a part of MulAdd. Note, IGC also expects type interpretation be a part of MulAdd and not a part of the type. Feel free to IM me to discuss this.

Copy link
Contributor

@MrSidims MrSidims May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mateuszchudyk please follow up with @vmaksimo about this comment (I'm OOO for the next 2 weeks), she will point out on spec changes and IGC patch, that adds type interpretation support.
Basically I'm fine to merge it as is, but please make sure, that we know, what we are doing with matrix special types. Please also note, that there is no such thing as Int4 interpretation (it is now a proper TypeInt 4 - see SPV_INTEL_int4 (this is to be backported by us soon)) and Int2 interpretation (and there won't be any counterpart).

const static char ConstantPipeStorage[] = "ConstantPipeStorage";
const static char VmeImageINTEL[] = "VmeImageINTEL";
const static char JointMatrixINTEL[] = "JointMatrixINTEL";
const static char BufferSurfaceINTEL[] = "BufferSurfaceINTEL";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sarnex @vmustya please correct me if I'm wrong - this is being deprecated, right?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sarnex @vmustya any updates on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, yes as far as I know it's being deprecated but @vmustya should confirm.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, the BufferSurfaceINTEL stateful raw buffers are still used for some workloads.

Decoder >> Id >> CompType >> Args;
}

void SPIRVTypeCooperativeMatrixKHR::validate() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's needed for IGC, but guess it's added just to avoid merge conflicts, right?

@MrSidims MrSidims requested a review from vmaksimo May 26, 2025 16:06
@mateuszchudyk
Copy link
Contributor

mateuszchudyk commented May 26, 2025

@fda0 is out of office until 23rd of June. I'll be the owner of this PR during that time.

EDIT: Due to other high priority tasks I have, this PR will wait until @fda0 returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants