-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL] Fix post-commit issues after PR#11548 #11682
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
sycl/source/handler.cpp
Outdated
@@ -239,9 +239,11 @@ event handler::finalize() { | |||
#ifdef XPTI_ENABLE_INSTRUMENTATION | |||
// uint32_t StreamID, uint64_t InstanceID, xpti_td* TraceEvent, | |||
int32_t StreamID = xptiRegisterStream(detail::SYCL_STREAM_NAME); | |||
auto [CmdTraceEvent, InstanceID] = emitKernelInstrumentationData( | |||
auto InstrumentationData = emitKernelInstrumentationData( |
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.
I'd rather keep the structured binding and re-write the lambda like this: https://godbolt.org/z/ofqcoaTGr
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.
Good idea, fixed.
const void *, uint64_t InstanceID, | ||
bool IsBegin) { |
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.
const void *, uint64_t InstanceID, | |
bool IsBegin) { | |
[[maybe_unused]] const void *UserData, | |
uint64_t InstanceID, bool IsBegin) { |
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.
Good idea, fixed.
@@ -76,7 +76,7 @@ void TraceTaskExecutionSignals(xpti::trace_event_data_t * /*Parent*/, | |||
|
|||
void TraceQueueLifetimeSignals(xpti::trace_event_data_t * /*Parent*/, | |||
xpti::trace_event_data_t *Event, | |||
const void *UserData, bool IsCreation) { | |||
const void *, bool IsCreation) { |
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.
likewise.
@againull, @aelovikov-intel thank you for fixing this |
auto EnqueueKernel = [&, CmdTraceEvent = CmdTraceEvent, | ||
InstanceID = InstanceID]() { |
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.
This breaks compilation when XPTI_ENABLE_INSTRUMENTATION
is not defined.
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.
Fix is here: #11742
Uh oh!
There was an error while loading. Please reload this page.