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

[SYCL] Add tests for SYCL2020 sub_groups features #283

Merged
merged 5 commits into from
Jun 2, 2021
Merged

[SYCL] Add tests for SYCL2020 sub_groups features #283

merged 5 commits into from
Jun 2, 2021

Conversation

alexbatashev
Copy link

@alexbatashev alexbatashev commented May 19, 2021 •
edited
Loading

Link to feature: intel/llvm#3765

@alexbatashev alexbatashev mentioned this pull request May 19, 2021
Merged
Pennycook
Pennycook previously approved these changes May 19, 2021
View reviewed changes
SYCL/SubGroup/sub_groups_sycl2020.cpp Outdated
Comment on lines 37 to 40
const size_t MaxNumSubgroups =
Kernel.get_info<sycl::info::kernel_device_specific::max_num_sub_groups>(
Q.get_device());
const size_t SubgroupSize = 32 / MaxNumSubgroups;
Copy link

Choose a reason for hiding this comment

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

Any reason not to use info::kernel_device_specific::max_sub_group_size instead?

Copy link
Author

Choose a reason for hiding this comment

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

That's actually a nice piece of advice. kernel_device_specific::max_num_sub_groups returns maximum number of subroups, not the one that was actually run. I updated implementation to use the correct values.

@cperkinsintel cperkinsintel mentioned this pull request May 20, 2021
Merged
@alexbatashev alexbatashev requested a review from Pennycook May 20, 2021 20:09
Pennycook
Pennycook previously approved these changes May 20, 2021
View reviewed changes
vladimirlaz
vladimirlaz suggested changes May 24, 2021
View reviewed changes
Copy link

@vladimirlaz vladimirlaz left a comment

Choose a reason for hiding this comment

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

@alexbatashev, the newly added test is failing on CUDA. Could you please fix it?

@alexbatashev
Copy link
Author

@alexbatashev, the newly added test is failing on CUDA. Could you please fix it?

I disabled test on CUDA, because sycl tools seemingly do not generate enough information for the runtime to function correctly. I'll create a tracker for that.

As for windows regression, it does not seem to be related to my patch.

@vladimirlaz
Copy link

@alexbatashev, the newly added test is failing on CUDA. Could you please fix it?

I disabled test on CUDA, because sycl tools seemingly do not generate enough information for the runtime to function correctly. I'll create a tracker for that.

As for windows regression, it does not seem to be related to my patch.

Could you please create tickets for exposed issues?

@alexbatashev
Copy link
Author

@alexbatashev, the newly added test is failing on CUDA. Could you please fix it?

I disabled test on CUDA, because sycl tools seemingly do not generate enough information for the runtime to function correctly. I'll create a tracker for that.
As for windows regression, it does not seem to be related to my patch.

Could you please create tickets for exposed issues?

I added you to watchers for the created issues.

vladimirlaz
vladimirlaz suggested changes May 27, 2021
View reviewed changes
Copy link

@vladimirlaz vladimirlaz left a comment

Choose a reason for hiding this comment

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

Could you split the test (move common part to the header)? You have just doubled test execution time which make cause timeouts on some configurations

@alexbatashev
Copy link
Author

Could you split the test (move common part to the header)? You have just doubled test execution time which make cause timeouts on some configurations

I switched to use of if constexpr which should save greatly on build times.

@vladimirlaz vladimirlaz requested a review from Pennycook June 2, 2021 04:59
@vladimirlaz vladimirlaz merged commit d07c07c into intel:intel Jun 2, 2021
@alexbatashev alexbatashev deleted the sub_group2020 branch June 2, 2021 17:34
smaslov-intel pushed a commit to smaslov-intel/llvm-test-suite that referenced this pull request Aug 12, 2021
@alexbatashev @smaslov-intel
[SYCL] Add tests for SYCL2020 sub_groups features (intel#283)
c403e31
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
@alexbatashev
[SYCL] Add tests for SYCL2020 sub_groups features (intel/llvm-test-su…
251f915 
…ite#283)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Pennycook Pennycook Pennycook approved these changes

@vladimirlaz vladimirlaz vladimirlaz approved these changes

@cperkinsintel cperkinsintel Awaiting requested review from cperkinsintel

@AlexeySachkov AlexeySachkov Awaiting requested review from AlexeySachkov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexbatashev @vladimirlaz @Pennycook