Skip to content

[SYCL] Treat profiling as not supported if submit timestamps are not #8279

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 6 commits into from
Feb 22, 2023

Conversation

sergey-semenov
Copy link
Contributor

With the recent fix to command_submit profiling information query it now requires that the plugin supports piGetDeviceAndHostTimer API. Because of that, we should not report queue profiling as a supported aspect unless both the underlying pi_queue supports profiling and piGetDeviceAndHostTimer is available.

In addition, add a feature_not_supported exception when a queue is constructed with enable_profiling property and a device that does not support profiling.

With the recent fix to command_submit profiling information query it now
requires that the plugin supports piGetDeviceAndHostTimer API. Because
of that, we should not report queue profiling as a supported aspect
unless both the underlying pi_queue supports profiling and
piGetDeviceAndHostTimer is available.

In addition, add a feature_not_supported exception when a queue is
constructed with enable_profiling property and a device that does not
support profiling.
@sergey-semenov sergey-semenov requested a review from a team as a code owner February 9, 2023 18:49
@sergey-semenov
Copy link
Contributor Author

Note that the spec does not currently state that the exception must be thrown in the case described in the commit message, but I don't expect it to be a controversial change (KhronosGroup/SYCL-Docs#359)

@sergey-semenov sergey-semenov temporarily deployed to aws February 9, 2023 19:34 — with GitHub Actions Inactive
@sergey-semenov sergey-semenov temporarily deployed to aws February 9, 2023 22:49 — with GitHub Actions Inactive
@sergey-semenov
Copy link
Contributor Author

sergey-semenov commented Feb 10, 2023

Looks like ESIMD tests assume that queue profiling is supported despite that not being the case with ESIMD emulator (it's just actually checked with this patch). I think the tests should be modified so that profiling is optional there.

I noticed, however, that several profiling related functions from the ESIMD emulator backend (piGetDeviceAndHostTime, piEventGetProfilingInfo) return PI_SUCCESS instead of an error despite not being able to do anything meaningful. @dongkyunahn-intel Is that intentional?

@sergey-semenov sergey-semenov changed the title [SYCL] Report profiling as not supported if submit timestamps are not [SYCL] Treat profiling as not supported if submit timestamps are not Feb 10, 2023
@dongkyunahn-intel
Copy link
Contributor

@sergey-semenov , as far as I remember, when I brought up ESIMD_EMULATOR backend, piEventGetProfilingInfo had to return PI_SUCCESS for the backend to pass some tests even if the implementation does nothing meaningful while profiling under emulation is not critical. That was the reason why warning message was added in the function. I was not aware that piGetDeviceAndHostTime was added.

@sergey-semenov
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1604

@sergey-semenov sergey-semenov temporarily deployed to aws February 16, 2023 16:13 — with GitHub Actions Inactive
@sergey-semenov
Copy link
Contributor Author

/verify with intel/llvm-test-suite#1604

@sergey-semenov sergey-semenov temporarily deployed to aws February 17, 2023 15:45 — with GitHub Actions Inactive
@sergey-semenov sergey-semenov temporarily deployed to aws February 21, 2023 06:08 — with GitHub Actions Inactive
@sergey-semenov sergey-semenov temporarily deployed to aws February 21, 2023 10:33 — with GitHub Actions Inactive
@sergey-semenov sergey-semenov temporarily deployed to aws February 21, 2023 14:55 — with GitHub Actions Inactive
@sergey-semenov sergey-semenov temporarily deployed to aws February 21, 2023 15:57 — with GitHub Actions Inactive
@sergey-semenov
Copy link
Contributor Author

@dm-vodopyanov @intel/llvm-reviewers-runtime Could you please take a look?

@bader bader merged commit 473bea3 into intel:sycl Feb 22, 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