Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions sycl/doc/SYCLInstrumentationUsingXPTI.md

Large diffs are not rendered by default.

11 changes: 11 additions & 0 deletions sycl/source/detail/device_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,17 @@ bool device_impl::isAssertFailSupported() const {
return MIsAssertFailSupported;
}

std::string device_impl::getDeviceName() const {
if (MDeviceName.empty()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if it is possible that race condition will happen on check for empty MDeviceName here?

Suggested change
if (MDeviceName.empty()) {
{

isn't it better to init MDeviceName in the constructor and use it without mutexes afterward?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Race condition will occur here, and that's why I double check it after mutex is acquired. As for the constructors, this might have been a good idea, but we have plenty of those, and I'm worried eventually we'll screw things up and report incorrect values.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On the second thought, what I want to do here is std::call_once

std::lock_guard Lock{MDeviceNameMutex};
if (MDeviceName.empty()) {
MDeviceName = get_info<info::device::name>();
}
}

return MDeviceName;
}

} // namespace detail
} // namespace sycl
} // __SYCL_INLINE_NAMESPACE(cl)
5 changes: 5 additions & 0 deletions sycl/source/detail/device_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <detail/platform_impl.hpp>

#include <memory>
#include <mutex>

__SYCL_INLINE_NAMESPACE(cl) {
namespace sycl {
Expand Down Expand Up @@ -224,6 +225,8 @@ class device_impl {

bool isAssertFailSupported() const;

std::string getDeviceName() const;

private:
explicit device_impl(pi_native_handle InteropDevice, RT::PiDevice Device,
PlatformImplPtr Platform, const plugin &Plugin);
Expand All @@ -233,6 +236,8 @@ class device_impl {
bool MIsHostDevice;
PlatformImplPtr MPlatform;
bool MIsAssertFailSupported = false;
mutable std::string MDeviceName;
mutable std::mutex MDeviceNameMutex;
}; // class device_impl

} // namespace detail
Expand Down
10 changes: 5 additions & 5 deletions sycl/source/detail/queue_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,12 @@ void *queue_impl::instrumentationProlog(const detail::code_location &CodeLoc,
DevStr = "ACCELERATOR";
else
DevStr = "UNKNOWN";
xptiAddMetadata(WaitEvent, "sycl_device", DevStr.c_str());
xpti::addMetadata(WaitEvent, "sycl_device", DevStr);
if (HasSourceInfo) {
xptiAddMetadata(WaitEvent, "sym_function_name", CodeLoc.functionName());
xptiAddMetadata(WaitEvent, "sym_source_file_name", CodeLoc.fileName());
xptiAddMetadata(WaitEvent, "sym_line_no",
std::to_string(CodeLoc.lineNumber()).c_str());
xpti::addMetadata(WaitEvent, "sym_function_name", CodeLoc.functionName());
xpti::addMetadata(WaitEvent, "sym_source_file_name", CodeLoc.fileName());
xpti::addMetadata(WaitEvent, "sym_line_no",
std::to_string(CodeLoc.lineNumber()));
}
xptiNotifySubscribers(StreamID, xpti::trace_wait_begin, nullptr, WaitEvent,
QWaitInstanceNo,
Expand Down
184 changes: 119 additions & 65 deletions sycl/source/detail/scheduler/commands.cpp

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions sycl/source/detail/scheduler/commands.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <cstdint>
#include <deque>
#include <memory>
#include <optional>
#include <set>
#include <unordered_set>
#include <vector>
Expand Down Expand Up @@ -152,9 +153,9 @@ class Command {
/// instrumentation to report these dependencies as edges.
void resolveReleaseDependencies(std::set<Command *> &list);
/// Creates an edge event when the dependency is a command.
void emitEdgeEventForCommandDependence(Command *Cmd, void *ObjAddr,
const std::string &Prefix,
bool IsCommand);
void emitEdgeEventForCommandDependence(
Command *Cmd, void *ObjAddr, bool IsCommand,
std::optional<access::mode> AccMode = std::nullopt);
/// Creates an edge event when the dependency is an event.
void emitEdgeEventForEventDependence(Command *Cmd, RT::PiEvent &EventAddr);
/// Creates a signal event with the enqueued kernel event handle.
Expand Down
2 changes: 1 addition & 1 deletion xpti/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set(CMAKE_CXX_STANDARD 14)

set(XPTI_DIR ${CMAKE_CURRENT_LIST_DIR})
# Setting the same version as SYCL
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD 17)

option(XPTI_ENABLE_WERROR OFF)

Expand Down
18 changes: 17 additions & 1 deletion xpti/include/xpti/xpti_data_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ enum class payload_flag_t {
using trace_point_t = uint16_t;
using event_type_t = uint16_t;
using string_id_t = int32_t;
using object_id_t = int32_t;

using safe_flag_t = std::atomic<bool>;
using safe_uint64_t = std::atomic<uint64_t>;
Expand All @@ -113,7 +114,7 @@ using safe_uint16_t = std::atomic<uint16_t>;
using safe_int64_t = std::atomic<int64_t>;
using safe_int32_t = std::atomic<int32_t>;
using safe_int16_t = std::atomic<int16_t>;
using metadata_t = std::unordered_map<string_id_t, string_id_t>;
using metadata_t = std::unordered_map<string_id_t, object_id_t>;

#define XPTI_EVENT(val) xpti::event_type_t(val)
#define XPTI_TRACE_POINT_BEGIN(val) xpti::trace_point_t(val << 1 | 0)
Expand All @@ -123,6 +124,12 @@ using metadata_t = std::unordered_map<string_id_t, string_id_t>;
#define XPTI_PACK16_RET32(value1, value2) ((value1 << 16) | value2)
#define XPTI_PACK32_RET64(value1, value2) (((uint64_t)value1 << 32) | value2)

struct object_data_t {
size_t size;
const char *data;
uint8_t type;
};

/// @brief Payload data structure that is optional for trace point callback
/// API
/// @details The payload structure, if determined at compile time, can deliver
Expand Down Expand Up @@ -456,6 +463,15 @@ enum class trace_activity_type_t {
sleep_activity = 1 << 3
};

enum class metadata_type_t {
binary = 0,
string = 1,
signed_integer = 2,
unsigned_integer = 3,
floating = 4,
boolean = 5
};

struct reserved_data_t {
/// Has a reference to the associated payload field for an event
payload_t *payload = nullptr;
Expand Down
12 changes: 10 additions & 2 deletions xpti/include/xpti/xpti_trace_framework.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ XPTI_EXPORT_API xpti::string_id_t xptiRegisterString(const char *string,
/// @return A reference to the string identified by the string ID.
XPTI_EXPORT_API const char *xptiLookupString(xpti::string_id_t id);

XPTI_EXPORT_API xpti::object_id_t xptiRegisterObject(const char *data,
size_t size, uint8_t type);

XPTI_EXPORT_API xpti::object_data_t xptiLookupObject(xpti::object_id_t id);

/// @brief Register a payload with the framework
/// @details Since a payload may contain multiple strings that may have been
/// defined on the stack, it is recommended the payload object is registered
Expand Down Expand Up @@ -367,7 +372,7 @@ xptiNotifySubscribers(uint8_t stream_id, uint16_t trace_type,
/// 3. XPTI_RESULT_DUPLICATE when the key-value pair already exists
XPTI_EXPORT_API xpti::result_t xptiAddMetadata(xpti::trace_event_data_t *e,
const char *key,
const char *value);
xpti::object_id_t value_id);

/// @brief Query the metadata table for a given event
/// @details In order to retrieve metadata information for a given event, you
Expand Down Expand Up @@ -420,6 +425,9 @@ typedef void (*xpti_finalize_t)(const char *);
typedef uint64_t (*xpti_get_unique_id_t)();
typedef xpti::string_id_t (*xpti_register_string_t)(const char *, char **);
typedef const char *(*xpti_lookup_string_t)(xpti::string_id_t);
typedef xpti::string_id_t (*xpti_register_object_t)(const char *, size_t,
uint8_t);
typedef xpti::object_data_t (*xpti_lookup_object_t)(xpti::object_id_t);
typedef uint64_t (*xpti_register_payload_t)(xpti::payload_t *);
typedef uint8_t (*xpti_register_stream_t)(const char *);
typedef xpti::result_t (*xpti_unregister_stream_t)(const char *);
Expand All @@ -440,7 +448,7 @@ typedef xpti::result_t (*xpti_notify_subscribers_t)(
uint8_t, uint16_t, xpti::trace_event_data_t *, xpti::trace_event_data_t *,
uint64_t instance, const void *temporal_user_data);
typedef xpti::result_t (*xpti_add_metadata_t)(xpti::trace_event_data_t *,
const char *, const char *);
const char *, xpti::object_id_t);
typedef xpti::metadata_t *(*xpti_query_metadata_t)(xpti::trace_event_data_t *);
typedef bool (*xpti_trace_enabled_t)();
}
161 changes: 161 additions & 0 deletions xpti/include/xpti/xpti_trace_framework.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
//
#pragma once

#include <cassert>
#include <cstdint>
#include <cstring>
#include <memory>
#include <sstream>
#include <thread>
Expand Down Expand Up @@ -275,6 +277,165 @@ class PlatformHelper {
};
} // namespace utils

template <typename T>
inline result_t addMetadata(trace_event_data_t *Event, const std::string &Key,
const T &Data) {
static_assert(std::is_trivially_copyable_v<T>,
"T must be trivially copyable");
static_assert(!std::is_same_v<T, const char *>);

const uint8_t Type = [] {
if (std::is_same_v<bool, T>) {
return static_cast<uint8_t>(metadata_type_t::boolean);
}
if (std::numeric_limits<T>::is_integer &&
std::numeric_limits<T>::is_signed) {
return static_cast<uint8_t>(metadata_type_t::signed_integer);
}
if (std::numeric_limits<T>::is_integer &&
!std::numeric_limits<T>::is_signed) {
return static_cast<uint8_t>(metadata_type_t::unsigned_integer);
}
if (std::numeric_limits<T>::is_specialized &&
!std::numeric_limits<T>::is_integer) {
return static_cast<uint8_t>(metadata_type_t::floating);
}

return static_cast<uint8_t>(metadata_type_t::binary);
}();

object_id_t Value = xptiRegisterObject(reinterpret_cast<const char *>(&Data),
sizeof(Data), Type);
return xptiAddMetadata(Event, Key.c_str(), Value);
}

template <>
inline result_t addMetadata<std::string>(trace_event_data_t *Event,
const std::string &Key,
const std::string &Data) {
const uint8_t Type = static_cast<uint8_t>(metadata_type_t::string);
object_id_t Value = xptiRegisterObject(Data.c_str(), Data.size(), Type);
return xptiAddMetadata(Event, Key.c_str(), Value);
}

template <>
inline result_t addMetadata<const char *>(trace_event_data_t *Event,
const std::string &Key,
const char *const &Data) {
const uint8_t Type = static_cast<uint8_t>(metadata_type_t::string);
object_id_t Value = xptiRegisterObject(Data, strlen(Data), Type);
return xptiAddMetadata(Event, Key.c_str(), Value);
}

template <typename T>
inline std::pair<std::string_view, T>
getMetadata(const metadata_t::value_type &MD) {
static_assert(std::is_trivially_copyable<T>::value,
"T must be trivially copyable");

object_data_t RawData = xptiLookupObject(MD.second);
assert(RawData.size == sizeof(T));

T Value = *reinterpret_cast<const T *>(RawData.data);

const char *Key = xptiLookupString(MD.first);

return std::make_pair(std::string_view(Key), Value);
}

template <>
inline std::pair<std::string_view, std::string>
getMetadata(const metadata_t::value_type &MD) {
object_data_t RawData = xptiLookupObject(MD.second);

std::string Value(RawData.data, RawData.size);

const char *Key = xptiLookupString(MD.first);

return std::make_pair(std::string_view(Key), Value);
}

template <>
inline std::pair<std::string_view, std::string_view>
getMetadata(const metadata_t::value_type &MD) {
object_data_t RawData = xptiLookupObject(MD.second);

std::string_view Value(RawData.data, RawData.size);

const char *Key = xptiLookupString(MD.first);

return std::make_pair(std::string_view(Key), Value);
}

inline std::string readMetadata(const metadata_t::value_type &MD) {
object_data_t RawData = xptiLookupObject(MD.second);

if (RawData.type == static_cast<uint8_t>(metadata_type_t::binary)) {
return std::string("Binary data, size: ") + std::to_string(RawData.size);
}

if (RawData.type == static_cast<uint8_t>(metadata_type_t::boolean)) {
bool Value = *reinterpret_cast<const bool *>(RawData.data);
return Value ? "true" : "false";
}

if (RawData.type == static_cast<uint8_t>(metadata_type_t::signed_integer)) {
if (RawData.size == 1) {
auto I = *reinterpret_cast<const int8_t *>(RawData.data);
return std::to_string(I);
}
if (RawData.size == 2) {
auto I = *reinterpret_cast<const int16_t *>(RawData.data);
return std::to_string(I);
}
if (RawData.size == 4) {
auto I = *reinterpret_cast<const int32_t *>(RawData.data);
return std::to_string(I);
}
if (RawData.size == 8) {
auto I = *reinterpret_cast<const int64_t *>(RawData.data);
return std::to_string(I);
}
}

if (RawData.type == static_cast<uint8_t>(metadata_type_t::unsigned_integer)) {
if (RawData.size == 1) {
auto I = *reinterpret_cast<const uint8_t *>(RawData.data);
return std::to_string(I);
}
if (RawData.size == 2) {
auto I = *reinterpret_cast<const uint16_t *>(RawData.data);
return std::to_string(I);
}
if (RawData.size == 4) {
auto I = *reinterpret_cast<const uint32_t *>(RawData.data);
return std::to_string(I);
}
if (RawData.size == 8) {
auto I = *reinterpret_cast<const uint64_t *>(RawData.data);
return std::to_string(I);
}
}

if (RawData.type == static_cast<uint8_t>(metadata_type_t::floating)) {
if (RawData.size == 4) {
auto F = *reinterpret_cast<const float *>(RawData.data);
return std::to_string(F);
}
if (RawData.size == 8) {
auto F = *reinterpret_cast<const double *>(RawData.data);
return std::to_string(F);
}
}

if (RawData.type == static_cast<uint8_t>(metadata_type_t::string)) {
return std::string(RawData.data, RawData.size);
}

return std::string("Unknown metadata type, size ") +
std::to_string(RawData.size);
}

namespace framework {
static thread_local uint64_t g_tls_uid = xpti::invalid_uid;
constexpr uint16_t signal = (uint16_t)xpti::trace_point_type_t::signal;
Expand Down
Loading