Skip to content
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

[SYCL][Level Zero] Implement sycl_ext_intel_queue_index extension #7599

Merged
merged 31 commits into from
Dec 13, 2022

Conversation

aelovikov-intel
Copy link
Contributor

@aelovikov-intel aelovikov-intel commented Nov 30, 2022

The feature needs to pass extra data to piQueueCreate which is
impossible with the current interface. As such, and because of the
current ABI freeze, a new piQueueCreateEx interface has been added
accepting pi_queue_properties *Properties (similarly to other
interfaces allowing optional/additional data) with the plan to retire
the old one at the next ABI break window.

Extension spec: #7520

@gmlueck
Copy link
Contributor

gmlueck commented Dec 1, 2022

Will you add any tests to llvm-test-suite?

@gmlueck
Copy link
Contributor

gmlueck commented Dec 1, 2022

Will you add any tests to llvm-test-suite?

Sorry, I just saw that you added tests in intel/llvm-test-suite#1425

@aelovikov-intel
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1425

@kbenzie
Copy link
Contributor

kbenzie commented Dec 12, 2022

@smaslov-intel I've added oneapi-src/unified-runtime#120 to track this.

@aelovikov-intel
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1425

@aelovikov-intel
Copy link
Contributor Author

The testing in "verify with" is clean.

@aelovikov-intel
Copy link
Contributor Author

@intel/dpcpp-esimd-reviewers can you please review this today?

Copy link
Contributor

@v-klochkov v-klochkov left a comment

Choose a reason for hiding this comment

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

Ok for ESIMD.
for @intel/llvm-reviewers-runtime changes please get one more review.

@v-klochkov v-klochkov requested a review from a team December 12, 2022 22:50
assert(Properties[0] == PI_QUEUE_FLAGS);
pi_queue_properties Flags = Properties[1];
// Extra data isn't supported yet.
assert(Properties[2] == 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but this assert can be triggered by the user. Would it be better to return PI_ERROR_INVALID_OPERATION instead to make it a recoverable error rather than a failed assert? Note that the call should be changed to call rather than call_nocheck.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline. This will be done in a follow-up patch.

@steffenlarsen
Copy link
Contributor

Failures in verifications are unrelated and have been reported separately. Merging this.

@steffenlarsen steffenlarsen merged commit d2ec964 into intel:sycl Dec 13, 2022
steffenlarsen pushed a commit to intel/llvm-test-suite that referenced this pull request Dec 13, 2022
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Dec 13, 2022
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
@aelovikov-intel aelovikov-intel deleted the queue-index branch May 1, 2023 16:12
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.

6 participants