Skip to content

[SYCL][E2E] Limit work group size in WorkGroupScratchMemory tests #17046

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
Feb 19, 2025

Conversation

RossBrunton
Copy link
Contributor

@RossBrunton RossBrunton commented Feb 18, 2025

Some devices don't support work group sizes of 1024, so use the maximum
size if it is smaller in the copy_dynamic_size.cpp and dynamic_unused.cpp tests.

@againull
Copy link
Contributor

Arc failure is known #16877:


Timed Out Tests (1):
SYCL :: Regression/multithread_write_accessor.cpp

@RossBrunton
Copy link
Contributor Author

@intel/llvm-gatekeepers This should be ready for merging now.

@dm-vodopyanov
Copy link
Contributor

dm-vodopyanov commented Feb 19, 2025

@intel/llvm-gatekeepers This should be ready for merging now.

[sycl-e2e] In tests, limit work group size

Please specify in PR caption which tests/test suite, and tags should be [SYCL][E2E]

Some devices don't support work group sizes of 1024, so use the maximum
size if it is smaller in the copy_dynamic_size.cpp and
dynamic_unused.cpp tests.
@RossBrunton RossBrunton changed the title [sycl-e2e] In tests, limit work group size [SYCL][E2E] In tests, limit work group size Feb 19, 2025
@dm-vodopyanov
Copy link
Contributor

dm-vodopyanov commented Feb 19, 2025

Just in case, PR caption still should be updated with tests or test suite names. This is to make it not so general.
Also, no need to update commit name, because updating PR caption is enough - we do squash and merge: a new commit will be created with name == PR caption name.

@ldrumm
Copy link
Contributor

ldrumm commented Feb 19, 2025 via email

@dm-vodopyanov
Copy link
Contributor

dm-vodopyanov commented Feb 19, 2025

Is there reasoning for this policy somewhere? It strikes me as redundant information more reliably available in the diffstat - and wastes those precious few 50 characters of available title. In UR we opted to use autotagging based on the diffstat path which worked really well. I think we should revise this policy in dpc++.

https://github.com/intel/llvm/blob/sycl/sycl/doc/developer/ContributeToDPCPP.md#commit-message

+ we use them when we write release notes - to distinguish what we include and what not faster and easier.

P.S. if I'm not mistaken, for upstreaming to llorg too.

But overall, I like the idea of auto-tagging👍

@RossBrunton RossBrunton changed the title [SYCL][E2E] In tests, limit work group size [SYCL][E2E] In WorkGroupScratchMemory tests, limit work group size Feb 19, 2025
@kbenzie
Copy link
Contributor

kbenzie commented Feb 19, 2025

But overall, I like the idea of auto-tagging👍

We did this with the labeler.yml workflow and the labeler.yml config file.

@dm-vodopyanov
Copy link
Contributor

#17075

Failed Tests (1):
SYCL :: ESIMD/Prefix_Local_sum2.cpp

@dm-vodopyanov dm-vodopyanov changed the title [SYCL][E2E] In WorkGroupScratchMemory tests, limit work group size [SYCL][E2E] Limit work group size in WorkGroupScratchMemory tests Feb 19, 2025
@dm-vodopyanov dm-vodopyanov merged commit 5edc8fe into intel:sycl Feb 19, 2025
28 of 29 checks passed
AlexeySachkov pushed a commit to AlexeySachkov/llvm that referenced this pull request Mar 31, 2025
…tel#17046)

Some devices don't support work group sizes of 1024, so use the maximum
size if it is smaller in the copy_dynamic_size.cpp and
dynamic_unused.cpp tests.
AlexeySachkov added a commit that referenced this pull request Apr 2, 2025
This is a joined cherry-pick of #16286, #17046 and
#17100 made to align tests between `sycl` and `sycl-rel-6_1_0`
branches to make the pre-commit more stable on the latter.

Descriptions of cherry-picked commits:

---

Fix postcommit failure and move tests in right folder (#16286)

opencl cpu fails without a stack trace

Patch-By: Victor Lomuller <[email protected]>

---

[SYCL][E2E] Limit work group size in WorkGroupScratchMemory tests
(#17046)

Some devices don't support work group sizes of 1024, so use the maximum
size if it is smaller in the copy_dynamic_size.cpp and
dynamic_unused.cpp tests.

Patch-By: Ross Brunton <[email protected]>

---

[SYCL][E2E] Use size_t rather than unsigned long in
WorkGroupScratchMemory tests (#17100)

The max work group size is a `size_t`, rather than an `unsigned long`.
This trips up older versions of MSVC where `size_t` happens to be
`unsigned long long`.

Patch-By: Ross Brunton <[email protected]>

---
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.

5 participants