Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL][MATRIX] Adding test cases for testing get_coord() feature. #1676

Merged
merged 6 commits into from
Mar 23, 2023

Conversation

arnamoy10
Copy link

@arnamoy10 arnamoy10 commented Mar 22, 2023

Currently, the tests are marked for failure, they will pass once the ICG
backend support is there.

The header file support is in this PR: intel/llvm#7851

Currently, the tests are marked for failure, they will pass once the ICG
backend suppport is there.
@arnamoy10 arnamoy10 requested a review from a team as a code owner March 22, 2023 15:54
@arnamoy10 arnamoy10 requested a review from againull March 22, 2023 15:54
@arnamoy10 arnamoy10 changed the title Get coord tests [SYCL][MATRIX] Adding test cases for testing get_coord() feature. Mar 22, 2023
Copy link

@dkhaldi dkhaldi left a comment

Choose a reason for hiding this comment

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

Tests look good but the XMX8 versions are missing.

@arnamoy10
Copy link
Author

Tests look good but the XMX8 versions are missing.

Added those tests

Copy link

@dkhaldi dkhaldi left a comment

Choose a reason for hiding this comment

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

LGTM

@againull againull merged commit 48c0dbb into intel Mar 23, 2023

// Calculation of global index
int sg_idx = (int)global_idy / SG_SZ;
global_index = col + sg_idx * 4 /*VNNI_FACTOR*/ * SG_SZ;
Copy link

Choose a reason for hiding this comment

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

Should this line be global_index = col * 4 + row % 4 + sg_idx * 4 /*VNNI_FACTOR*/ * SG_SZ; instead?

[row, col] are the coordinates within the non-VNNIed subgroup matrix (32x16). I think we need to map [row, col] back to VNNIed indices when summing columns.

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

Successfully merging this pull request may close these issues.

4 participants