[SYCL] Represent JointMatrixINTEL type as extension type #8343

MrSidims · 2023-02-14T15:50:19Z

This patch is build on top of https://reviews.llvm.org/D141008

It adds:
ConvertSYCLJointMatrixINTELType - Convert SYCL joint_matrix type
which is represented as a pointer to a structure to LLVM extension type
with the parameters that follow SPIR-V JointMatrixINTEL type.
The expected representation is:
target("spirv.JointMatrixINTEL", %element_type, %rows%, %cols%, %scope%,
%use%, (optional) %element_type_interpretation%)

Better approach is to introduce joint matrix type to clang, but it's off the table now, since we are lacking OpenCL
spec.

The SPIR-V spec

MrSidims · 2023-02-14T15:51:01Z

@jcranmer-intel please take a look if implementation like this makes sense

MrSidims · 2023-02-14T15:55:31Z

llvm/docs/SPIRVUsage.rst

+     OpTypeReserveId    ``spirv.ReserveId``    (none)
+     OpTypeQueue        ``spirv.Queue``        (none)
+     OpTypePipe         ``spirv.Pipe``         access qualifier
+     OpTypePipeStorage  ``spirv.PipeStorage``  (none)


Probably, JointMatrixINTEL should also be mentioned here at least in intel/llvm repo

jcranmer-intel · 2023-02-15T22:29:22Z

clang/lib/CodeGen/CodeGenTypes.cpp

+      // 'tf32' interpretation is mapped to '0'
+      return getJointMatrixINTELExtType<true>(CompTy, TemplateArgs, 0);
+    } else if (LlvmTyName == "bfloat16") {
+      CompTy = llvm::Type::getInt16Ty(getLLVMContext());


LLVM has a bfloat type, so surely it would make more sense to use that instead of i16? Or is there something about the JointMatrixINTEL spec that I'm missing?

The thing is that in SPIR-V we don't have bfloat type (yet?). Instead we have conversion instructions and using short value as bfloat16 storage. Same for SYCL - here bfloat16 class is a wrapper around int16 storage. So for the consistency int16 is used here as well, though there is no other reason not to reuse LLVM's bf16 (just because we still will replace it with int16 in SPIR-V). So I lean towards appying this suggestion.

Actually, lets keep i16 to be aligned with SPIR-V spec. Once we have proper SPIR-V bf16 type we can start generating in by DPC++ compiler either here or elsewhere.

) The expected representation is: target("spirv.JointMatrixINTEL", %element_type, %rows%, %cols%, %scope%, %use%, (optional) %element_type_interpretation%) TODO: figure out, how to deal with the switch from old API (Matrix has Layout) to new API (Layout was removed) Depends on: #1799 intel/llvm#8343

The expected representation is: target("spirv.JointMatrixINTEL", %element_type, %rows%, %cols%, %scope%, %use%, (optional) %element_type_interpretation%) TODO: figure out, how to deal with the switch from old API (Matrix has Layout) to new API (Layout was removed) Depends on: intel#1799 intel#8343 Original commit: KhronosGroup/SPIRV-LLVM-Translator@ee03f5f

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

MrSidims · 2023-04-03T16:43:38Z

@intel/dpcpp-cfe-reviewers please take a look

Fznamznon · 2023-04-04T10:30:17Z

So, why don't we use built-in type instead of parsing the Matrix type from headers?

MrSidims · 2023-04-04T11:20:47Z

So, why don't we use built-in type instead of parsing the Matrix type from headers?

Which one? I don't see the conversation resolved. Until then we need to unblock switch to opaque pointers.

Fznamznon

Well, that is just a switch from one fragile solution to another. A couple of nits to comments, otherwise lgtm.

clang/lib/CodeGen/CodeGenTypes.cpp

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

MrSidims · 2023-04-11T16:13:15Z

@intel/llvm-reviewers-runtime please take a look at test changes in sycl directory

dm-vodopyanov

sycl/test/matrix changes LGTM

sycl/test/matrix/legacy/matrix-int8-test.cpp

sycl/test/matrix/matrix-int8-test.cpp

MrSidims · 2023-04-12T09:34:31Z

clang/test/CodeGenSYCL/matrix.cpp

-// CHECK: @_Z2f2{{.*}}(%spirv.JointMatrixINTEL._long_10_2_0_0
-void f2(__spv::__spirv_JointMatrixINTEL<uint64_t, 10, 2, 0, 0> *matrix) {}
+// CHECK: @_Z2f2{{.*}}(target("spirv.JointMatrixINTEL", i64, 10, 2, 0, 0, 0)
+void f2(__spv::__spirv_JointMatrixINTEL<uint64_t, 10, 2, 0, 0, 0> *matrix) {}


@aelovikov-intel here is the test for unsigned. Would you mind if I won't duplicate it in sycl headers?

Hmm, I'm not sure what's the purpose of the SYCL RT tests then, but yes, no need for unsigned there.

