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

[SYCL] Add basic tests for non-uniform groups #1574

Open
wants to merge 4 commits into
base: intel
Choose a base branch
from

Conversation

Pennycook
Copy link

Tests for intel/llvm#8202.

@Pennycook Pennycook requested a review from a team as a code owner February 3, 2023 22:07
@Pennycook
Copy link
Author

@steffenlarsen - What's the new magic for running clang-format? I tried this, but it doesn't seem to be obeying the expected formatting...

git diff -U0 --no-color HEAD^ | llvm/clang/tools/clang-format/clang-format-diff.py -p1 -i

@steffenlarsen
Copy link

@steffenlarsen - What's the new magic for running clang-format? I tried this, but it doesn't seem to be obeying the expected formatting...

git diff -U0 --no-color HEAD^ | llvm/clang/tools/clang-format/clang-format-diff.py -p1 -i

That is what I normally use, but sometimes different versions of clang-format may disagree on certain scenarios. You could try building a new version of clang-format and adding it to your PATH or just apply the suggested format by hand.

@Pennycook
Copy link
Author

You could try building a new version of clang-format and adding it to your PATH or just apply the suggested format by hand.

Good idea. Building a new version of clang-format fixed it.

Copy link

@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!

@Pennycook Pennycook marked this pull request as draft February 7, 2023 23:43
@Pennycook
Copy link
Author

Converting this to draft to prevent it from being merged, same as intel/llvm#8202.

Tests the ability to create an instance of each new group type,
and the correctness of the core member functions.

Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook Pennycook changed the title [SYCL] Add basic masked_sub_group tests [SYCL] Add basic tests for non-uniform groups Feb 16, 2023
@Pennycook Pennycook marked this pull request as ready for review February 16, 2023 19:35
@Pennycook
Copy link
Author

@steffenlarsen, @JackAKirk - These should be fixed, now, and aligned with the latest code over at intel/llvm#8202.

Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook
Copy link
Author

@cperkinsintel, @steffenlarsen: I tried to make it so these won't run on CPU, but the failure logs in intel/llvm#8202 suggest it's still failing on the CPU. Have I got the UNSUPPORTED syntax wrong?

@steffenlarsen
Copy link

@cperkinsintel, @steffenlarsen: I tried to make it so these won't run on CPU, but the failure logs in intel/llvm#8202 suggest it's still failing on the CPU. Have I got the UNSUPPORTED syntax wrong?

Syntax looks right and since you're only using GPU_RUN_PLACEHOLDER it should not even try to run these on CPU. It may have just been a CI bug so I've restarted the verification on your PR.

@cperkinsintel
Copy link

These tests are for a new feature. They have successfully run on the feature PR here: intel/llvm#8202

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.

3 participants