[SYCL][Joint Matrix][E2E] Uncomment the Joint Matrix tests for combination 32x32x16 #16191

YixingZhang007 · 2024-11-26T21:58:10Z

Description:
The support for combination 32x32x16 on DG2 has been implemented in commit intel/intel-graphics-compiler@6a06c93. This PR re-enables the Joint Matrix tests for the combination 32x32x16, which were previously unsupported.

This PR also adds XFAIL: gpu to two tests, joint_matrix_bf16_fill_k_cache_arg_dim.cpp and joint_matrix_bf16_fill_k_cache_runtime_dim.cpp, because the driver has not yet been updated to include the necessary changes in IGC to support these tests. As a result, these tests should not be tested in CI at this time.

dkhaldi

LGTM

YixingZhang007 · 2024-11-30T02:39:18Z

@intel/llvm-gatekeepers This PR has been approved and all the CI tests have passed. Could you please help merge the PR? Thanks for helping :)

maarquitos14 · 2024-12-02T09:38:11Z

sycl/test/e2e_test_requirements/no-xfail-without-tracker.cpp

 // (i.e. reduce) the number and the list below.
 //
-// NUMBER-OF-XFAIL-WITHOUT-TRACKER: 77
+// NUMBER-OF-XFAIL-WITHOUT-TRACKER: 79


Quoting from line 43 from this very same file:

// That number *must not* increase. Any PR which causes this number to grow // should be rejected and it should be updated to either keep the number as-is // or have it reduced (preferably, down to zero).

Please, read carefully the header of this file to understand what you need to do. Basically, you cannot add new XFAIL directives without a tracker to address it.

Sorry about the mistake! Thanks for pointing it out :)
I have removed the change in file no-xfail-without-tracker.cpp, and added XFAIL-TRACKER to the tests.

dkhaldi · 2024-12-02T15:15:07Z

sycl/test-e2e/Matrix/joint_matrix_bf16_fill_k_cache_arg_dim.cpp


 // -ffp-model=precise is added to not depend on compiler defaults.

+// XFAIL: gpu


Please add
// XFAIL-TRACKER: CMPLRLLVM-63710
so you don't increase number of xfail without tracker

Also, should you add the xfail on only !igc-dev?

Thanks for the suggestion! I have made the change.
These two tests are not supported in the igc-dev CI test because the commit in IGC has not yet been integrated into the driver. I will remove the XFAIL once the IGC change has been pull downed.

@YixingZhang007 ,
looking at the branch that is used for igc-dev here https://github.com/intel/llvm/blob/sycl/devops/dependencies-igc-dev.json

it should include your changes for runtime dimension support:
https://github.com/intel/intel-graphics-compiler/commits/6ee988a5948c6aaecbfe00605f52694c55cf1052/IGC/Compiler/Optimizer/OpenCLPasses/JointMatrixFuncsResolutionPass/JointMatrixFuncsResolutionPass.cpp

So, tests should be passing already with igc-dev in intel/llvm CI.
What am I missing?

I’ve noticed that the igc_dev CI test is not reflecting the IGC commits it's supposed to be testing. It seems to still be using an older version of IGC, likely the version set for the regular IGC.

I’m experiencing the same issue with another LLVM PR (#15880), where my IGC commit should already be integrated into igc_dev, but the tests get_coordinate_ops.cpp is still failing due to errors that should have been addressed by that IGC commit. As a temporary approach, I’ve added XFAIL to the test while we wait for this issue to be resolved or until the regular IGC is updated to include the changes.

Please let me know if this approach would work and if you have any suggestions! Thanks!

@YixingZhang007 ,
"while we wait for this issue to be resolved" - does it mean someone is working on resolving the issue? If not, could you please submit the issue to https://github.com/intel/llvm/issues, so that it is reported, and we can assign someone to work on it?
@jsji , @sarnex , FYI

I'm OK with adding XFAIL, since we have this problem.

They are working on something called IGC 2 I think, so maybe they enabled it by default?

intel/intel-graphics-compiler@4617a3e seems intended to me

I have created an issue #16251 regarding this.

If so, we need to make sure we softlink to libigc.so to libigc.so.2 for igc-dev now.

yeah im gonna look at the issue and try to see what's going on, i gotta fix igc-dev containers first

dkhaldi · 2024-12-02T15:15:22Z

sycl/test-e2e/Matrix/joint_matrix_bf16_fill_k_cache_runtime_dim.cpp

 // -ffp-model=precise is added to not depend on compiler defaults.

+// XFAIL: gpu
+


Please add
// XFAIL-TRACKER: CMPLRLLVM-63710
so you don't increase number of xfail without tracker

Thanks! I have updated the test.

dkhaldi · 2024-12-02T15:15:45Z

sycl/test/e2e_test_requirements/no-xfail-without-tracker.cpp

 // CHECK-NEXT: Matrix/SG32/joint_matrix_rowmajorA_rowmajorB.cpp
 // CHECK-NEXT: Matrix/SG32/joint_matrix_unaligned_k.cpp
+// CHECK-NEXT: Matrix/joint_matrix_bf16_fill_k_cache_arg_dim.cpp
+// CHECK-NEXT: Matrix/joint_matrix_bf16_fill_k_cache_runtime_dim.cpp


Remove these two addition as you add the trackers in the tests

dkhaldi

LGTM

YixingZhang007 · 2024-12-03T21:03:53Z

This PR would be merged for now, marking the tests joint_matrix_bf16_fill_k_cache_runtime_dim.cpp and joint_matrix_bf16_fill_k_cache_arg_dim.cpp with XFAIL: gpu. Once the igc-dev CI issue is resolved, it will be updated to XFAIL: !igc-dev.

YixingZhang007 · 2024-12-03T21:04:30Z

@intel/llvm-gatekeepers This PR has been approved and all the CI tests have passed. Could you please help merge the PR? Thanks for helping :)