Well, matrix API is changing overtime, so having several compilation tests using real headers is good to have. One the extension is stable we will remove them (note, the E2E tests were in different repo and require specific hardware)

MrSidims · 2023-04-12T17:40:07Z

@intel/llvm-gatekeepers please help with merge. From what I see
Failed Tests (1):
SYCL :: Basic/memory-consumption.cpp

and ESIMD tests

are known issues reported elsewhere

steffenlarsen · 2023-04-12T21:24:18Z

@MrSidims - Could you please push a merge commit? Most of these failures should have been fixed on tip, so hopefully we should see fewer CI failures now.

MrSidims · 2023-04-12T21:51:44Z

@MrSidims - Could you please push a merge commit? Most of these failures should have been fixed on tip, so hopefully we should see fewer CI failures now.

sure, done!

…el#8343)" It appears to be, that mem2reg and SROA passes can't handle target extension type properly. It means, that with turned on optimizations alloca/load/store sequences of joint matrix types won't be eliminated. It results in a crash in IGC since it can't handle such case yet. Note, it means that matrix samples compiled with -O0 also don't work now. So we have to (temporary?) revert this patch. This reverts commit 6f8e456.

…9841) This reverts commit 4447a50. Previous attempt: #8343 What changed: One extra patch is being added to the headers: ca0595b with this patch clang won't generate llvm.memcpy for trivial c'tor. So later on inst combine won't replace it with a cast to i64 followed by load + store which SROA + mem2reg won't be able to handle for target extension types. It adds: ConvertSYCLJointMatrixINTELType - Convert SYCL joint_matrix type which is represented as a pointer to a structure to LLVM extension type with the parameters that follow SPIR-V JointMatrixINTEL type. The expected representation is: target("spirv.JointMatrixINTEL", %element_type, %rows%, %cols%, %scope%, %use%, (optional) %element_type_interpretation%) Better approach is to introduce joint matrix type to clang, but it's off the table now, since we are lacking OpenCL spec. Co-authored-by: Joshua Cranmer <joshua.cranmer@intel.com> --------- Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com> Co-authored-by: Alexey Bader <alexey.bader@intel.com>

…ntel#9841) This reverts commit 4447a50. Previous attempt: intel#8343 What changed: One extra patch is being added to the headers: intel@ca0595b with this patch clang won't generate llvm.memcpy for trivial c'tor. So later on inst combine won't replace it with a cast to i64 followed by load + store which SROA + mem2reg won't be able to handle for target extension types. It adds: ConvertSYCLJointMatrixINTELType - Convert SYCL joint_matrix type which is represented as a pointer to a structure to LLVM extension type with the parameters that follow SPIR-V JointMatrixINTEL type. The expected representation is: target("spirv.JointMatrixINTEL", %element_type, %rows%, %cols%, %scope%, %use%, (optional) %element_type_interpretation%) Better approach is to introduce joint matrix type to clang, but it's off the table now, since we are lacking OpenCL spec. Co-authored-by: Joshua Cranmer <joshua.cranmer@intel.com> --------- Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com> Co-authored-by: Alexey Bader <alexey.bader@intel.com>

…get ext type The expected representation is: target("spirv.JointMatrixINTEL", %element_type, %rows%, %cols%, %scope%, %use%, (optional) %element_type_interpretation%) TODO: figure out, how to deal with the switch from old API (Matrix has Layout) to new API (Layout was removed) Depends on: intel/llvm#8343

MrSidims requested a review from jcranmer-intel February 14, 2023 15:50

MrSidims changed the title ~~Try matrix opaque~~ [SYCL] Represent JointMatrixINTEL type as extension type Feb 14, 2023

MrSidims requested a review from dkhaldi February 14, 2023 15:51

MrSidims commented Feb 14, 2023

View reviewed changes

MrSidims temporarily deployed to aws February 15, 2023 12:38 — with GitHub Actions Inactive

MrSidims mentioned this pull request Feb 15, 2023

Split JointMatrixMadINTEL instruction into 4 KhronosGroup/SPIRV-LLVM-Translator#1833

Merged

dkhaldi requested a review from yubingex007-a11y February 15, 2023 16:26

jcranmer-intel reviewed Feb 15, 2023

View reviewed changes

MrSidims mentioned this pull request Feb 21, 2023

[OpaquePointers] Add support for JointMatrixINTEL target ext type KhronosGroup/SPIRV-LLVM-Translator#1852

Merged

[SYCL] Represent JointMatrixINTEL type as extension type

b9529a6

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

MrSidims force-pushed the try-matrix-opaque branch from 7707bb0 to b9529a6 Compare March 31, 2023 12:37

Apply clang-format

366c3e3

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

MrSidims marked this pull request as ready for review March 31, 2023 12:46

MrSidims requested a review from a team as a code owner March 31, 2023 12:46

MrSidims temporarily deployed to aws March 31, 2023 13:05 — with GitHub Actions Inactive

Fix issue and test

5d16304

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

MrSidims requested a review from a team as a code owner March 31, 2023 15:40

