Skip to content

Commit 3bab70f

Browse files
tovinkeretovinkere
committed
[XPTI][SYCL] Fixes failing tests
 - Address the failing tests by fixing the checks as the streams are now initialized in XPTIRegistry - Addresses a bug in xpti::tracepoint_scope_t that caused a failure in test-e2e/Tracing/task_execution.cpp due to missing metadata Signed-off-by: Vasanth Tovinkere <[email protected]>
1 parent 82f0a33 commit 3bab70f

File tree

8 files changed

+49
-49
lines changed

8 files changed

+49
-49
lines changed

‎sycl/source/detail/global_handler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void GlobalHandler::TraceEventXPTI(const char *Message) {
9696
CodeLocation.fileName(), CodeLocation.functionName(),
9797
CodeLocation.lineNumber(), CodeLocation.columnNumber(), nullptr);
9898

99-
TP.stream(GSYCLStreamID)
99+
TP.stream(detail::GSYCLStreamID)
100100
.traceType(xpti::trace_point_type_t::diagnostics)
101101
.parentEvent(GSYCLCallEvent)
102102
.notify(static_cast<const void *>(Message));

‎sycl/source/detail/queue_impl.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ event queue_impl::memset(void *Ptr, int Value, size_t Count,
167167
xpti::framework::tracepoint_scope_t TP(
168168
CodeLocation.fileName(), FuncName, CodeLocation.lineNumber(),
169169
CodeLocation.columnNumber(), (void *)this);
170-
TP.stream(GSYCLStreamID)
170+
TP.stream(detail::GSYCLStreamID)
171171
.traceType(xpti::trace_point_type_t::node_create)
172-
.parentEvent(GSYCLGraphEvent);
172+
.parentEvent(detail::GSYCLGraphEvent);
173173

174174
TP.addMetadata([&](auto TEvent) {
175175
xpti::addMetadata(TEvent, "sycl_device",
@@ -219,7 +219,7 @@ event queue_impl::memcpy(void *Dest, const void *Src, size_t Count,
219219
xpti::framework::tracepoint_scope_t TP(
220220
CodeLoc.fileName(), CodeLoc.functionName(), CodeLoc.lineNumber(),
221221
CodeLoc.columnNumber(), (void *)this);
222-
TP.stream(GSYCLStreamID)
222+
TP.stream(detail::GSYCLStreamID)
223223
.traceType(xpti::trace_point_type_t::node_create)
224224
.parentEvent(GSYCLGraphEvent);
225225
const char *UserData = "memory_transfer_node::memcpy";
@@ -692,7 +692,7 @@ void queue_impl::constructorNotification() {
692692
#if XPTI_ENABLE_INSTRUMENTATION
693693
if (xptiTraceEnabled()) {
694694
// Making it ABI compatible and not removing the member variable
695-
MStreamID = GSYCLStreamID;
695+
MStreamID = detail::GSYCLStreamID;
696696
constexpr uint16_t NotificationTraceType =
697697
static_cast<uint16_t>(xpti::trace_point_type_t::queue_create);
698698
if (xptiCheckTraceEnabled(MStreamID, NotificationTraceType)) {

‎sycl/source/detail/scheduler/commands.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ Command::Command(
575575
return;
576576
// Obtain the stream ID so all commands can emit traces to that stream;
577577
// copying it to the member variable to avoid ABI breakage
578-
MStreamID = GSYCLStreamID;
578+
MStreamID = detail::GSYCLStreamID;
579579
#endif
580580
}
581581

‎sycl/source/detail/ur.cpp‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,20 @@ static void initializeAdapters(std::vector<adapter_impl *> &Adapters,
119119
} \
120120
}
121121

122+
#ifdef XPTI_ENABLE_INSTRUMENTATION
123+
// We want XPTI initialized as early as possible, so we do it here. This
124+
// allows XPTI calls in the loader to be pre-initialized.
125+
if (xptiTraceEnabled()) {
126+
// Initialize the XPTI framework.
127+
// Not sure this is the best place to initialize the framework; SYCL runtime
128+
// team needs to advise on the right place, until then we piggy-back on the
129+
// initialization of the UR layer.
130+
131+
// This is done only once, even if multiple adapters are initialized.
132+
GlobalHandler::instance().getXPTIRegistry().initializeFrameworkOnce();
133+
}
134+
#endif
135+
122136
UrFuncInfo<UrApiKind::urLoaderConfigCreate> loaderConfigCreateInfo;
123137
auto loaderConfigCreate =
124138
loaderConfigCreateInfo.getFuncPtrFromModule(ur::getURLoaderLibrary());
@@ -239,17 +253,6 @@ static void initializeAdapters(std::vector<adapter_impl *> &Adapters,
239253
}
240254
}
241255

242-
#ifdef XPTI_ENABLE_INSTRUMENTATION
243-
if (xptiTraceEnabled()) {
244-
// Initialize the XPTI framework.
245-
// Not sure this is the best place to initialize the framework; SYCL runtime
246-
// team needs to advise on the right place, until then we piggy-back on the
247-
// initialization of the UR layer.
248-
249-
// This is done only once, even if multiple adapters are initialized.
250-
GlobalHandler::instance().getXPTIRegistry().initializeFrameworkOnce();
251-
}
252-
#endif
253256
#undef CHECK_UR_SUCCESS
254257
}
255258

‎sycl/source/detail/xpti_registry.hpp‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -77,44 +77,44 @@ class XPTIRegistry {
7777
xptiFrameworkInitialize();
7878
// Register the streams that we will be using
7979
// SYCL events
80-
GSYCLStreamID =
80+
detail::GSYCLStreamID =
8181
this->initializeStream(SYCL_STREAM_NAME, GMajVer, GMinVer, GVerStr);
8282
// SYCL buffer events
83-
GBufferStreamID = this->initializeStream(SYCL_BUFFER_STREAM_NAME, GMajVer,
84-
GMinVer, GVerStr);
83+
detail::GBufferStreamID = this->initializeStream(
84+
SYCL_BUFFER_STREAM_NAME, GMajVer, GMinVer, GVerStr);
8585
// SYCL image events
86-
GImageStreamID = this->initializeStream(SYCL_IMAGE_STREAM_NAME, GMajVer,
87-
GMinVer, GVerStr);
86+
detail::GImageStreamID = this->initializeStream(
87+
SYCL_IMAGE_STREAM_NAME, GMajVer, GMinVer, GVerStr);
8888
// Memory allocation events
89-
GMemAllocStreamID = this->initializeStream(SYCL_MEM_ALLOC_STREAM_NAME,
90-
GMajVer, GMinVer, GVerStr);
89+
detail::GMemAllocStreamID = this->initializeStream(
90+
SYCL_MEM_ALLOC_STREAM_NAME, GMajVer, GMinVer, GVerStr);
9191
// UR API events
92-
GUrApiStreamID =
92+
detail::GUrApiStreamID =
9393
this->initializeStream(UR_API_STREAM_NAME, GMajVer, GMinVer, GVerStr);
9494

9595
auto SYCLEventTP = xptiCreateTracepoint("sycl.application.graph", nullptr,
9696
0, 0, nullptr);
97-
GSYCLGraphEvent = SYCLEventTP->event_ref();
98-
if (GSYCLGraphEvent) {
97+
detail::GSYCLGraphEvent = SYCLEventTP->event_ref();
98+
if (detail::GSYCLGraphEvent) {
9999
// The graph event is a global event and will be used as the parent for
100100
// all nodes (command groups, memory allocations, etc)
101-
xptiNotifySubscribers(GSYCLStreamID, xpti::trace_graph_create, nullptr,
102-
GSYCLGraphEvent, GSYCLGraphEvent->instance_id,
103-
nullptr);
101+
xptiNotifySubscribers(detail::GSYCLStreamID, xpti::trace_graph_create,
102+
nullptr, detail::GSYCLGraphEvent,
103+
detail::GSYCLGraphEvent->instance_id, nullptr);
104104
}
105105
auto MemAllocEventTP =
106106
xptiCreateTracepoint("sycl.memory.alloc", nullptr, 0, 0, nullptr);
107-
GMemAllocEvent = MemAllocEventTP->event_ref();
107+
detail::GMemAllocEvent = MemAllocEventTP->event_ref();
108108

109109
// We capture all API calls in a single event, so that we can minimize
110110
// XPTI infra calls
111111
auto APIEventTP =
112112
xptiCreateTracepoint("api.function", nullptr, 0, 0, nullptr);
113-
GApiEvent = APIEventTP->event_ref();
113+
detail::GApiEvent = APIEventTP->event_ref();
114114

115115
auto SYCLExceptionsTP =
116116
xptiCreateTracepoint("sycl.exceptions", nullptr, 0, 0, nullptr);
117-
GSYCLCallEvent = SYCLExceptionsTP->event_ref();
117+
detail::GSYCLCallEvent = SYCLExceptionsTP->event_ref();
118118
});
119119
#endif
120120
}

‎sycl/test-e2e/XPTI/basic_event_collection_linux.cpp‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,17 @@
55

66
#include "basic_event_collection.inc"
77
//
8-
// CHECK: xptiTraceInit: Stream Name = ur.call
9-
// CHECK: xptiTraceInit: Stream Name = sycl.experimental.mem_alloc
10-
// CHECK: xptiTraceInit: Stream Name = sycl
11-
// CHECK-NEXT: Graph create
12-
// CHECK: UR Call Begin : urPlatformGet
13-
// CHECK: UR Call Begin : urContextCreate
14-
// CHECK: UR Call Begin : urQueueCreate
15-
// CHECK: UR Call Begin : urDeviceSelectBinary
16-
// CHECK: UR Call Begin : urKernelCreate
17-
// CHECK-NEXT: UR Call Begin : urPlatformGetInfo
18-
// CHECK-NEXT: UR Call Begin : urPlatformGetInfo
19-
// CHECK-NEXT: UR Call Begin : urKernelSetExecInfo
20-
// CHECK-NEXT: UR Call Begin : urKernelRetain
21-
// CHECK: UR Call Begin : urKernelGetGroupInfo
22-
// CHECK-NEXT: UR Call Begin : urEnqueueKernelLaunchWithArgsExp
8+
// CHECK-DAG: xptiTraceInit: Stream Name = sycl
9+
// CHECK-DAG: xptiTraceInit: Stream Name = sycl.experimental.buffer
10+
// CHECK-DAG: xptiTraceInit: Stream Name = sycl.experimental.image
11+
// CHECK-DAG: xptiTraceInit: Stream Name = sycl.experimental.mem_alloc
12+
// CHECK-DAG: xptiTraceInit: Stream Name = ur.api
13+
// CHECK-DAG: xptiTraceInit: Stream Name = ur.call
14+
// CHECK-DAG: Graph create
15+
// CHECK-DAG: UR Call Begin : urPlatformGet
16+
// CHECK-DAG: UR Call Begin : urContextCreate
17+
// CHECK-DAG: UR Call Begin : urQueueCreate
18+
// CHECK-DAG: UR Call Begin : urDeviceSelectBinary
2319
// CHECK: UR Call Begin : urKernelCreate
2420
// CHECK-NEXT: UR Call Begin : urPlatformGetInfo
2521
// CHECK-NEXT: UR Call Begin : urPlatformGetInfo

‎xpti/include/xpti/xpti_trace_framework.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,7 @@ class tracepoint_scope_t {
12861286
///
12871287
tracepoint_scope_t &
12881288
addMetadata(const std::function<void(xpti::trace_event_data_t *)> &Callback) {
1289-
if (xptiCheckTraceEnabled(MStreamId, MTraceType) && MTraceEvent) {
1289+
if (MTraceEvent) {
12901290
Callback(MTraceEvent);
12911291
}
12921292
return *this;

‎xptifw/src/xpti_trace_framework.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,6 +1767,7 @@ class Notifications {
17671767
// value
17681768
if (StreamFlags.count(TraceType) == 0)
17691769
return false;
1770+
// Otherwise, it is success
17701771
return StreamFlags[TraceType];
17711772
}
17721773
}

0 commit comments

Comments
 (0)