Skip to content

Conversation

dm-vodopyanov
Copy link
Contributor

No description provided.

@dm-vodopyanov dm-vodopyanov requested a review from a team as a code owner September 15, 2025 13:01
@dm-vodopyanov dm-vodopyanov changed the title [SYCL][Doc] Add is_gpu_integrated query to sycl_ext_intel_device_info [SYCL][Doc] Add sycl_ext_oneapi_device_is_integrated spec extension Sep 15, 2025
@aelovikov-intel
Copy link
Contributor

Copy link
Contributor

@gmlueck gmlueck left a comment

Choose a reason for hiding this comment

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

I saw that an earlier version of this PR added the query to sycl_ext_intel_device_info. Why did you change your mind and add it as a separate extension?

If we need to add this query as both an info descriptor and also an aspect (telling whether the info descriptor is supported), then it might be better to add it to sycl_ext_intel_device_info because it matches the pattern we use there.

== Overview

This extension allows host code to check whether a physical device, represented
by a SYCL device, is integrated with the host.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest a slight rewording here because the difference between "integrated" and "discrete" isn't always clear cut:

This extension allows host code to check whether a SYCL GPU device is "integrated" or "discrete". An integrated GPU device often shares memory with the host CPU, while a discrete GPU is often a separate card with its own memory. However, some GPU devices may not fit neatly into either category, so implementors may provide their own judgement when categorizing a GPU device as "integrated" vs. "discrete".

@bashbaug does this seem like a reasonable description? Any suggestions?

|Return type
|Description

|`sycl::ext::oneapi::info::device::device_is_integrated`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|`sycl::ext::oneapi::info::device::device_is_integrated`
|`sycl::ext::oneapi::info::device::is_integrated_gpu`

The word "device" seems redundant because this is a device query.


|`sycl::ext::oneapi::info::device::device_is_integrated`
|bool
|Returns `true` if device is integrated, otherwise returns `false`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|Returns `true` if device is integrated, otherwise returns `false`.
|Returns `true` if the device is an integrated GPU, otherwise returns `false`. Any device which returns `true` for this query will also have the aspect `aspect::gpu`.

|Aspect
|Description

|`ext_oneapi_device_is_integrated`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|`ext_oneapi_device_is_integrated`
|`ext_oneapi_is_integrated_gpu`

|`ext_oneapi_device_is_integrated`
|Indicates that the device supports the
`sycl::device::get_info<sycl::ext::oneapi::info::device::device_is_integrated>()`
call.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why can't we support this call for all devices? Do we need special support in the backend driver?

If we could support the query for all devices, then we would just need an aspect, rather than having both the info descriptor and an aspect (which tells whether the info descriptor is supported).

@@ -0,0 +1,121 @@
= sycl_ext_oneapi_device_is_integrated
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
= sycl_ext_oneapi_device_is_integrated
= sycl_ext_oneapi_is_integrated_gpu

May as well rename this too to match the query name.

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.

4 participants