MrSidims requested a review from bso-intel March 31, 2023 15:40

MrSidims temporarily deployed to aws March 31, 2023 15:59 — with GitHub Actions Inactive

check for RD

3edb1bd

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

MrSidims temporarily deployed to aws April 3, 2023 13:49 — with GitHub Actions Inactive

MrSidims temporarily deployed to aws April 3, 2023 14:22 — with GitHub Actions Inactive

Fznamznon approved these changes Apr 6, 2023

View reviewed changes

clang/lib/CodeGen/CodeGenTypes.cpp Outdated Show resolved Hide resolved

clang/lib/CodeGen/CodeGenTypes.cpp Outdated Show resolved Hide resolved

clang/lib/CodeGen/CodeGenTypes.cpp Outdated Show resolved Hide resolved

Typos fixes

8d29f3c

Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com>

MrSidims temporarily deployed to aws April 6, 2023 16:02 — with GitHub Actions Inactive

MrSidims temporarily deployed to aws April 6, 2023 16:33 — with GitHub Actions Inactive

MrSidims temporarily deployed to aws April 8, 2023 11:07 — with GitHub Actions Inactive

MrSidims temporarily deployed to aws April 9, 2023 12:24 — with GitHub Actions Inactive

Merge remote-tracking branch 'origin/sycl' into try-matrix-opaque

055c129

MrSidims temporarily deployed to aws April 10, 2023 22:23 — with GitHub Actions Inactive

MrSidims temporarily deployed to aws April 11, 2023 00:05 — with GitHub Actions Inactive

dm-vodopyanov approved these changes Apr 11, 2023

View reviewed changes

aelovikov-intel reviewed Apr 11, 2023

View reviewed changes

sycl/test/matrix/legacy/matrix-int8-test.cpp Show resolved Hide resolved

sycl/test/matrix/matrix-int8-test.cpp Show resolved Hide resolved

Merge remote-tracking branch 'origin/sycl' into try-matrix-opaque

3e8d12a

MrSidims commented Apr 12, 2023

View reviewed changes

aelovikov-intel approved these changes Apr 12, 2023

View reviewed changes

MrSidims requested a review from a team April 12, 2023 17:35

Merge remote-tracking branch 'origin/sycl' into try-matrix-opaque

a6e4e23

MrSidims temporarily deployed to aws April 12, 2023 22:44 — with GitHub Actions Inactive

MrSidims temporarily deployed to aws April 12, 2023 23:52 — with GitHub Actions Inactive

bader merged commit 6f8e456 into intel:sycl Apr 13, 2023

MrSidims mentioned this pull request Jun 13, 2023

[SYCL] Re-land "Represent JointMatrixINTEL type as extension type" #9841

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL] Represent JointMatrixINTEL type as extension type #8343

[SYCL] Represent JointMatrixINTEL type as extension type #8343

MrSidims commented Feb 14, 2023 •

edited by bader

Loading

MrSidims commented Feb 14, 2023

MrSidims Feb 14, 2023

jcranmer-intel Feb 15, 2023

MrSidims Feb 20, 2023 •

edited

Loading

MrSidims Mar 31, 2023

MrSidims commented Apr 3, 2023

Fznamznon commented Apr 4, 2023

MrSidims commented Apr 4, 2023

Fznamznon left a comment

MrSidims commented Apr 11, 2023

dm-vodopyanov left a comment

MrSidims Apr 12, 2023

aelovikov-intel Apr 12, 2023

MrSidims Apr 12, 2023 •

edited

Loading

MrSidims commented Apr 12, 2023

steffenlarsen commented Apr 12, 2023

MrSidims commented Apr 12, 2023

[SYCL] Represent JointMatrixINTEL type as extension type #8343

[SYCL] Represent JointMatrixINTEL type as extension type #8343

Conversation

MrSidims commented Feb 14, 2023 • edited by bader Loading

MrSidims commented Feb 14, 2023

MrSidims Feb 14, 2023

Choose a reason for hiding this comment

jcranmer-intel Feb 15, 2023

Choose a reason for hiding this comment

MrSidims Feb 20, 2023 • edited Loading

Choose a reason for hiding this comment

MrSidims Mar 31, 2023

Choose a reason for hiding this comment

MrSidims commented Apr 3, 2023

Fznamznon commented Apr 4, 2023

MrSidims commented Apr 4, 2023

Fznamznon left a comment

Choose a reason for hiding this comment

MrSidims commented Apr 11, 2023

dm-vodopyanov left a comment

Choose a reason for hiding this comment

MrSidims Apr 12, 2023

Choose a reason for hiding this comment

aelovikov-intel Apr 12, 2023

Choose a reason for hiding this comment

MrSidims Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

MrSidims commented Apr 12, 2023

steffenlarsen commented Apr 12, 2023

MrSidims commented Apr 12, 2023

MrSidims commented Feb 14, 2023 •

edited by bader

Loading

MrSidims Feb 20, 2023 •

edited

Loading

MrSidims Apr 12, 2023 •

edited

Loading