Skip to content

[SYCL] Make device ids unique per backend #4247

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 64 commits into from
Sep 24, 2021

Conversation

bso-intel
Copy link
Contributor

We decided to make device id numbers unique per backend.
Also, by adding the device_type into each device prefix listing in sycl-ls,
the user can easily set SYCL_DEVICE_FILTER correctly.
Future work: refactor devices and plafroms cache to optimize the device retrieval.

Signed-off-by: Byoungro So byoungro.so@intel.com

We decided to make device id numbers unique per backend.
Also, by adding the device_type into each device prefix listing in sycl-ls,
the user can easily set SYCL_DEVICE_FILTER correctly.
Future work: refactor devices and plafroms cache to optimize the device retrieval.

Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
@bso-intel bso-intel marked this pull request as ready for review August 17, 2021 23:43
@bso-intel bso-intel requested review from bader, pvchupin, smaslov-intel and a team as code owners August 17, 2021 23:43
bso-intel and others added 6 commits August 18, 2021 18:01
Co-authored-by: Alexey Bader <alexey.bader@intel.com>
Co-authored-by: Alexey Bader <alexey.bader@intel.com>
Co-authored-by: Alexey Bader <alexey.bader@intel.com>
Co-authored-by: Alexey Bader <alexey.bader@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
@bso-intel
Copy link
Contributor Author

The failures in Jenkins/Precommit are addressed in intel/llvm-test-suite#256

bader
bader previously approved these changes Aug 20, 2021
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

Documentation updates look good to me.
One minor performance related suggestion, but I'll runtime team to decide if it's worth applying.

@bso-intel
Copy link
Contributor Author

@romanovvlad
Please review.

Co-authored-by: Romanov Vlad <vlad.romanov@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
@bso-intel
Copy link
Contributor Author

bso-intel commented Sep 20, 2021

@romanovvlad ,
I believe I addressed all your feedback.
I decided to remove "resetDeviceIds()" because it is not necessary.
Therefore, I did not need recursive mutex.
Other than that, I changed the code as we discussed in the meeting.
Please review.
Thanks.

bso-intel and others added 3 commits September 21, 2021 09:25
Co-authored-by: Romanov Vlad <vlad.romanov@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
bso-intel and others added 3 commits September 22, 2021 10:01
Co-authored-by: Romanov Vlad <vlad.romanov@intel.com>
Co-authored-by: Romanov Vlad <vlad.romanov@intel.com>
Signed-off-by: Byoungro So <byoungro.so@intel.com>
@bso-intel
Copy link
Contributor Author

Here is the non-verbose sycl-ls output:

bso@scsel-cfl-10:/iusers/bso/sycl/llvm-priv2$ sycl-ls
[opencl:acc:0] Intel(R) FPGA Emulation Platform for OpenCL(TM), Intel(R) FPGA Emulation Device 1.2 [2021.12.6.0.19_160000]
[opencl:gpu:1] Intel(R) OpenCL HD Graphics, Intel(R) UHD Graphics 630 [0x3e92] 3.0 [21.34.20767]
[opencl:cpu:2] Intel(R) OpenCL, Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz 2.1 [2021.12.6.0.19_160000]
[level_zero:gpu:0] Intel(R) Level-Zero, Intel(R) UHD Graphics 630 [0x3e92] 1.1 [1.1.20767]
[host:host:0] SYCL host platform, SYCL host device 1.2 [1.2]

Here is the verbose sycl-ls output:

bso@scsel-cfl-10:/iusers/bso/sycl/llvm-priv2$ sycl-ls --verbose
[opencl:acc:0] Intel(R) FPGA Emulation Platform for OpenCL(TM), Intel(R) FPGA Emulation Device 1.2 [2021.12.6.0.19_160000]
[opencl:gpu:1] Intel(R) OpenCL HD Graphics, Intel(R) UHD Graphics 630 [0x3e92] 3.0 [21.34.20767]
[opencl:cpu:2] Intel(R) OpenCL, Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz 2.1 [2021.12.6.0.19_160000]
[level_zero:gpu:0] Intel(R) Level-Zero, Intel(R) UHD Graphics 630 [0x3e92] 1.1 [1.1.20767]
[host:host:0] SYCL host platform, SYCL host device 1.2 [1.2]

