Skip to content

[SYCL] Updated Level-Zero backend spec according to SYCL 2020 standard #4560

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Sep 17, 2021
Merged

[SYCL] Updated Level-Zero backend spec according to SYCL 2020 standard #4560

merged 8 commits into from
Sep 17, 2021

Conversation

smaslov-intel
Copy link
Contributor

@smaslov-intel smaslov-intel commented Sep 14, 2021 •
edited
Loading

After #4512 the implementation already follows the API being documented here.
Signed-off-by: Sergey V Maslov [email protected]

@smaslov-intel smaslov-intel requested a review from a team as a code owner September 14, 2021 00:50
@smaslov-intel smaslov-intel changed the title [SYCL] Updated according to SYCL 2020 standard [SYCL] Updated Level-Zero backend spec according to SYCL 2020 standard Sep 14, 2021
vladimirlaz
vladimirlaz reviewed Sep 14, 2021
View reviewed changes
Copy link
Contributor

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

We should also define macro: see intel/llvm-test-suite#461 (comment)

gmlueck
gmlueck reviewed Sep 14, 2021
View reviewed changes
Copy link
Contributor

@gmlueck gmlueck left a comment

Choose a reason for hiding this comment

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

In addition to the specific comments below, I have a general question. Are we making any guarantee about the stability of this API? For example, #4542 changes the API to make_kernel_bundle() in an incompatible way. If we want to reserve the right to make breaking changes like this, I think we need to mark this API as "experimental", and include some text saying that APIs may change in the future in ways that break existing code.

@smaslov-intel
Copy link
Contributor Author

We should also define macro: see intel/llvm-test-suite#461 (comment)

Added.

@smaslov-intel
Copy link
Contributor Author

In addition to the specific comments below, I have a general question. Are we making any guarantee about the stability of this API? For example, #4542 changes the API to make_kernel_bundle() in an incompatible way. If we want to reserve the right to make breaking changes like this, I think we need to mark this API as "experimental", and include some text saying that APIs may change in the future in ways that break existing code.

The intention is that this API is stable and final, and that's why I removed wording that it's a draft. The make_kernel_bundle is the only exception at the moment, and I plan to fix that in a follow-up PR.

vladimirlaz
vladimirlaz previously approved these changes Sep 15, 2021
View reviewed changes
@gmlueck
Copy link
Contributor

gmlueck commented Sep 15, 2021

If you want to address these in #4542 instead, though, that's fine with me.

Yes, that's what I will do.

I see that you closed #4542 and replaced it with #4576. Will you update #4576 to reflect changes to the Level Zero interop specification?

@smaslov-intel
Copy link
Contributor Author

If you want to address these in #4542 instead, though, that's fine with me.

Yes, that's what I will do.

I see that you closed #4542 and replaced it with #4576. Will you update #4576 to reflect changes to the Level Zero interop specification?

Sure, after this PR is merged, I will update "make_kernel_bundle" description in #4576

@smaslov-intel
[SYCL] Updated according to SYCL 2020 API
59d3f03 
Signed-off-by: Sergey V Maslov <[email protected]>
@smaslov-intel
address review comments
b117203 
Signed-off-by: Sergey V Maslov <[email protected]>
@smaslov-intel
address comments
9cf7e95 
Signed-off-by: Sergey V Maslov <[email protected]>
@smaslov-intel
address comments
6590123 
Signed-off-by: Sergey V Maslov <[email protected]>
@smaslov-intel
address comments
5bfc929 
Signed-off-by: Sergey V Maslov <[email protected]>
@smaslov-intel
address comments
2a66484 
Signed-off-by: Sergey V Maslov <[email protected]>
@smaslov-intel
address comments
cbc54ed 
Signed-off-by: Sergey V Maslov <[email protected]>
gmlueck
gmlueck previously approved these changes Sep 15, 2021
View reviewed changes
@vladimirlaz vladimirlaz merged commit 8bbebf5 into intel:sycl Sep 17, 2021
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Sep 18, 2021
@alexbatashev
Merge remote-tracking branch 'upstream/sycl' into xpti_tests_enable
bc2716c 
* upstream/sycl: (36 commits)
  [SYCL] Add SYCL2020 target::device enumeration value (intel#4587)
  [SYCL][Doc] Update ITT instrumentation docs (intel#4503)
  [SYCL][L0] Make all L0 events have device-visibility (intel#4534)
  [SYCL] Updated Level-Zero backend spec according to SYCL 2020 standard (intel#4560)
  [SYCL] Add error_code support for SYCL 1.2.1 exception classes (intel#4574)
  [SYCL][CI] Provide --ci-defaults option for config script (intel#4583)
  [CI] Switch GitHub Actions to Ubuntu 20.04 (intel#4582)
  [SYCL][CUDA] Fix context clearing in PiCuda tests (intel#4483)
  [SYCL] Hide SYCL service kernels (intel#4519)
  [SYCL][L0] Fix mismatched ZE call count (intel#4559)
  [SYCL] Remove function pointers extension (intel#4459)
  [GitHub Actions] Uplift clang version in post-commit validation (intel#4581)
  [SYCL] Ignore usm prefetch dummy flag (intel#4568)
  [SYCL][Group algorithms] Add group sorting algorithms implementation (intel#4439)
  [SYCL] Resolve name clash with a user defined symbol (intel#4570)
  [clang-offload-wrapper] Do not create .tgtimg section with -emit-reg-funcs=0 (intel#4577)
  [SYCL][FPGA] Remove deprecated attribute functionality (intel#4532)
  [SYCL] Remove _class aliases (intel#4465)
  [SYCL][CUDA][HIP] Report every device in its own platform (intel#4571)
  [SYCL][L0] make_device shouldn't need platform as an input (intel#4561)
  ...
@gmlueck gmlueck mentioned this pull request Sep 20, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vladimirlaz vladimirlaz vladimirlaz approved these changes

@gmlueck gmlueck gmlueck left review comments

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
@smaslov-intel @gmlueck @vladimirlaz