Skip to content

[SYCL][CUDA][HIP] Add missing PI_DEVICE_INFO_IMAGE_SRGB case #8779

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 2 commits into from
Apr 19, 2023

Conversation

MartinWehking
Copy link
Contributor

Adds PI_DEVICE_INFO_IMAGE_SRGB to the switch case in piDeviceGetInfo.
It's a required fix for #8433 .
Before, sycl-ls --verbose in #8433 terminated with an error since the default case was invoked (pi::die)

@MartinWehking MartinWehking requested a review from a team as a code owner March 27, 2023 10:00
@MartinWehking MartinWehking requested a review from jchlanda March 27, 2023 10:00
@GeorgeWeb
Copy link
Contributor

GeorgeWeb commented Mar 27, 2023

We should add this case to hip_piDeviceGetInfo in pi_hip.cpp as well, because it is also missing from there, and the Hip plugin's function is set up to assert in the same way as in the Cuda plugin.

That should then be enough to unblock the device aspect printing PR for sycl-ls mentioned in the description.

@MartinWehking MartinWehking requested a review from a team as a code owner March 27, 2023 10:38
@MartinWehking MartinWehking requested a review from againull March 27, 2023 10:38
@MartinWehking MartinWehking changed the title [SYCL][CUDA] Add missing PI_DEVICE_INFO_IMAGE_SRGB case [SYCL][CUDA][HIP] Add missing PI_DEVICE_INFO_IMAGE_SRGB case Mar 27, 2023
@MartinWehking MartinWehking temporarily deployed to aws March 27, 2023 11:20 — with GitHub Actions Inactive
@GeorgeWeb
Copy link
Contributor

LGTM.

@MartinWehking MartinWehking temporarily deployed to aws March 27, 2023 13:21 — with GitHub Actions Inactive
@jchlanda
Copy link
Contributor

A quick grep through CUDA's plugin shows the following enums missing:

PI_EXT_CODEPLAY_DEVICE_INFO_SUPPORTS_FUSION
PI_EXT_ONEAPI_DEVICE_INFO_MAX_GLOBAL_WORK_GROUPS
PI_EXT_ONEAPI_DEVICE_INFO_MAX_WORK_GROUPS_1D
PI_EXT_ONEAPI_DEVICE_INFO_MAX_WORK_GROUPS_2D
PI_DEVICE_INFO_IL_VERSION
PI_DEVICE_INFO_QUEUE_PROPERTIES

I guess it's OK to call die on the extensions, but maybe we should return PI_ERROR_INVALID_VALUE for the remaining two?

@MartinWehking
Copy link
Contributor Author

MartinWehking commented Apr 4, 2023

@jchlanda
That's a good idea.
However, I think it's better to leave that for a future PR since this one is just for unblocking #8433

@MartinWehking MartinWehking temporarily deployed to aws April 17, 2023 20:46 — with GitHub Actions Inactive
@againull againull merged commit 9d734ae into intel:sycl Apr 19, 2023
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