Logger callback function sink
This PR implements https://github.com/oneapi-src/unified-runtime/issues/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, 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 new test suite LoggerWithCallbackSink has been added to test this new functionality.
intel/llvm PR: https://github.com/intel/llvm/pull/14422
If the idea is for the SYCL to hoist UR logs and print them using its own facility, wouldn't it make sense for urLoaderConfigSetLoggerCallback to override (or be in addition to) the default sink so that the users only ever need to interact with SYCL interfaces?
@pbalcer thanks for your comments! I'm not sure about this one maybe you can clarify?
If the idea is for the SYCL to hoist UR logs and print them using its own facility, wouldn't it make sense for
urLoaderConfigSetLoggerCallbackto override (or be in addition to) the default sink so that the users only ever need to interact with SYCL interfaces?
I think you mean so the user at the SYCL level doesn't need to interact with UR directly right? I'm not sure if that would be possible for this case as the callback would need to be passed through programmatically?
@pbalcer thanks for your comments! I'm not sure about this one maybe you can clarify?
If the idea is for the SYCL to hoist UR logs and print them using its own facility, wouldn't it make sense for
urLoaderConfigSetLoggerCallbackto override (or be in addition to) the default sink so that the users only ever need to interact with SYCL interfaces?I think you mean so the user at the SYCL level doesn't need to interact with UR directly right? I'm not sure if that would be possible for this case as the callback would need to be passed through programmatically?
The way you implemented it right now, for a SYCL end-user (I don't mean developer of SYCL runtime) to receive any sort of messages from the UR layer, they'd have to set UR_LOG_FOO=callback env variable. If the plan for the callback is to pass the responsibility for emitting logs to SYCL, it would make more sense to me if SYCL was also responsible for controlling the verbosity of those logs, using whatever methods that exist today. This way, end-users interact only with the highest level of abstraction (SYCL), not having to be aware of the underlying implementation (UR).
But for that to be possible, the set callback function needs to always work, independent of the UR-level log setting.
There is a workaround for warnings for now in this PR. and it is probably going to be merged first so could you delete it in this PR once merged as this should fix it properly @martygrant? Thanks!
Edit: After a rethought that would cause e2e test failures as this will be merged after the deleting PI PR. So, we should attach the deletion of the workaround with changing the adapters to use this new logger mechanism and not in this PR. I added an issue for more context on this here.
Unified Runtime -> intel/llvm Repo Move Notice
Information
The source code of Unified Runtime has been moved to intel/llvm under the unified-runtime top-level directory, all future development will now be carried out there. This was done in https://github.com/intel/llvm/pull/17043.
The code will be mirrored to oneapi-src/unified-runtime and the specification will continue to be hosted at oneapi-src.github.io/unified-runtime.
The contribution guide has been updated with new instructions for contributing to Unified Runtime.
PR Migration
All open PRs including this one will be labelled auto-close and shall be automatically closed after 30 days. To allow for some breathing space, this automation will not be enabled until next week (27/02/2025).
Should you wish to continue with your PR you will need to migrate it to intel/llvm. We have provided a script to help automate this process.
This is an automated comment.
Unified Runtime -> intel/llvm Repo Move Notice
Following on from the previous notice, we have now enabled workflows to automatically label and close PRs because the Unified Runtime source code has moved to intel/llvm.
This PR has now been marked with the auto-close label and will be automatically closed after 30 days.
Please review the previous notice for more information, including assistance with migrating your PR to intel/llvm.
Should there be a reason for this PR to remain open, manually remove the auto-close label.
This is an automated comment.
Automatic PR Closure Notice
Information
This PR has been closed automatically. It was marked with the auto-close label 30 days ago as part of the Unified Runtime source code migration to the intel/llvm repository - https://github.com/intel/llvm/pull/17043.
All Unified Runtime development should be done in intel/llvm, details can be found in the updated contribution guide. This repository will continue to exist as a mirror and will host the specification documentation.
Next Steps
Should you wish to re-open this PR it must be moved to intel/llvm. We have provided a script to help automate this process, otherwise no actions are required.
This is an automated comment.