Platforms: 5
Platform [#1]:
    Version  : OpenCL 1.2 Intel(R) FPGA SDK for OpenCL(TM), Version 20.3
    Name     : Intel(R) FPGA Emulation Platform for OpenCL(TM)
    Vendor   : Intel(R) Corporation
    Devices  : 1
        Device [#3]:
        Type       : acc
        Version    : 1.2
        Name       : Intel(R) FPGA Emulation Device
        Vendor     : Intel(R) Corporation
        Driver     : 2021.12.6.0.19_160000
Platform [#2]:
    Version  : OpenCL 3.0
    Name     : Intel(R) OpenCL HD Graphics
    Vendor   : Intel(R) Corporation
    Devices  : 1
        Device [#4]:
        Type       : gpu
        Version    : 3.0
        Name       : Intel(R) UHD Graphics 630 [0x3e92]
        Vendor     : Intel(R) Corporation
        Driver     : 21.34.20767
Platform [#3]:
    Version  : OpenCL 2.1 LINUX
    Name     : Intel(R) OpenCL
    Vendor   : Intel(R) Corporation
    Devices  : 1
        Device [#5]:
        Type       : cpu
        Version    : 2.1
        Name       : Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz
        Vendor     : Intel(R) Corporation
        Driver     : 2021.12.6.0.19_160000
Platform [#4]:
    Version  : 1.1
    Name     : Intel(R) Level-Zero
    Vendor   : Intel(R) Corporation
    Devices  : 1
        Device [#1]:
        Type       : gpu
        Version    : 1.1
        Name       : Intel(R) UHD Graphics 630 [0x3e92]
        Vendor     : Intel(R) Corporation
        Driver     : 1.1.20767
Platform [#5]:
    Version  : 1.2
    Name     : SYCL host platform
    Vendor   :
    Devices  : 1
        Device [#1]:
        Type       : host
        Version    : 1.2
        Name       : SYCL host device
        Vendor     :
        Driver     : 1.2
default_selector()      : gpu, Intel(R) Level-Zero, Intel(R) UHD Graphics 630 [0x3e92] 1.1 [1.1.20767]
host_selector()         : host, SYCL host platform, SYCL host device 1.2 [1.2]
accelerator_selector()  : acc, Intel(R) FPGA Emulation Platform for OpenCL(TM), Intel(R) FPGA Emulation Device 1.2 [2021.12.6.0.19_160000]
cpu_selector()          : cpu, Intel(R) OpenCL, Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz 2.1 [2021.12.6.0.19_160000]
gpu_selector()          : gpu, Intel(R) Level-Zero, Intel(R) UHD Graphics 630 [0x3e92] 1.1 [1.1.20767]
custom_selector(gpu)    : gpu, Intel(R) Level-Zero, Intel(R) UHD Graphics 630 [0x3e92] 1.1 [1.1.20767]
custom_selector(cpu)    : cpu, Intel(R) OpenCL, Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz 2.1 [2021.12.6.0.19_160000]
custom_selector(acc)    : acc, Intel(R) FPGA Emulation Platform for OpenCL(TM), Intel(R) FPGA Emulation Device 1.2 [2021.12.6.0.19_160000]

It is hard to determine the device number for each selector's chosen device.
I hope that the user can find the [SYCL_DEVICE_FILTER] information at the beginning of the output.

@bso-intel
Copy link
Contributor Author

bso-intel commented Sep 23, 2021

Thanks, @romanovvlad .
I don't know who is going to merge this PR.
IMPORTANT!!!
Please merge this PR and intel/llvm-test-suite#256 together.
The Jenkins/Precommit result below shows the result that used PR-256.

Copy link
Contributor

@pvchupin pvchupin left a comment

Choose a reason for hiding this comment

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

doc ok

@bso-intel
Copy link
Contributor Author

@smaslov-intel,
It says I still need your approval.
Thanks.

@bso-intel
Copy link
Contributor Author

@vladimirlaz

As we discussed before, this PR should be merged together with intel/llvm-test-suite#256 at the same time.
If either PR is merged alone, it will cause Precommit tests on llvm-test-suite will fail.
If you are not the right person, please recommend anyone I should contact.
Thanks.

@pvchupin
Copy link
Contributor

I see testing clean for this change even though intel/llvm-test-suite#256 is not merged.
So it looks like this specific change can be merged safely first and then intel/llvm-test-suite#256 can follow later?

@bso-intel
Copy link
Contributor Author

@pvchupin
No, please do not merge this PR alone.
I changed the Precommit testing manually against PR-256 to show the clean result when both PR are combined together.
If this PR is merged alone, it will cause many tests in Precommit to fail.

@vladimirlaz
Copy link
Contributor

@bso-intel I will merge both PRs later today to minimize the time when expected regressions will be seen.

@vladimirlaz vladimirlaz merged commit 7aa5be0 into intel:sycl Sep 24, 2021
@bso-intel bso-intel deleted the unique-device-id-per-backend branch September 28, 2021 21:33
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