[SYCL] Add spec for sycl_ext_intel_cslice #7513
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a proposed extension specification that allows partitioning a device by "cslice" (aka CCS-es).
It turns out that ATS-M does not support this type of partitioning, so reword the overview to note that PVC is the only supported device.
jandres742
reviewed
Nov 23, 2022
bashbaug
reviewed
Nov 23, 2022
Co-authored-by: Ben Ashbaugh <[email protected]>
aelovikov-intel
added a commit
to aelovikov-intel/llvm
that referenced
this pull request
Dec 2, 2022
With this change, on PVC sub-sub-devices now require info::partition_property::ext_intel_partition_by_cslice instead of info::partition_property::partition_by_affinity_domain that wasn't quite accurately describing the actual scheme. On other devices, CSlice-based partitioning is now disabled because that's not how the actual H/W works. If precise manual access to individual CCS is required than sycl_ext_intel_queue_index extension should be used instead. Extension specification is being added in intel#7513.
romanovvlad
reviewed
Dec 4, 2022
| // Available only when | ||
| // Prop == info::partition_property::ext_intel_partition_by_cslice | ||
| template <info::partition_property Prop> | ||
| std::vector<device> create_sub_devices() const; |
There was a problem hiding this comment.
Shouldn't it be ext_intel_create_sub_devices since this is an extension?
There was a problem hiding this comment.
I think it's OK to use create_sub_devices since the template parameter already has "ext_intel". Code calling this function will clearly be to an extension because it will look like:
d.create_sub_devices<info::partition_property::ext_intel_partition_by_cslice>();
There was a problem hiding this comment.
@romanovvlad: Does my answer address your concern?
There was a problem hiding this comment.
I'm satisfied with Greg's answer.
|
Ping @intel/dpcpp-specification-reviewers, can someone other than me review this? |
jbrodman
reviewed
Dec 8, 2022
| `info::partition_property::ext_intel_partition_by_cslice`. | ||
|
|
||
| The only Intel GPU device that currently supports this type of partitioning is | ||
| PVC, and this support is only available when the device driver is configured in |
There was a problem hiding this comment.
Replace PVC with "the Data Center GPU Max series, formerly known as Ponte Vecchio".
jbrodman
approved these changes
Dec 8, 2022
bashbaug
reviewed
Dec 8, 2022
Co-authored-by: Ben Ashbaugh <[email protected]>
bashbaug
approved these changes
Dec 9, 2022
steffenlarsen
pushed a commit
that referenced
this pull request
Dec 12, 2022
With this change, on PVC sub-sub-devices now require `info::partition_property::ext_intel_partition_by_cslice` instead of `info::partition_property::partition_by_affinity_domain` that wasn't quite accurately describing the actual scheme. The old behavior could be temporarily restored via `SYCL_PI_LEVEL_ZERO_EXPOSE_CSLICE_IN_AFFINITY_PARTITIONING` environment variable but it is immediately deprecated, and customers are encouraged to switch to the new partitioning scheme as soon as possible. However, even in this scenario, `sub_sub_device.get_info<info::device::partition_type_property>()` would return `info::partition_property::ext_intel_partition_by_cslice`. That is due to the fact that the whole device hierarchy is pre-populated in the plugin, and we don't know in advance what partitioning would be used in `get_sub_devices` call from SYCL RT. On other devices, CSlice-based partitioning is now disabled because that's not how the actual H/W works. If precise manual access to individual CCS is required than `sycl_ext_intel_queue_index` extension should be used instead. Extension specification is being added in #7513.
againull
pushed a commit
that referenced
this pull request
Feb 7, 2023
Recently, an extension was added here: #7513 This extension allows partitioning a device by "cslice" (aka CCS-es). With this change, on PVC sub-sub-devices now require info::partition_property::ext_intel_partition_by_cslice instead of info::partition_property::partition_by_affinity_domain that wasn't quite accurately describing the actual scheme. Level Zero backend support was added here: #7626 In this change, we are adding support in OpenCL plugin. This change makes use of the cl_intel_command_queue_families recently added. Signed-off-by: Arvind Sudarsanam <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a proposed extension specification that allows partitioning a device by "cslice" (aka CCS-es).