Skip to content

[SYCL] Re-enable several joint matrix E2E tests #15473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

[SYCL] Re-enable several joint matrix E2E tests #15473

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a2faa9f
Fix device assertion bug on windows
lbushi25 Aug 29, 2024
5bac94b
Remove XFAIL from some Matrix tests and add UNSUPPORTED for dg2 arch
lbushi25 Sep 23, 2024
f2bc9ed
Merge branch 'sycl' into reenable_matrix_tests
lbushi25 Sep 23, 2024
18ef16d
Add comment that VNNI transform and sub-group size of 32 not supporte…
lbushi25 Sep 23, 2024
d8b2f4d
Add comment that VNNI transform and sub-group size of 32 not supporte…
lbushi25 Sep 23, 2024
369cf10
Add comment that VNNI transform and sub-group size of 32 not supporte…
lbushi25 Sep 23, 2024
01644e9
Add comment that VNNI transform and sub-group size of 32 not supporte…
lbushi25 Sep 23, 2024
fca17b1
Add
lbushi25 Sep 23, 2024
dc3f925
Add comment that VNNI transform and sub-group size of 32 not supporte…
lbushi25 Sep 23, 2024
214af8b
Add comment that VNNI transform and sub-group size of 32 not supporte…
lbushi25 Sep 23, 2024
02c750e
Add UNSUPPORTED directive for joint matrix tests on PVC
lbushi25 Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions sycl/test-e2e/Matrix/SG32/joint_matrix_annotated_ptr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// VNNI transform and sub-group size 32 are not supported yet on DG2 by IGC
// UNSUPPORTED: gpu-intel-dg2
// REQUIRES: aspect-ext_intel_matrix
// REQUIRES-INTEL-DRIVER: lin: 27501, win: 101.4943

// RUN: %{build} -o %t.out
// RUN: %{run} %t.out

// Currently row major B fails when annotated_ptr is used
// XFAIL: gpu
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024

Choose a reason for hiding this comment

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

Does this test pass on PVC? I did not see any unexpected pass on PVC for this test...

Copy link
Contributor Author

@lbushi25 lbushi25 Sep 23, 2024

Choose a reason for hiding this comment

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

I forgot to test this on PVC. I just tested it now and yes you are right, it fails.
I'm adding an unsupported directive for pvc on these tests.

Copy link
Contributor

@dkhaldi dkhaldi Sep 23, 2024

Choose a reason for hiding this comment

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

The test is supported on PVC but it is XFAIL.
So you should keep both:
// UNSUPPORTED: gpu-intel-dg2
and
// XFAIL: gpu
(same goes to the other tests)


#include "../common.hpp"

#define SG_SZ 32
Expand Down
4 changes: 2 additions & 2 deletions sycl/test-e2e/Matrix/SG32/joint_matrix_bfloat16_colmajorA_colmajorB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// VNNI transform and sub-group size 32 are not supported yet on DG2 by IGC
// UNSUPPORTED: gpu-intel-dg2
// REQUIRES: aspect-ext_intel_matrix
// REQUIRES-INTEL-DRIVER: lin: 27501, win: 101.4943

Expand All @@ -14,8 +16,6 @@
// This tests support of col major layout for matrix B which does transpose and
// then VNNI transform. This is currently only available on AMX

// XFAIL: gpu
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024

Choose a reason for hiding this comment

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

Does this test pass on PVC? I did not see any unexpected pass on PVC for this test...


#include "../common.hpp"
#include <iostream>
#include <sycl/detail/core.hpp>
Expand Down
4 changes: 2 additions & 2 deletions sycl/test-e2e/Matrix/SG32/joint_matrix_colA_rowB_colC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// VNNI transform and sub-group size 32 are not supported yet on DG2 by IGC
// UNSUPPORTED: gpu-intel-dg2
// REQUIRES: aspect-ext_intel_matrix
// REQUIRES-INTEL-DRIVER: lin: 27501, win: 101.4943

// RUN: %{build} -o %t.out
// RUN: %{run} %t.out

// XFAIL:*
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024

Choose a reason for hiding this comment

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

Does this test pass on PVC? I did not see any unexpected pass on PVC for this test...


#include "../common.hpp"

#define SG_SZ 32
Expand Down
4 changes: 2 additions & 2 deletions sycl/test-e2e/Matrix/SG32/joint_matrix_int8_colmajorA_colmajorB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// VNNI transform and sub-group size 32 are not supported yet on DG2 by IGC
// UNSUPPORTED: gpu-intel-dg2
// REQUIRES: aspect-ext_intel_matrix
// REQUIRES-INTEL-DRIVER: lin: 27501, win: 101.4943

Expand All @@ -14,8 +16,6 @@
// This tests support of col major layout for matrix B which does transpose and
// then VNNI transform. This is currently only available on AMX

// XFAIL: gpu
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024

Choose a reason for hiding this comment

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

Does this test pass on PVC? I did not see any unexpected pass on PVC for this test...


#include "../common.hpp"

using namespace sycl;
Expand Down
4 changes: 2 additions & 2 deletions sycl/test-e2e/Matrix/joint_matrix_bfloat16_colmajorA_colmajorB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// VNNI transform and sub-group size 32 are not supported yet on DG2 by IGC
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

sub-group size 32 part is not relevant for this test. VNNI transform part needs to be more specific

// UNSUPPORTED: gpu-intel-dg2
// REQUIRES: aspect-ext_intel_matrix

// RUN: %{build} -o %t.out
Expand All @@ -13,8 +15,6 @@
// This tests support of col major layout for matrix B which does transpose and
// then VNNI transform. This is currently only available on AMX

// XFAIL: gpu
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024

Choose a reason for hiding this comment

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

Does this test pass on PVC? I did not see any unexpected pass on PVC for this test...


#include "common.hpp"

constexpr size_t TN = 16;
Expand Down
4 changes: 2 additions & 2 deletions sycl/test-e2e/Matrix/joint_matrix_colA_rowB_colC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// VNNI transform and sub-group size 32 are not supported yet on DG2 by IGC
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

sub-group size 32 part is not relevant for this test. To avoid confusion, I suggest mention only VNNI transform is not supported.

Edit: also, after looking at implementation I see that some VNNI transform is implemented on DG2 in IGC, so in general this statement is not correct. It needs to be more specific. Same is true for other messages below, which mention VNNI transform is not supported.

// UNSUPPORTED: gpu-intel-dg2
// REQUIRES: aspect-ext_intel_matrix

// RUN: %{build} -o %t.out
// RUN: %{run} %t.out

// XFAIL:*
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024

Choose a reason for hiding this comment

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

Does this test pass on PVC? I did not see any unexpected pass on PVC for this test...


#include "common.hpp"

constexpr size_t TN = 16;
Expand Down
4 changes: 2 additions & 2 deletions sycl/test-e2e/Matrix/joint_matrix_int8_colmajorA_colmajorB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// VNNI transform and sub-group size 32 are not supported yet on DG2 by IGC
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

sub-group size 32 part is not relevant for this test. VNNI transform part of the statement needs to be more specific.

// UNSUPPORTED: gpu-intel-dg2
// REQUIRES: aspect-ext_intel_matrix

// RUN: %{build} -o %t.out
Expand All @@ -13,8 +15,6 @@
// This tests support of col major layout for matrix B which does transpose and
// then VNNI transform. This is currently only available on AMX

// XFAIL: gpu
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin Sep 23, 2024

Choose a reason for hiding this comment

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

Does this test pass on PVC? I did not see any unexpected pass on PVC for this test...


#include "common.hpp"

constexpr size_t TN = 16;
Expand Down
Loading