diff --git a/dnn-providers/miopen-provider/.cursor/rules/hipdnn-architecture.mdc b/dnn-providers/miopen-provider/.cursor/rules/hipdnn-architecture.mdc index d45c278868d..3e01135241d 100644 --- a/dnn-providers/miopen-provider/.cursor/rules/hipdnn-architecture.mdc +++ b/dnn-providers/miopen-provider/.cursor/rules/hipdnn-architecture.mdc @@ -37,7 +37,7 @@ FlatBuffer Graph → Engine Selection → Plan Creation → GPU Execution - **Plugin uses**: Plugin SDK (header-only), Data SDK (header-only), MIOpen library - **Plugin SDK uses**: Data SDK -- **Data SDK uses**: FlatBuffers, spdlog +- **Data SDK uses**: FlatBuffers - **No direct dependencies** on hipDNN Backend or Frontend ## Key Classes diff --git a/projects/hipdnn/backend/src/CMakeLists.txt b/projects/hipdnn/backend/src/CMakeLists.txt index 423694a2408..565e12c844e 100644 --- a/projects/hipdnn/backend/src/CMakeLists.txt +++ b/projects/hipdnn/backend/src/CMakeLists.txt @@ -44,6 +44,7 @@ target_link_libraries(hipdnn_backend_private PRIVATE ${CMAKE_DL_LIBS}) target_compile_definitions(hipdnn_backend_private PRIVATE HIPDNN_BACKEND_COMPILATION) # Enable spdlog support via shared cmake function +hipdnn_add_dependency(spdlog VERSION ${HIPDNN_SPDLOG_VERSION}) hipdnn_enable_spdlog(hipdnn_backend_private) clang_tidy_check(hipdnn_backend_private) diff --git a/projects/hipdnn/cmake/Spdlog.cmake b/projects/hipdnn/cmake/Spdlog.cmake index 05a5c324d08..ee79b3864dc 100644 --- a/projects/hipdnn/cmake/Spdlog.cmake +++ b/projects/hipdnn/cmake/Spdlog.cmake @@ -1,7 +1,7 @@ # Copyright © Advanced Micro Devices, Inc., or its affiliates. # SPDX-License-Identifier: MIT -# Shared spdlog/fmt configuration for hipDNN components (backend, plugins, etc.) +# Shared spdlog/fmt configuration for hipDNN components # This module provides a unified function to enable spdlog support for any target. # Function to enable spdlog support for a target @@ -14,7 +14,7 @@ # - Finds spdlog if not already available # - Adds spdlog include directories (header-only, no linking to avoid compile option inheritance) # - Configures fmt (external or bundled) -# - Adds required compile definitions (HIPDNN_PLUGIN_USE_SPDLOG, FMT_HEADER_ONLY, etc.) +# - Adds required compile definitions (FMT_HEADER_ONLY, etc.) # # Note: We use hipdnn_add_dependency_includes() instead of target_link_libraries() to avoid # inheriting compile options like /Zc:__cplusplus from spdlog which are incompatible with @@ -53,7 +53,7 @@ function(hipdnn_enable_spdlog TARGET_NAME) # Use include-only approach to avoid inheriting compile options (e.g., /Zc:__cplusplus) # that are incompatible with clang++ on Windows hipdnn_add_dependency_includes(${TARGET_NAME} ${_spdlog_target} - COMPILE_DEFINITIONS HIPDNN_PLUGIN_USE_SPDLOG FMT_HEADER_ONLY) + COMPILE_DEFINITIONS FMT_HEADER_ONLY) # Handle external fmt configuration # Only add SPDLOG_FMT_EXTERNAL if spdlog was actually built with it. diff --git a/projects/hipdnn/data_sdk/CMakeLists.txt b/projects/hipdnn/data_sdk/CMakeLists.txt index be1e76b92e6..dccc8a71aa8 100644 --- a/projects/hipdnn/data_sdk/CMakeLists.txt +++ b/projects/hipdnn/data_sdk/CMakeLists.txt @@ -10,10 +10,8 @@ set(HIP_DNN_DATA_SDK_INSTALL_INCLUDE_DIR "${CMAKE_INSTALL_INCLUDEDIR}/hipdnn/dat set(HIP_DNN_DATA_SDK_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/include") hipdnn_add_dependency(flatbuffers VERSION ${HIPDNN_FLATBUFFERS_VERSION}) -hipdnn_add_dependency(spdlog VERSION ${HIPDNN_SPDLOG_VERSION}) hipdnn_add_dependency(nlohmann_json VERSION ${HIPDNN_NLOHMANN_JSON_VERSION}) -find_package(fmt ${HIPDNN_FMTLIB_VERSION} QUIET) find_package(hip REQUIRED) # For dependencies we are going to check if they weren't imported, and then install it along with @@ -85,12 +83,6 @@ if(TARGET FlatBuffers) ) endif() -# Process spdlog dependency -_hipdnn_data_sdk_process_dependency( - spdlog_header_only spdlog::spdlog_header_only INCLUDE_DIR ${HIP_DNN_SPDLOG_INCLUDE_DIR} - INCLUDE_SUBDIR spdlog -) - # Process nlohmann_json dependency _hipdnn_data_sdk_process_dependency( nlohmann_json nlohmann_json::nlohmann_json INCLUDE_DIR ${HIP_DNN_NLOHMANN_JSON_INCLUDE_DIR} diff --git a/projects/hipdnn/data_sdk/include/hipdnn_data_sdk/logging/Logger.hpp b/projects/hipdnn/data_sdk/include/hipdnn_data_sdk/logging/Logger.hpp index b3569961ce4..c196255bd6a 100644 --- a/projects/hipdnn/data_sdk/include/hipdnn_data_sdk/logging/Logger.hpp +++ b/projects/hipdnn/data_sdk/include/hipdnn_data_sdk/logging/Logger.hpp @@ -24,7 +24,7 @@ // // Other components should use their scoped macros: // - Frontend: HIPDNN_FE_LOG_* (auto-inits, uses "hipdnn_frontend") -// - Plugins: HIPDNN_PLUGIN_LOG_* (dual-mode: spdlog or stream) +// - Plugins: HIPDNN_PLUGIN_LOG_* // - Backend: Has its own logging implementation namespace hipdnn_data_sdk::logging diff --git a/projects/hipdnn/docs/Design.md b/projects/hipdnn/docs/Design.md index 1cc658841f3..f6f3ec4b51c 100644 --- a/projects/hipdnn/docs/Design.md +++ b/projects/hipdnn/docs/Design.md @@ -59,7 +59,7 @@ hipDNN provides three header-only SDK libraries that serve as the foundation for The Data SDK contains FlatBuffers schemas and data structures for graph representation. -- **Dependencies**: FlatBuffers and spdlog +- **Dependencies**: FlatBuffers - **Purpose**: Provides data structures and serialization for graphs, tensors, and configurations - **Expected Usage**: Consumed by Frontend, Backend, and Plugins for graph data handling - **Core Functionality**: diff --git a/projects/hipdnn/plugin_sdk/cmake/hipdnn_plugin_sdkConfig.cmake.in b/projects/hipdnn/plugin_sdk/cmake/hipdnn_plugin_sdkConfig.cmake.in index e02a96471b3..bf2afe5989a 100644 --- a/projects/hipdnn/plugin_sdk/cmake/hipdnn_plugin_sdkConfig.cmake.in +++ b/projects/hipdnn/plugin_sdk/cmake/hipdnn_plugin_sdkConfig.cmake.in @@ -6,112 +6,6 @@ include(CMakeFindDependencyMacro) find_dependency(hipdnn_data_sdk @HIPDNN_PLUGIN_SDK_DATA_SDK_MIN_VERSION@ CONFIG REQUIRED) -# Helper function to add include directories and optional compile definitions from dependency targets -# Used for header-only consumption of 3rd party dependencies -# Automatically detects target type and uses appropriate visibility (INTERFACE for interface libs, PUBLIC for others) -function(hipdnn_add_dependency_includes TARGET_NAME HEADER_LIB_TARGET_NAME) - set(options "") - set(oneValueArgs "") - set(multiValueArgs COMPILE_DEFINITIONS) - cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) - - if(NOT TARGET ${TARGET_NAME}) - message(FATAL_ERROR "hipdnn_add_dependency_includes: Target '${TARGET_NAME}' does not exist") - return() - endif() - - if(NOT TARGET ${HEADER_LIB_TARGET_NAME}) - message(FATAL_ERROR "hipdnn_add_dependency_includes: Header library target '${HEADER_LIB_TARGET_NAME}' does not exist") - return() - endif() - - # Determine visibility based on target type - get_target_property(_target_type ${TARGET_NAME} TYPE) - if(_target_type STREQUAL "INTERFACE_LIBRARY") - set(_visibility INTERFACE) - else() - set(_visibility PUBLIC) - endif() - - get_target_property(_dep_includes ${HEADER_LIB_TARGET_NAME} INTERFACE_INCLUDE_DIRECTORIES) - if(_dep_includes) - message(VERBOSE "${TARGET_NAME} adding includes from ${HEADER_LIB_TARGET_NAME}: ${_dep_includes}") - target_include_directories(${TARGET_NAME} SYSTEM ${_visibility} ${_dep_includes}) - endif() - - if(ARG_COMPILE_DEFINITIONS) - target_compile_definitions(${TARGET_NAME} ${_visibility} ${ARG_COMPILE_DEFINITIONS}) - endif() -endfunction() - -# Function to enable spdlog support for a target -# Call this after your target is created - spdlog will be found automatically if not already available -# Example usage: -# hipdnn_enable_spdlog(my_plugin_target) -# -# Note: We use hipdnn_add_dependency_includes() instead of target_link_libraries() to avoid -# inheriting compile options like /Zc:__cplusplus from spdlog which are incompatible with -# clang++ on Windows. -function(hipdnn_enable_spdlog TARGET_NAME) - # Try to find spdlog if not already available - if(NOT TARGET spdlog::spdlog_header_only AND NOT TARGET spdlog_header_only) - find_package(spdlog QUIET) - endif() - - # Handle locally fetched spdlog (creates alias if needed) - if(TARGET spdlog_header_only AND NOT TARGET spdlog::spdlog_header_only) - add_library(spdlog::spdlog_header_only ALIAS spdlog_header_only) - endif() - - # Determine which spdlog target to use - if(TARGET spdlog::spdlog_header_only) - set(_spdlog_target spdlog::spdlog_header_only) - elseif(TARGET spdlog_header_only) - set(_spdlog_target spdlog_header_only) - else() - message(FATAL_ERROR "hipdnn_enable_spdlog: spdlog not found. " - "Ensure spdlog is installed or available via CMAKE_PREFIX_PATH.") - endif() - - # Check if spdlog was built with external fmt by inspecting its compile definitions - get_target_property(_spdlog_defs ${_spdlog_target} INTERFACE_COMPILE_DEFINITIONS) - set(_spdlog_uses_external_fmt FALSE) - if(_spdlog_defs) - if("SPDLOG_FMT_EXTERNAL" IN_LIST _spdlog_defs) - set(_spdlog_uses_external_fmt TRUE) - endif() - endif() - - # Use include-only approach to avoid inheriting compile options (e.g., /Zc:__cplusplus) - # that are incompatible with clang++ on Windows - hipdnn_add_dependency_includes(${TARGET_NAME} ${_spdlog_target} - COMPILE_DEFINITIONS HIPDNN_PLUGIN_USE_SPDLOG FMT_HEADER_ONLY) - - # Handle external fmt configuration - # Only add SPDLOG_FMT_EXTERNAL if spdlog was actually built with it. - # Do NOT add it just because fmt happens to be installed on the system, - # as this would cause spdlog to look for instead of its bundled headers. - if(_spdlog_uses_external_fmt) - find_package(fmt QUIET) - if(NOT fmt_FOUND) - message(FATAL_ERROR "hipdnn_enable_spdlog: spdlog requires external fmt but fmt was not found. " - "Ensure fmt is installed or available via CMAKE_PREFIX_PATH.") - endif() - if(TARGET fmt::fmt-header-only) - hipdnn_add_dependency_includes(${TARGET_NAME} fmt::fmt-header-only - COMPILE_DEFINITIONS SPDLOG_FMT_EXTERNAL) - elseif(TARGET fmt::fmt) - hipdnn_add_dependency_includes(${TARGET_NAME} fmt::fmt - COMPILE_DEFINITIONS SPDLOG_FMT_EXTERNAL) - else() - message(FATAL_ERROR "hipdnn_enable_spdlog: fmt package found but no usable target. " - "Expected fmt::fmt-header-only or fmt::fmt.") - endif() - endif() - - message(STATUS "hipdnn_enable_spdlog: Enabled spdlog for target ${TARGET_NAME}") -endfunction() - include("${CMAKE_CURRENT_LIST_DIR}/hipdnn_plugin_sdkTargets.cmake") check_required_components(hipdnn_plugin_sdk) diff --git a/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginHelpers.hpp b/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginHelpers.hpp index 896081a3aae..03c2c759a83 100644 --- a/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginHelpers.hpp +++ b/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginHelpers.hpp @@ -9,18 +9,10 @@ #include -// Logging macros for plugin API entry/exit -// Use fmt-style if spdlog is enabled, otherwise stream-style -#ifdef HIPDNN_PLUGIN_USE_SPDLOG -#define LOG_API_ENTRY(format, ...) \ - HIPDNN_PLUGIN_LOG_INFO("API called: [{}] " format, __func__, __VA_ARGS__) -#define LOG_API_SUCCESS(func_name, format, ...) \ - HIPDNN_PLUGIN_LOG_INFO("API success: [{}] " format, func_name, __VA_ARGS__) -#else +// Logging macros for plugin API entry/exit (stream-style) #define LOG_API_ENTRY(msg) HIPDNN_PLUGIN_LOG_INFO("API called: [" << __func__ << "] " << msg) #define LOG_API_SUCCESS(func_name, msg) \ HIPDNN_PLUGIN_LOG_INFO("API success: [" << func_name << "] " << msg) -#endif namespace hipdnn_plugin_sdk { diff --git a/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp b/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp index 934451eb64b..39120f09275 100644 --- a/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp +++ b/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp @@ -5,28 +5,15 @@ /** * @file PluginLogging.hpp - * @brief Plugin logging macros with dual-mode support + * @brief Plugin logging macros using stream-style syntax * * This header provides HIPDNN_PLUGIN_LOG_* macros for use in plugins. * - * Two modes are supported based on HIPDNN_PLUGIN_USE_SPDLOG: - * - * 1. HIPDNN_PLUGIN_USE_SPDLOG defined: - * - Uses spdlog/fmt style logging with format strings - * - Usage: HIPDNN_PLUGIN_LOG_INFO("Value: {}", value); - * - Requires linking against spdlog - * - Used by production plugins (miopen-provider, hipblaslt-provider) - * - * 2. HIPDNN_PLUGIN_USE_SPDLOG not defined (default): - * - Uses stream-style logging - * - Usage: HIPDNN_PLUGIN_LOG_INFO("Value: " << value); - * - No spdlog dependency required - * - Used by test plugins and lightweight integrations + * Usage: HIPDNN_PLUGIN_LOG_INFO("Value: " << value); * * The component name is stored at runtime when initializeCallbackLogging() is called. */ -// Always include the SDK logging infrastructure #include #include @@ -76,96 +63,6 @@ inline std::string getComponentName() } // namespace hipdnn_plugin_sdk::logging -#ifdef HIPDNN_PLUGIN_USE_SPDLOG -// ============================================================================ -// Spdlog-style Plugin Logging (HIPDNN_PLUGIN_LOG_*) -// ============================================================================ -// Usage: HIPDNN_PLUGIN_LOG_INFO("Value: {}", someValue); - -#include -#include -#include - -#include -#include -#include -#include -#include - -// Check if the global log level would allow this message before -// retrieving component name (which requires mutex lock + string allocation). -// Note: spdlog's should_log checks the logger's level, but we can short-circuit -// earlier by checking the SDK log level. -#define _HIPDNN_SPDLOG_ACTION(spdlog_level, hipdnn_severity, ...) \ - do \ - { \ - if(::hipdnn_data_sdk::logging::isLogLevelEnabled(hipdnn_severity)) \ - { \ - auto componentName = ::hipdnn_plugin_sdk::logging::detail::getComponentName(); \ - auto logger = spdlog::get(componentName); \ - if(logger && logger->should_log(spdlog_level)) \ - { \ - logger->log(spdlog_level, __VA_ARGS__); \ - } \ - } \ - } while(0) - -#define HIPDNN_PLUGIN_LOG_TRACE(...) \ - _HIPDNN_SPDLOG_ACTION(spdlog::level::level_enum::trace, HIPDNN_SEV_INFO, __VA_ARGS__) -#define HIPDNN_PLUGIN_LOG_INFO(...) \ - _HIPDNN_SPDLOG_ACTION(spdlog::level::level_enum::info, HIPDNN_SEV_INFO, __VA_ARGS__) -#define HIPDNN_PLUGIN_LOG_WARN(...) \ - _HIPDNN_SPDLOG_ACTION(spdlog::level::level_enum::warn, HIPDNN_SEV_WARN, __VA_ARGS__) -#define HIPDNN_PLUGIN_LOG_ERROR(...) \ - _HIPDNN_SPDLOG_ACTION(spdlog::level::level_enum::err, HIPDNN_SEV_ERROR, __VA_ARGS__) -#define HIPDNN_PLUGIN_LOG_FATAL(...) \ - _HIPDNN_SPDLOG_ACTION(spdlog::level::level_enum::critical, HIPDNN_SEV_FATAL, __VA_ARGS__) - -namespace hipdnn_plugin_sdk::logging -{ - -/** - * @brief Initialize spdlog-based callback logging for plugins - * - * Creates an async spdlog logger that forwards messages to the callback. - * The component name is stored and used by logging macros. - */ -// HIPDNN_HIDDEN ensures each shared object has its own copy of the static mutex -HIPDNN_HIDDEN inline void initializeCallbackLogging(const std::string& componentName, - hipdnnCallback_t callbackFunction) -{ - // Store the component name for use by macros - detail::setComponentName(componentName); - - try - { - static std::mutex s_callbackInitMutex; - std::lock_guard lock(s_callbackInitMutex); - - if(spdlog::get(componentName)) - { - return; - } - - if(!spdlog::thread_pool()) - { - spdlog::init_thread_pool(8192, 1); - } - - auto callbackLogger = hipdnn_plugin_sdk::logging::detail::createAsyncCallbackLoggerMt( - callbackFunction, componentName); - spdlog::register_logger(callbackLogger); - } - catch(const spdlog::spdlog_ex& ex) - { - std::cerr << "hipDNN SDK: Failed to initialize callback logger for component '" - << componentName << "'. Error: " << ex.what() << "\n"; - } -} - -} // namespace hipdnn_plugin_sdk::logging - -#else // ============================================================================ // Stream-style Plugin Logging (HIPDNN_PLUGIN_LOG_*) // ============================================================================ @@ -245,5 +142,3 @@ inline void initializeCallbackLogging(const std::string& componentName, } } // namespace hipdnn_plugin_sdk::logging - -#endif // HIPDNN_PLUGIN_USE_SPDLOG diff --git a/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/logging/CallbackSink.hpp b/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/logging/CallbackSink.hpp deleted file mode 100644 index 5a94ddf4062..00000000000 --- a/projects/hipdnn/plugin_sdk/include/hipdnn_plugin_sdk/logging/CallbackSink.hpp +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright © Advanced Micro Devices, Inc., or its affiliates. -// SPDX-License-Identifier: MIT - -#pragma once - -#include -#include -#include -#include -#include -#include -#include -#include - -namespace hipdnn_plugin_sdk::logging::detail -{ - -inline hipdnnSeverity_t spdlogToHipdnnSeverity(spdlog::level::level_enum level) -{ - switch(level) - { - case spdlog::level::critical: - return HIPDNN_SEV_FATAL; - case spdlog::level::err: - return HIPDNN_SEV_ERROR; - case spdlog::level::warn: - return HIPDNN_SEV_WARN; - case spdlog::level::off: - return HIPDNN_SEV_OFF; - default: - return HIPDNN_SEV_INFO; - } -} - -// This warning is for cross-compiler compatibility, so it's safe to ignore since we only use Clang. -// Moreover, MSVC is likely the incompatible compiler here. -// NOLINTBEGIN(portability-template-virtual-member-function) -template -class CallbackSink final : public spdlog::sinks::base_sink -{ -public: - explicit CallbackSink(hipdnnCallback_t callback) - : _callbackFn{callback} - { - } - ~CallbackSink() override = default; - -protected: - void sink_it_(const spdlog::details::log_msg& msg) override - { - if(_callbackFn == nullptr) - { - return; - } - - spdlog::memory_buf_t formatted; - spdlog::sinks::base_sink::formatter_->format(msg, formatted); - std::string formattedStr(formatted.data(), formatted.size()); - - while(!formattedStr.empty() && (formattedStr.back() == '\n' || formattedStr.back() == '\r')) - { - formattedStr.pop_back(); - } - - hipdnnSeverity_t severity = spdlogToHipdnnSeverity(msg.level); - - _callbackFn(severity, formattedStr.c_str()); - } - - void flush_() override {} - -private: - hipdnnCallback_t _callbackFn; -}; -// NOLINTEND(portability-template-virtual-member-function) - -using CallbackSinkMt = CallbackSink; - -/** - * @brief Generate the spdlog pattern string for callback-based logging - * - * This pattern includes only the component name prefix in brackets for consistency - * with backend logging format. The backend adds timestamp, thread ID, and log level - * when receiving the callback message. - * - * @param componentName The name of the component - * @return The spdlog pattern string with bracketed component prefix - */ -inline std::string generatePatternString(const std::string& componentName) -{ - return "[" + componentName + "] %v"; -} - -inline std::shared_ptr createAsyncCallbackLoggerMt(hipdnnCallback_t callback, - const std::string& source) -{ - auto sink = std::make_shared(callback); - auto logger = std::make_shared( - source, sink, spdlog::thread_pool(), spdlog::async_overflow_policy::block); - logger->set_pattern(generatePatternString(source)); - logger->flush_on(spdlog::level::info); - return logger; -} - -template -inline std::shared_ptr createCallbackLoggerMt(hipdnnCallback_t callback, - const std::string& source) -{ - auto logger = Factory::template create(source, callback); - logger->set_pattern(generatePatternString(source)); - return logger; -} - -} // namespace hipdnn_plugin_sdk::logging::detail