-
Notifications
You must be signed in to change notification settings - Fork 788
[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
Conversation
dde0594
to
18bf9b2
Compare
- 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); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
Signed-off-by: Jaime Arteaga [email protected]