YixingZhang007 requested a review from a team as a code owner November 26, 2024 21:58

YixingZhang007 changed the title ~~[SYCL][Joint Matrix][E2E] Uncomment the Joint Matrix tests for combination 32x32x16 combination~~ [SYCL][Joint Matrix][E2E] Uncomment the Joint Matrix tests for combination 32x32x16 Nov 26, 2024

YixingZhang007 marked this pull request as draft November 26, 2024 21:58

YixingZhang007 had a problem deploying to WindowsCILock November 26, 2024 21:59 — with GitHub Actions Error

YixingZhang007 self-assigned this Nov 26, 2024

YixingZhang007 had a problem deploying to WindowsCILock November 26, 2024 22:16 — with GitHub Actions Error

YixingZhang007 temporarily deployed to WindowsCILock November 26, 2024 22:23 — with GitHub Actions Inactive

YixingZhang007 temporarily deployed to WindowsCILock November 26, 2024 22:48 — with GitHub Actions Inactive

dkhaldi approved these changes Nov 27, 2024

View reviewed changes

YixingZhang007 force-pushed the uncomment_test_32x32x16 branch from fb3a74e to 1785eb9 Compare November 29, 2024 19:02

YixingZhang007 temporarily deployed to WindowsCILock November 29, 2024 19:02 — with GitHub Actions Inactive

YixingZhang007 had a problem deploying to WindowsCILock November 29, 2024 19:15 — with GitHub Actions Error

YixingZhang007 temporarily deployed to WindowsCILock November 29, 2024 19:20 — with GitHub Actions Inactive

YixingZhang007 temporarily deployed to WindowsCILock November 29, 2024 19:33 — with GitHub Actions Inactive

YixingZhang007 temporarily deployed to WindowsCILock November 30, 2024 00:00 — with GitHub Actions Inactive

YixingZhang007 had a problem deploying to WindowsCILock November 30, 2024 00:14 — with GitHub Actions Error

YixingZhang007 temporarily deployed to WindowsCILock November 30, 2024 00:15 — with GitHub Actions Inactive

YixingZhang007 had a problem deploying to WindowsCILock November 30, 2024 00:29 — with GitHub Actions Error

YixingZhang007 had a problem deploying to WindowsCILock November 30, 2024 00:39 — with GitHub Actions Error

YixingZhang007 temporarily deployed to WindowsCILock November 30, 2024 00:52 — with GitHub Actions Inactive

YixingZhang007 had a problem deploying to WindowsCILock November 30, 2024 01:17 — with GitHub Actions Error

Uncomment the tests for Joint Matrix combination 32x32x16

bf10daf

YixingZhang007 force-pushed the uncomment_test_32x32x16 branch from 3eee097 to bf10daf Compare November 30, 2024 01:29

YixingZhang007 temporarily deployed to WindowsCILock November 30, 2024 01:30 — with GitHub Actions Inactive

YixingZhang007 temporarily deployed to WindowsCILock November 30, 2024 01:56 — with GitHub Actions Inactive

YixingZhang007 marked this pull request as ready for review November 30, 2024 02:35

YixingZhang007 requested a review from a team as a code owner November 30, 2024 02:35

YixingZhang007 requested a review from maarquitos14 November 30, 2024 02:35

maarquitos14 requested changes Dec 2, 2024

View reviewed changes

dkhaldi reviewed Dec 2, 2024

View reviewed changes

YixingZhang007 added 2 commits December 2, 2024 08:14

resolve comments

7561b2f

code clean up

f764365

YixingZhang007 had a problem deploying to WindowsCILock December 2, 2024 16:17 — with GitHub Actions Error

maarquitos14 approved these changes Dec 2, 2024

View reviewed changes

code clean up

48957e1

YixingZhang007 temporarily deployed to WindowsCILock December 2, 2024 16:28 — with GitHub Actions Inactive

dkhaldi approved these changes Dec 2, 2024

View reviewed changes

YixingZhang007 temporarily deployed to WindowsCILock December 2, 2024 18:13 — with GitHub Actions Inactive

YuriPlyakhin approved these changes Dec 3, 2024

View reviewed changes

sarnex merged commit 4881d6d into intel:sycl Dec 3, 2024
14 checks passed


		// -ffp-model=precise is added to not depend on compiler defaults.

		// XFAIL: gpu

[SYCL][Joint Matrix][E2E] Uncomment the Joint Matrix tests for combination 32x32x16 #16191

[SYCL][Joint Matrix][E2E] Uncomment the Joint Matrix tests for combination 32x32x16 #16191

Uh oh!

Conversation

YixingZhang007 commented Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkhaldi left a comment

Choose a reason for hiding this comment

Uh oh!

YixingZhang007 commented Nov 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkhaldi Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriPlyakhin Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YixingZhang007 Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriPlyakhin Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkhaldi left a comment

Choose a reason for hiding this comment

Uh oh!

YixingZhang007 commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YixingZhang007 commented Dec 3, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

YixingZhang007 commented Nov 26, 2024 •

edited

Loading

dkhaldi Dec 2, 2024 •

edited

Loading

YuriPlyakhin Dec 2, 2024 •

edited

Loading

YixingZhang007 Dec 3, 2024 •

edited

Loading

YuriPlyakhin Dec 3, 2024 •

edited

Loading

YixingZhang007 commented Dec 3, 2024 •

edited

Loading