Skip to content
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

[SYCL][Doc] Add new device descriptors to sycl_ext_intel_device_info extension #17386

Open
wants to merge 2 commits into
base: sycl
Choose a base branch
from

Conversation

againull
Copy link
Contributor

No description provided.

@againull againull requested a review from a team as a code owner March 11, 2025 07:23

| Device Descriptors | Return Type | Description |
| ------------------ | ----------- | ----------- |
| `ext::intel::info::device::current_clock_throttle_reasons` | `std::vector<ext::intel::throttle_reason>` | Returns the set of throttle reasons describing why the frequency is being limited by the hardware. Returns empty set if frequency is not throttled. |
Copy link
Contributor

Choose a reason for hiding this comment

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

As written, this descriptor is GPU-specific: the introduction talks about the "GPU clock" and the descriptions of the throttle reasons say "GPU". The other queries that are GPU-specific (e.g. number of EUs) are all prefixed with gpu_.

I think we should either:

  1. Rename this to gpu_current_throttle_reasons; or
  2. Remove "GPU" from the introduction and description.

I have a slight preference for 2, because the throttle reasons are already guarded by an aspect. If we never implement this extension on another device (e.g., CPU), we just won't enable the aspect anywhere except GPUs. But removing "GPU" from the description gives us the option to try and implement it on other devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, sounds reasonable, I have applied option 2.

@againull againull requested a review from Pennycook March 11, 2025 15:24
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.

2 participants