Skip to content

[SYCL][ESIMD] Add more stringent compile time checks to local_accessor version of block_load/block_store, gather/scatter API #11653

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.

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

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

fineg74
Copy link
Contributor

@fineg74 fineg74 commented Oct 25, 2023

No description provided.

@fineg74 fineg74 requested a review from a team as a code owner October 25, 2023 02:28
@fineg74 fineg74 temporarily deployed to WindowsCILock October 25, 2023 02:30 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to WindowsCILock October 25, 2023 02:49 — with GitHub Actions Inactive

// Incompatible mode (read).
SYCL_EXTERNAL void
kernel6(local_accessor<const int, 1> &buf) SYCL_ESIMD_FUNCTION {
Copy link
Contributor

Choose a reason for hiding this comment

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

'const int' means it is read only. It is surprising that SYCL implementation allowed creating it because
a) it is odd to have read-only local accessor - that memory cannot be written and thus is not usable
b) SYCL 2020 SPEC says: "This specialization of accessor is only available for access modes access_mode::read_write and access_mode::atomic.".

simd<int, 32> v(0, 1);
// CHECK: block_load_store.cpp:57{{.*}}error: no matching function
// function for call to 'block_store'
block_store<int, 32>(buf, 0, v);
Copy link
Contributor

Choose a reason for hiding this comment

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

kernel1() in this test is the only positive test for load/store for read/write accessor.
Wouldn't it be useful to add 2 more cases - 1 load from read accesor and 1 store to write accessor?

@againull againull merged commit 8d7396d into intel:sycl Oct 26, 2023
@fineg74 fineg74 deleted the accessorCheck2 branch October 26, 2023 15:53
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.

3 participants