Skip to content

[UR] Logger callback function sink #17095

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

Merged
merged 6 commits into from
Apr 10, 2025

Conversation

martygrant
Copy link
Contributor

@martygrant martygrant commented Feb 20, 2025

Migrated from oneapi-src/unified-runtime#1748

This PR implements oneapi-src/unified-runtime#1330 through a new logger sink: a user configurable callback. It introduces some spec additions:

  • typedef void (*ur_logger_output_callback_t)(ur_logger_level_t level, const char *pLoggerMsg, void *pUserData)
  • urSetLoggerCallback(ur_adapter_handle_t hAdapter, ur_logger_output_callback_t pfnLoggerCallback, void *pUserData, ur_logger_level_t level
    )`
  • urSetLoggerCallbackLevel(ur_adapter_handle_t hAdapter, ur_logger_level_t level)
  • typedef enum ur_logger_level_t (moved the logger::level enum into the spec)

This new logger sink will only be constructed once a user makes a call to urSetLoggerCallback (which is called from the UR urAdapterSetLoggerCallback entry point), supplying their own callback function. They can set the minimum logging level through urSetLoggerCallbackLevel. Any subsequent logging calls will additionally make a call to the supplied callback where the log level, message and user data will be sent.

A callback has been setup in the SYCL RT in sycl/source/detail/ur.cpp to print logs to std::err:

void urLoggerCallback([[maybe_unused]] ur_logger_level_t level, const char *msg,
                      [[maybe_unused]] void *userData) {
  std::cerr << msg << std::endl;
}

A new test suite LoggerWithCallbackSink has been added to test this new functionality.

@martygrant
Copy link
Contributor Author

Pre commit AMD job is failing with Memory access fault by GPU node-1 (Agent handle: 0x10478090) on address 0x7b62a0014000. Reason: Page not present or supervisor privilege. this is being tracked here #10460

@martygrant
Copy link
Contributor Author

@intel/dpcpp-nativecpu-reviewers @intel/llvm-reviewers-cuda need a review for this PR please

@martygrant martygrant force-pushed the martin/loggerCallbackSink branch 2 times, most recently from 0e668ea to ecc346c Compare April 10, 2025 09:27
@martygrant martygrant force-pushed the martin/loggerCallbackSink branch from ecc346c to 24655c5 Compare April 10, 2025 09:47
@martygrant martygrant force-pushed the martin/loggerCallbackSink branch from 24655c5 to 537764f Compare April 10, 2025 13:08
martygrant and others added 6 commits April 10, 2025 14:12
… callback function. Configurable through two new entry points with adapter implementations: urAdapterSetLoggerCallback() and urAdapterSetLoggerCallbackLevel(). Moved logger::level enum to the spec, named ur_logger_level_t. Added new unit test suite for these entry points.

Co-authored-by: Kenneth Benzie (Benie) <[email protected]>
…ssage and link up a logger callback from SYCL RT to simply print to stdout, this is setup during adapter

initialisation in sycl/source/detail.cpp

also remove setErrorMessage from OpenCL and Native CPU adapters where there was no usage
@martygrant martygrant force-pushed the martin/loggerCallbackSink branch from 537764f to cef6bb2 Compare April 10, 2025 13:14
@martygrant
Copy link
Contributor Author

@intel/unified-runtime-reviewers-level-zero need a review for this please, this was a previously approved PR in the old UR repo but has had some new changes since then

@martygrant martygrant merged commit 87acf73 into intel:sycl Apr 10, 2025
32 checks passed
RossBrunton added a commit to RossBrunton/intel-llvm that referenced this pull request Apr 15, 2025
Change intel#17095 introduced a failure in the logger_validation test due to
an incorrect merging of the default and "current" log level. This
change reverts to the old behavior where an invalid `output` value causes
the logger to use the default `level`.
martygrant pushed a commit that referenced this pull request Apr 16, 2025
Change #17095 introduced a failure in the logger_validation test due to
an incorrect merging of the default and "current" log level. This
change reverts to the old behavior where an invalid `output` value
causes
the logger to use the default `level`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants