Skip to content

[SYCL] Implement new env var SYCL_DEVICE_FILTER #2239

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 52 commits into from
Sep 17, 2020

Conversation

bso-intel
Copy link
Contributor

@bso-intel bso-intel commented Aug 1, 2020

This PR implemented the new env var SYCL_DEVICE_FILTER to allow users to specify which specific device they want to use in all different device selectors. (not the scope of this PR, implemented in #2241)
This env var is also used to load only related plugins in SYCL RT. (This PR implemented only this)

SYCL_DEVICE_FILTER=backend:device_type:device#, where device_type={host,cpu,gpu,acc,*}, backend={opencl,level_zero,cuda,*}, device#={int}
All three fields are optional, but at least one entry should be present.

Device# field is a relative index in the devices returned from platform::get_device() call. Users can use a utility tool "sycl-ls" to find the relative index, too. Once Device# is specified, the program behavior may not be the same in different systems since the relative index varies in different system.

It can list any number of triples separated by commas(,). For example, SYCL_DEVICE_FILTER=cpu,level_zero:gpu,cuda:gpu:0

For convenience, we also support ‘*’ to indicate all possible options of the triple field.
For example, “SYCL_DEVICE_FILTER=opencl:*” indicates that all different device_types that supports opencl.
To request level_zero GPU + opencl CPU, “SYCL_DEVICE_FILTER=opencl:cpu,level_zero:gpu” can be used.

HOST device is always available by default no matter how SYCL_DEVICE_FILTER is set.

This env var will affect all known device selectors since this env var will limit loading only specified plugins into the SYCL RT. For example, SYCL_DEVICE_FILTER=level_zero and cpu_selector will throw an error because the CPU device is not supported by the level_zero backend. To use both CPU device and Level_Zero GPU, one can use SYCL_DEVICE_FILTER=cpu,level_zero.

This new env var takes a list of triples {device_type, backend, device_num}
1. This list means SYCL_RT will only use those specified devices.
2. This list also limits related plugins to be loaded by SYCL RT.
This PR only implemented new env var and selective plugin loading (intel#2)

Signed-off-by: Byoungro So <[email protected]>
Signed-off-by: Byoungro So <[email protected]>
Signed-off-by: Byoungro So <[email protected]>
@Pennycook
Copy link
Contributor

I'm not a big fan of the syntax. I think the name of the environment variable should at least be changed to SYCL_DEVICE_TRIPLES if it accepts one or more triples.

@bso-intel bso-intel changed the title [SYCL] Implement new env var SYCL_DEVICE_TRIPLE [SYCL][WIP] Implement new env var SYCL_DEVICE_TRIPLE Aug 3, 2020
@keryell
Copy link
Contributor

keryell commented Aug 4, 2020

OK, so it is completely unrelated with the obvious https://llvm.org/doxygen/classllvm_1_1Triple.html
But SYCL_DEVICE_TRIPLE is very unclear. Why TRIPLE is important here? Why not a user-friendly semantic name rather a syntactic one?
Also I would like to avoid a déjà vue évolution: the llvm::Triple evolved to be able to store... 4 (four) elements! :-( So, soon SYCL_DEVICE_TRIPLE might store 5 pieces of information... :-/

@bso-intel bso-intel changed the title [SYCL][WIP] Implement new env var SYCL_DEVICE_TRIPLE [SYCL][WIP] Implement new env var SYCL_DEVICE_TRIPLES Aug 14, 2020
@bso-intel bso-intel changed the title [SYCL][WIP] Implement new env var SYCL_DEVICE_TRIPLES [SYCL] Implement new env var SYCL_DEVICE_TRIPLES Aug 25, 2020
@bso-intel
Copy link
Contributor Author

Hi @vladimirlaz, Did I address all your feedback?

vladimirlaz
vladimirlaz previously approved these changes Sep 16, 2020
v-klochkov
v-klochkov previously approved these changes Sep 16, 2020
@bso-intel bso-intel dismissed stale reviews from v-klochkov and vladimirlaz via f3c6387 September 16, 2020 17:46
Signed-off-by: Byoungro So <[email protected]>
@v-klochkov
Copy link
Contributor

Changes look good to me.
Please take a look at the 1 failed LIT test - select_device_opencl.cpp

@bso-intel
Copy link
Contributor Author

Changes look good to me.
Please take a look at the 1 failed LIT test - select_device_opencl.cpp

I am still looking at the failure. Somehow, after I commit your suggested code change, it does not work.
There should be a bug somewhere, but I still can't find it. I will continue looking at it.

Signed-off-by: Byoungro So <[email protected]>
@bso-intel
Copy link
Contributor Author

Code owner review required
Waiting on code owner review from @kbobrovs, @pvchupin, @smaslov-intel, and/or @intel/llvm-reviewers-runtime.

Sorry, the lastest code change cancelled your approvals.
Could you approve again? Thanks.

@pvchupin pvchupin merged commit 14e227c into intel:sycl Sep 17, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Sep 17, 2020
* upstream/sycl: (405 commits)
  [SYCL] Implement new env var SYCL_DEVICE_FILTER (intel#2239)
  [Driver][SYCL] Make /MD the default for -fsycl (intel#2478)
  [SYCL]: basic support of contexts with multiple devices in Level-Zero (intel#2440)
  [SYCL] Fix LIT regression after 9dd18ca (intel#2481)
  [SYCL][L0] Kernel Destroy in piKernelRelease (intel#2475)
  [SYCL] Emit an aliased function only if it is used (intel#2430)
  [Driver][SYCL] Add defaultlib directive for sycl lib (intel#2464)
  [Driver][SYCL] Improve situations where .exe is added for AOT tools (intel#2467)
  [SYCL][L0]: Check Queue refcnt prior to using members in event wait/release (intel#2471)
  [SYCL] Unroll several loops in __init method accessor class (intel#2449)
  [SYCL][Doc] Add link to use pinned memory spec (intel#2463)
  [SYCL] Link SYCL device libraries by default. (intel#2400)
  Revert "[SYCL] XFAIL test blcoking pulldown"
  Avoid usage of deprecated "VectorType::getNumElements" (intel#737)
  Fix nullptr dereference (intel#741)
  Do not translate arbitrary precision operations without corresponding extensions (intel#714)
  Add Constrained Floating-Point Intrinsics support
  [SYCL] Take into account auxiliary cmake options for Level Zero loader
  [InstCombine] improve fold of pointer differences
  [InstCombine] add ptr difference tests; NFC
  ...
@bso-intel bso-intel deleted the sycl-device-triple branch September 22, 2020 14:52
Chenyang-L pushed a commit that referenced this pull request Feb 18, 2025
Add guards to InvalidNullPointerPlatform
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.