Skip to content

Add umfInit/umfTearDown to urAdapterGet/urAdapterRelease #2007

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.

Sign up for GitHub

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
igchor merged 2 commits into from
Aug 27, 2024
Merged

Add umfInit/umfTearDown to urAdapterGet/urAdapterRelease #2007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eeaa897
Add umfInit/umfTearDown to urAdapterGet/urAdapterRelease
Aug 23, 2024
581a0d8
Add a workaround for the logger on windows
Aug 27, 2024
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
31 changes: 28 additions & 3 deletions source/adapters/level_zero/adapter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,27 @@
#include "ur_level_zero.hpp"
#include <iomanip>

// As windows order of unloading dlls is reversed from linux, windows will call
// umfTearDown before it could release umf objects in level_zero, so we call
// umfInit on urAdapterGet and umfAdapterTearDown to enforce the teardown of umf
// after umf objects are destructed.
#if defined(_WIN32)
#include <umf.h>
#endif

// Due to multiple DLLMain definitions with SYCL, Global Adapter is init at
// variable creation.
#if defined(_WIN32)
ur_adapter_handle_t_ *GlobalAdapter = new ur_adapter_handle_t_();
#else
ur_adapter_handle_t_ *GlobalAdapter;
#endif

// This is a temporary workaround on windows, where UR adapter is teardowned
// before the UR loader, which will result in access violation when we use print
// function as the overrided print function was already released with the UR
// adapter.
// TODO: Change adapters to use a common sink class in the loader instead of
// using thier own sink class that inherit from logger::Sink.
Comment on lines +30 to +35
Copy link
Contributor Author

@omarahmed1111 omarahmed1111 Aug 27, 2024

Choose a reason for hiding this comment

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

This is not ideal, but it should work for now to solve a release problem regarding the logger. @igchor @nrspruit If this is okay, I will merge this asap.

Copy link
Contributor

@igchor igchor Aug 27, 2024

Choose a reason for hiding this comment

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

Yeah, I think that's fine for now. Eventually, we could do a similar thing like here: #2000. Just keep a global instance of shared_ptr<> to the logger (here

unified-runtime/source/common/logger/ur_logger.hpp

Line 24 in b766009

static Logger logger =
) and every time get_logger is called, we would return shared_ptr instead of a reference.

class ur_legacy_sink : public logger::Sink {
public:
ur_legacy_sink(std::string logger_name = "", bool skip_prefix = true)
Expand All @@ -32,7 +45,11 @@ class ur_legacy_sink : public logger::Sink {
fprintf(stderr, "%s", msg.c_str());
}

~ur_legacy_sink() = default;
~ur_legacy_sink() {
#if defined(_WIN32)
logger::isTearDowned = true;
#endif
};
};

ur_result_t initPlatforms(PlatformVec &platforms) noexcept try {
Expand Down Expand Up @@ -74,7 +91,14 @@ ur_result_t initPlatforms(PlatformVec &platforms) noexcept try {
return exceptionToResult(std::current_exception());
}

ur_result_t adapterStateInit() { return UR_RESULT_SUCCESS; }
ur_result_t adapterStateInit() {

#if defined(_WIN32)
umfInit();
#endif

return UR_RESULT_SUCCESS;
}

ur_adapter_handle_t_::ur_adapter_handle_t_()
: logger(logger::get_logger("level_zero")) {
Expand Down Expand Up @@ -258,6 +282,7 @@ ur_result_t adapterStateTeardown() {
// Due to multiple DLLMain definitions with SYCL, register to cleanup the
// Global Adapter after refcnt is 0
#if defined(_WIN32)
umfTearDown();
std::atexit(globalAdapterOnDemandCleanup);
#endif

Expand Down
18 changes: 18 additions & 0 deletions source/common/logger/ur_sinks.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

namespace logger {

#if defined(_WIN32)
inline bool isTearDowned = false;
#endif

class Sink {
public:
template <typename... Args>
Expand All @@ -28,7 +32,21 @@ class Sink {
}

format(buffer, fmt, std::forward<Args &&>(args)...);
// This is a temporary workaround on windows, where UR adapter is teardowned
// before the UR loader, which will result in access violation when we use print
// function as the overrided print function was already released with the UR
// adapter.
// TODO: Change adapters to use a common sink class in the loader instead of
// using thier own sink class that inherit from logger::Sink.
#if defined(_WIN32)
if (isTearDowned) {
std::cerr << buffer.str() << "\n";
} else {
print(level, buffer.str());
}
#else
print(level, buffer.str());
#endif
}

void setFlushLevel(logger::Level level) { this->flush_level = level; }
Expand Down