Skip to content

[SYCL] Implement SYCL 2020 spec functionality: no propagation from function to the caller #4084

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 17 commits into from
Jul 16, 2021

Conversation

smanna12
Copy link
Contributor

@smanna12 smanna12 commented Jul 9, 2021

In SYCL 1.2.1 spec, the attributes get propagated from device functions to a kernel.
The SYCL 2020 requirement mandating the avoidance of the propagation of all kernel attributes to the caller when used on a function.

Attributes that should not be propagated from device functions to a kernel to match with new SYCL 2020 spec.
1. scheduler_target_fmax_mhz
2. kernel_args_restrict
3. no_global_work_offset
4. max-work-group-size
5. max-global-work-dim
6. num-simd-work-items
7. reqd-sub-group-size
8. reqd-work-group-size
9. named_sub_group_size
10. sycl_explicit_simd

This patch
i. keeps the SYCL 1.2.1 spec functionality and propagates the attributes with the older SYCL mode(-sycl-std=2017)
iii. adds or updates tests to validate the propagating behavior with SYCL 1.2.1 and SYCL 2020 specs.

Signed-off-by: Soumi Manna [email protected]

@smanna12 smanna12 changed the title [SYCL] Implement SYCL 2020 spec functionality: no propagation from fu… [SYCL] Implement SYCL 2020 spec functionality: no propagation from function to the caller Jul 9, 2021
smanna12 added 2 commits July 9, 2021 06:15
Signed-off-by: Soumi Manna <[email protected]>
Signed-off-by: Soumi Manna <[email protected]>
Signed-off-by: Soumi Manna <[email protected]>
@smanna12
Copy link
Contributor Author

smanna12 commented Jul 12, 2021

Runtime test failures for Basic/parallel_for_range.cpp has been tracked here: intel/llvm-test-suite#353
TableGen support (will be added in a separate patch) has been tracked here: #4094

Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

I'm a little confused. What is the difference between this PR and #3836?

@smanna12
Copy link
Contributor Author

smanna12 commented Jul 12, 2021

I'm a little confused. What is the difference between this PR and #3836?

No changes in doc. This PR is different with #3836 with couple of things:

  1. All Sema/Codegen tests are updated separately in SYCL 2017 and SYCL 2020 modes so that changes can actually be tested by calling FileCheck in the RUN lines.
  2. This PR only copies the attributes below in SYCL 2017 (always propagate to match with SYCL 1.2.1 spec functionality) and SYCL 2020 (propagate when DirectlyCalled is called to match with SYCL 2020 spec functionality).
                 IntelReqdSubGroupSizeAttr, IntelNamedSubGroupSizeAttr,
                 ReqdWorkGroupSizeAttr, SYCLIntelKernelArgsRestrictAttr,
                 SYCLIntelNumSimdWorkItemsAttr,
                 SYCLIntelSchedulerTargetFmaxMhzAttr,
                 SYCLIntelMaxWorkGroupSizeAttr, SYCLIntelMaxGlobalWorkDimAttr,
                 SYCLIntelNoGlobalWorkOffsetAttr, SYCLSimdAttr

Rest of the attributes (no change for them to avoid duplicating them) applied to kernel if DirectlyCalled is TRUE irrespective of SYCL version.

I had rebase problems with my branch with #3836 , so created new one.

@smanna12
Copy link
Contributor Author

Pre-commit test failures for Basic/parallel_for_range.cpp have been fixed now: intel/llvm-test-suite#353.

smanna12 added 2 commits July 13, 2021 13:54
Signed-off-by: Soumi Manna <[email protected]>
AaronBallman
AaronBallman previously approved these changes Jul 14, 2021
Copy link
Contributor

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

@smanna12
Copy link
Contributor Author

smanna12 commented Jul 14, 2021

Thanks @AaronBallman for the reviews.

Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

I haven't fully looked through the tests yet, but I had a couple of initial comments

Signed-off-by: Soumi Manna <[email protected]>
smanna12 added 2 commits July 14, 2021 18:47
Signed-off-by: Soumi Manna <[email protected]>
smanna12 added 2 commits July 15, 2021 18:16
Signed-off-by: Soumi Manna <[email protected]>
Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@smanna12
Copy link
Contributor Author

smanna12 commented Jul 16, 2021

Thanks for the reviews everyone.

@AaronBallman AaronBallman merged commit 2667e3e into intel:sycl Jul 16, 2021
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