Skip to content

[SYCL][L0] Improve L0 timestamps #6006

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 1 commit into from
Apr 13, 2022
Merged

Conversation

jandres742
Copy link
Contributor

  • Timer resolution is returned by L0 by default on nanoseconds.
  • Properly mask kernel timestamp based on valid bits.
  • Return ContextEndTime as end time, not as delta wrt start.

Signed-off-by: Jaime Arteaga [email protected]

@jandres742 jandres742 requested a review from a team as a code owner April 13, 2022 02:43
@jandres742 jandres742 force-pushed the fixL0timestamps branch 2 times, most recently from dde0594 to 18bf9b2 Compare April 13, 2022 13:16
@jandres742 jandres742 changed the title Improve L0 timestamps [SYCL][L0] Improve L0 timestamps Apr 13, 2022
- Timer resolution is returned by L0 by default on nanoseconds.
- Properly mask kernel timestamp based on valid bits.
- Return ContextEndTime as end time, not as delta wrt start.

Signed-off-by: Jaime Arteaga <[email protected]>
1ULL)
: ((1ULL << Event->Context->Devices[0]
->ZeDeviceProperties->kernelTimestampValidBits) -
1ULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please compute device first and the use it 5146-5158 to make it cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smaslov-intel : Please see here #6010

@@ -5157,27 +5164,27 @@ pi_result piEventGetProfilingInfo(pi_event Event, pi_profiling_info ParamName,
switch (ParamName) {
case PI_PROFILING_INFO_COMMAND_START: {
ZE_CALL(zeEventQueryKernelTimestamp, (Event->ZeEvent, &tsResult));
uint64_t ContextStartTime = tsResult.context.kernelStart * ZeTimerFreq;
uint64_t ContextStartTime =
(tsResult.context.kernelStart & TimestampMaxValue) * ZeTimerResolution;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that the Level-Zero spec guidance needs to be adjusted?

https://spec.oneapi.io/level-zero/latest/core/PROG.html#kernel-timestamp-events

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smaslov-intel if you are referring to

const double timestampFreq = NS_IN_SEC / device_properties.timerResolution;

this is valid only if you use ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES_1_2

uint64_t timerResolution
[out] Returns the resolution of device timer used for profiling, timestamps, etc.
When stype==ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES the units are in nanoseconds.
When stype==ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES_1_2 units are in cycles/sec

is that what you are referring to or something else in the spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, thanks

const uint64_t TimestampMaxValue =
(1LL << Device->ZeDeviceProperties->kernelTimestampValidBits) - 1;
ContextEndTime += TimestampMaxValue - ContextStartTime;
ContextEndTime += TimestampMaxValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

@againull againull merged commit 7efb3e6 into intel:sycl Apr 13, 2022
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.

3 participants