Skip to content

[SYCL] Remove duplicate devices on submission to kernel_bundle API functions #4790

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 4 commits into from
Oct 28, 2021

Conversation

cperkinsintel
Copy link
Contributor

@cperkinsintel cperkinsintel commented Oct 20, 2021

When submitting devices to ::compile(), ::link(), ::build() and get_kernel_bundle(), section 4.11.7 of the SYCL 2020 spec states that the compilation is for each unique device, duplicate devices removed. Here we ensure that the devices sent to the backend are each unique. Also, LevelZero doesn't support compilation for more than one device but it calls die() presently. Changing that as well. Tests at intel/llvm-test-suite#527

Here the test PR is run against this PR (successfully): http://icl-jenkins.sc.intel.com:8080/job/SYCL_CI/job/intel/job/Lin/job/LLVM_Test_Suite/7703/

Signed-off-by: Chris Perkins [email protected]

…pec states that the compilation is for each unique device, duplicate devices removed. Here we ensure that the devices sent to the backend are each unique. Also, LevelZero doesn't support compilation for more than one device but it calls die() presently. Changing that as wel. Testss coming shortly.

Signed-off-by: Chris Perkins <[email protected]>
…o filter out duplicate devices

Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel cperkinsintel changed the title [SYCL] duplicate devices removed on submission to ::compile() [SYCL] duplicate devices removed on submission to kernel_bundle API functions Oct 20, 2021
@cperkinsintel cperkinsintel marked this pull request as ready for review October 21, 2021 01:22
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

Is it possible to have a unit-test for this change?

@bader bader changed the title [SYCL] duplicate devices removed on submission to kernel_bundle API functions [SYCL] Remove duplicate devices on submission to kernel_bundle API functions Oct 21, 2021
if (NumDevices != 1)
die("piProgramCreateWithBinary: level_zero supports only one device.");
if (NumDevices != 1) {
zePrint("piProgramCreateWithBinary: level_zero supports only one device.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please clarify why it is not ok to use "die" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's mostly a matter of getting the devices to behave similarly. Right now, only the OpenCL GPU device supports compilation for multiple unique devices. All the other non-supporting devices throw exceptions when asked ... except LevelZero which dies.

@cperkinsintel
Copy link
Contributor Author

@s-kanaev
I added tests to the llvm-test-suite: intel/llvm-test-suite#527
Here they are run against each other: http://icl-jenkins.sc.intel.com:8080/job/SYCL_CI/job/intel/job/Lin/job/LLVM_Test_Suite/7703/

…d of device and b) use the private function dev.getNative() to ensure that device comparisons are unique

Signed-off-by: Chris Perkins <[email protected]>
@cperkinsintel
Copy link
Contributor Author

ping to reviewers.

@bader bader requested a review from againull October 26, 2021 17:31
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

RT changes LGTM

@againull againull merged commit c222497 into intel:sycl Oct 28, 2021
@cperkinsintel cperkinsintel deleted the cperkins-filter-duplicate-devices branch October 28, 2021 17:01
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