Skip to content

[SYCL] Diagnose local accessor use in single_task or parallel_for(range) #8581

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 34 commits into from
Mar 24, 2023

Conversation

mmoadeli
Copy link
Contributor

@mmoadeli mmoadeli commented Mar 8, 2023

  • According to local accessors of the SYCL specification, a local accessor must not be used in a SYCL kernel function that is invoked via single_task or via the simple form of parallel_for that takes a range parameter.
  • Add test.

mmoadeli and others added 18 commits January 31, 2023 09:49
Update failing tests due to adding diagnostic for const qualified DataT only allowed for non readonly accessor.
- Use -Xclang -verify for testing
Having it requires handling over 20 expected-erros.
…o inheritance from base class hitting assert.
…hat is invoked via single_task or via the simple form of parallel_for that takes a range parameter.
@mmoadeli mmoadeli requested a review from a team as a code owner March 8, 2023 23:51
@mmoadeli mmoadeli requested a review from bso-intel March 8, 2023 23:51
@mmoadeli mmoadeli changed the title Diagnostic on using local accessor in single_task or parallel_for(range) [SYCL] Diagnostic on using local accessor in single_task or parallel_for(range) Mar 8, 2023
@mmoadeli mmoadeli temporarily deployed to aws March 9, 2023 00:15 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 9, 2023 09:55 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 9, 2023 11:03 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 9, 2023 11:43 — with GitHub Actions Inactive
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

LGTM!

@mmoadeli mmoadeli temporarily deployed to aws March 16, 2023 13:57 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 16, 2023 15:01 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 16, 2023 15:59 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 16, 2023 16:29 — with GitHub Actions Inactive
@mmoadeli
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1646

@mmoadeli mmoadeli temporarily deployed to aws March 16, 2023 17:27 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 16, 2023 18:49 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 20, 2023 13:41 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws March 20, 2023 23:53 — with GitHub Actions Inactive
@mmoadeli
Copy link
Contributor Author

mmoadeli commented Mar 22, 2023

@bader there are a number of places in corresponding PR at llvm-test-suite which needs to be updated. I'll have them sorted out by tomorrow. That will make both ready to merge.

@bader bader changed the title [SYCL] Diagnostic on using local accessor in single_task or parallel_for(range) [SYCL] Diagnose local accessor use in single_task or parallel_for(range) Mar 22, 2023
@bader
Copy link
Contributor

bader commented Mar 22, 2023

@bader there are a number of places in corresponding PR at llvm-test-suite. I'll have them sorted out by tomorrow. That will make both ready to merge.

👍
Let @intel/llvm-gatekeepers know when it's ready.

@bader bader temporarily deployed to aws March 22, 2023 22:51 — with GitHub Actions Inactive
@bader bader temporarily deployed to aws March 22, 2023 23:28 — with GitHub Actions Inactive
@mmoadeli
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1646

@bader bader temporarily deployed to aws March 23, 2023 02:25 — with GitHub Actions Inactive
@bader bader temporarily deployed to aws March 23, 2023 03:17 — with GitHub Actions Inactive
@mmoadeli
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1646

@bader bader temporarily deployed to aws March 23, 2023 09:26 — with GitHub Actions Inactive
@bader bader temporarily deployed to aws March 24, 2023 12:24 — with GitHub Actions Inactive
@steffenlarsen
Copy link
Contributor

Windows L0 Failed Tests (1):
SYCL :: USM/memcpy2d.cpp
Unrelated. Has been temporarily disabled.

@steffenlarsen steffenlarsen merged commit 18899cc into intel:sycl Mar 24, 2023
bader added a commit that referenced this pull request Mar 24, 2023
bader added a commit that referenced this pull request Mar 24, 2023
…_for(range)" (#8773)

Reverts #8581

This PR introduced multiple failures in post-commit and pre-commit
tests.
@bader
Copy link
Contributor

bader commented Mar 24, 2023

@mmoadeli, I had to revert this patch because it introduced a lot of failures to both pre- and post-commit validation. Please, fix all these issues before re-applying the patch again.

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