[hipDNN] Logging clean-up and feedback follow-up from #4312

dnn-providers/hipblaslt-provider/CMakeLists.txt

@@ -110,16 +110,12 @@ add_library(
     HipblasltMatmulDesc.cpp
     HipblasltMatrixLayout.cpp
 )
-target_compile_definitions(hipblaslt_plugin_private PRIVATE)
 target_include_directories(hipblaslt_plugin_private PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 target_link_libraries(hipblaslt_plugin_private PUBLIC roc::hipblaslt hipdnn_data_sdk hipdnn_plugin_sdk hip::host)
 
-# Enable spdlog support via plugin_sdk helper function
-hipdnn_enable_spdlog(hipblaslt_plugin_private)
 target_compile_options(hipblaslt_plugin_private PRIVATE ${HIPDNN_WARNING_COMPILE_OPTIONS})
 
 add_library(hipblaslt_plugin SHARED HipblasltPluginPublic.cpp)
-target_compile_definitions(hipblaslt_plugin PRIVATE)
 target_link_libraries(hipblaslt_plugin PRIVATE hipblaslt_plugin_private)
 target_link_libraries(
     hipblaslt_plugin PRIVATE "-Xlinker --exclude-libs=ALL"

dnn-providers/hipblaslt-provider/HipblasltUtils.hpp

@@ -13,15 +13,15 @@
 #include <hipdnn_plugin_sdk/PluginLogging.hpp>
 #include <string>
 
-#define LOG_ON_HIPBLASLT_FAILURE(status)                                              \
-    do                                                                                \
-    {                                                                                 \
-        if(status != HIPBLAS_STATUS_SUCCESS)                                          \
-        {                                                                             \
-            HIPDNN_PLUGIN_LOG_ERROR(                                                  \
-                "hipBLASLt error occurred: {}",                                       \
-                hipblaslt_plugin::hipblaslt_utils::hipblas_status_to_string(status)); \
-        }                                                                             \
+#define LOG_ON_HIPBLASLT_FAILURE(status)                                                 \
+    do                                                                                   \
+    {                                                                                    \
+        if(status != HIPBLAS_STATUS_SUCCESS)                                             \
+        {                                                                                \
+            HIPDNN_PLUGIN_LOG_ERROR(                                                     \
+                "hipBLASLt error occurred: "                                             \
+                << hipblaslt_plugin::hipblaslt_utils::hipblas_status_to_string(status)); \
+        }                                                                                \
     } while(0)
 
 #define THROW_ON_HIPBLASLT_FAILURE(status)                                                     \

dnn-providers/hipblaslt-provider/HipdnnEnginePluginHandle.hpp

@@ -41,13 +41,13 @@ struct HipdnnEnginePluginHandle
     void storeEngineDetailsDetachedBuffer(const void* ptr,
                                           std::unique_ptr<flatbuffers::DetachedBuffer> buffer)
     {
-        HIPDNN_PLUGIN_LOG_INFO("Storing detached buffer at address: {:p}", ptr);
+        HIPDNN_PLUGIN_LOG_INFO("Storing detached buffer at address: " << ptr);
         _engineDetailsBuffers[ptr] = std::move(buffer);
     }
 
     void removeEngineDetailsDetachedBuffer(const void* ptr)
     {
-        HIPDNN_PLUGIN_LOG_INFO("Removing detached buffer at address: {:p}", ptr);
+        HIPDNN_PLUGIN_LOG_INFO("Removing detached buffer at address: " << ptr);
 
         auto it = _engineDetailsBuffers.find(ptr);
         if(it != _engineDetailsBuffers.end())
@@ -57,10 +57,11 @@ struct HipdnnEnginePluginHandle
         else
         {
             HIPDNN_PLUGIN_LOG_WARN(
-                "No detached buffer found at address: {:p}. Could not remove engine "
-                "details. Ensure you "
-                "are using the same hipdnn handle you used for engine details creation",
-                ptr);
+                "No detached buffer found at address: "
+                << ptr
+                << ". Could not remove engine "
+                   "details. Ensure you "
+                   "are using the same hipdnn handle you used for engine details creation");
         }
     }
 

dnn-providers/hipblaslt-provider/engines/plans/HipblasltMatmulPlanBuilder.cpp

@@ -119,7 +119,7 @@ void HipblasltMatmulPlanBuilder::buildPlan(
     validateGraphConfiguration(opGraph);
 
     const auto& nodeWrapper = opGraph.getNodeWrapper(0);
-    HIPDNN_PLUGIN_LOG_INFO("Building matmul plan for node: {}", nodeWrapper.name());
+    HIPDNN_PLUGIN_LOG_INFO("Building matmul plan for node: " << nodeWrapper.name());
 
     const auto& attr = nodeWrapper.attributesAs<hipdnn_data_sdk::data_objects::MatmulAttributes>();
     MatmulParams params(attr, opGraph.getTensorMap());

dnn-providers/hipblaslt-provider/integration_tests/CMakeLists.txt

@@ -25,9 +25,6 @@ target_link_libraries(
     hipblaslt_plugin_integration_tests PUBLIC GTest::gtest hip::host hipdnn_test_sdk hipdnn_plugin_sdk Threads::Threads
 )
 
-# Enable spdlog support for fmt-style logging
-hipdnn_enable_spdlog(hipblaslt_plugin_integration_tests)
-
 # Ensure that the hipblaslt_plugin is built before the hipblaslt_plugin_integration_tests, as the
 # integration tests depend on the hipBLASLt Provider Plugin
 add_dependencies(hipblaslt_plugin_integration_tests hipblaslt_plugin)

dnn-providers/hipblaslt-provider/integration_tests/IntegrationGraphVerificationHarness.hpp

@@ -89,7 +89,7 @@ class IntegrationGraphVerificationHarness : public ::testing::TestWithParam<Test
             << "At least one output tensor id must be specified for "
                "validation.";
 
-        HIPDNN_PLUGIN_LOG_INFO("Validating {} output tensors", outputTensorIds.size());
+        HIPDNN_PLUGIN_LOG_INFO("Validating " << outputTensorIds.size() << " output tensors");
 
         // Lazily register validators after graph execution since tensor Ids and types may be inferred during graph finalization
         for(const auto& registerValidator : _deferredValidators)

dnn-providers/hipblaslt-provider/integration_tests/main.cpp

@@ -4,7 +4,6 @@ SPDX-License-Identifier: MIT
 */
 
 #include <gtest/gtest.h>
-#include <spdlog/spdlog.h>
 
 #include <hipdnn_frontend.hpp>
 #include <hipdnn_test_sdk/utilities/HipErrorHandler.hpp>
@@ -19,7 +18,5 @@ int main(int argc, char** argv)
     testing::TestEventListeners& listeners = testing::UnitTest::GetInstance()->listeners();
     listeners.Append(new hipdnn_test_sdk::utilities::HipErrorHandler);
 
-    auto result = RUN_ALL_TESTS();
-    spdlog::shutdown();
-    return result;
+    return RUN_ALL_TESTS();
 }

dnn-providers/hipblaslt-provider/tests/main.cpp

@@ -4,7 +4,6 @@ SPDX-License-Identifier: MIT
 */
 
 #include <gtest/gtest.h>
-#include <spdlog/spdlog.h>
 
 #include <hipdnn_plugin_sdk/PluginLogging.hpp>
 #include <hipdnn_test_sdk/utilities/HipErrorHandler.hpp>
@@ -13,14 +12,12 @@ SPDX-License-Identifier: MIT
 int main(int argc, char** argv)
 {
     ::testing::InitGoogleTest(&argc, argv);
-    hipdnn::logging::initializeCallbackLogging("hipblaslt_plugin_tests",
-                                               hipdnn_test_sdk::utilities::testLoggingCallback);
+    hipdnn_plugin_sdk::logging::initializeCallbackLogging(
+        "hipblaslt_plugin_tests", hipdnn_test_sdk::utilities::testLoggingCallback);
 
     // Register HipErrorHandler to check and clear HIP errors after each test
     testing::TestEventListeners& listeners = testing::UnitTest::GetInstance()->listeners();
     listeners.Append(new hipdnn_test_sdk::utilities::HipErrorHandler);
 
-    auto result = RUN_ALL_TESTS();
-    spdlog::shutdown();
-    return result;
+    return RUN_ALL_TESTS();
 }

dnn-providers/miopen-provider/CMakeLists.txt

@@ -132,12 +132,9 @@ add_library(
     MiopenPlugin.cpp
     MiopenPluginPublic.cpp
 )
-target_compile_definitions(miopen_plugin_impl PRIVATE)
 target_include_directories(miopen_plugin_impl PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 target_link_libraries(miopen_plugin_impl PUBLIC MIOpen hipdnn_data_sdk hipdnn_plugin_sdk)
 
-# Enable spdlog support via plugin_sdk helper function
-hipdnn_enable_spdlog(miopen_plugin_impl)
 target_compile_options(miopen_plugin_impl PRIVATE ${HIPDNN_WARNING_COMPILE_OPTIONS})
 set_target_properties(miopen_plugin_impl PROPERTIES
     CXX_VISIBILITY_PRESET hidden
@@ -150,16 +147,14 @@ endif()
 
 # Static library for unit tests to link against (provides access to internal implementation)
 add_library(miopen_plugin_private STATIC $<TARGET_OBJECTS:miopen_plugin_impl>)
-target_compile_definitions(miopen_plugin_private PRIVATE)
 target_include_directories(miopen_plugin_private PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
-# Link to miopen_plugin_impl to inherit its PUBLIC properties (including spdlog setup)
+# Link to miopen_plugin_impl to inherit its PUBLIC properties
 target_link_libraries(miopen_plugin_private PUBLIC MIOpen hipdnn_data_sdk hipdnn_plugin_sdk miopen_plugin_impl)
 # Backwards compatibility alias
 add_library(miopen_legacy_plugin_private ALIAS miopen_plugin_private)
 
 # miopen_legacy_plugin - REAL target for current TheRock compatibility
 add_library(miopen_legacy_plugin SHARED $<TARGET_OBJECTS:miopen_plugin_impl>)
-target_compile_definitions(miopen_legacy_plugin PRIVATE)
 target_link_libraries(miopen_legacy_plugin PRIVATE MIOpen hipdnn_data_sdk hipdnn_plugin_sdk)
 target_link_libraries(miopen_legacy_plugin PRIVATE "-Xlinker --exclude-libs=ALL")
 target_compile_options(miopen_legacy_plugin PRIVATE ${HIPDNN_WARNING_COMPILE_OPTIONS})
@@ -176,7 +171,6 @@ set_target_properties(
 
 # miopen_plugin - REAL target for updated TheRock
 add_library(miopen_plugin SHARED $<TARGET_OBJECTS:miopen_plugin_impl>)
-target_compile_definitions(miopen_plugin PRIVATE)
 target_link_libraries(miopen_plugin PRIVATE MIOpen hipdnn_data_sdk hipdnn_plugin_sdk)
 target_link_libraries(miopen_plugin PRIVATE "-Xlinker --exclude-libs=ALL")
 target_compile_options(miopen_plugin PRIVATE ${HIPDNN_WARNING_COMPILE_OPTIONS})

dnn-providers/miopen-provider/HipdnnEnginePluginHandle.hpp

@@ -42,13 +42,13 @@ struct HipdnnEnginePluginHandle
     void storeEngineDetailsDetachedBuffer(const void* ptr,
                                           std::unique_ptr<flatbuffers::DetachedBuffer> buffer)
     {
-        HIPDNN_PLUGIN_LOG_INFO("Storing detached buffer at address: {:p}", ptr);
+        HIPDNN_PLUGIN_LOG_INFO("Storing detached buffer at address: " << ptr);
         _engineDetailsBuffers[ptr] = std::move(buffer);
     }
 
     void removeEngineDetailsDetachedBuffer(const void* ptr)
     {
-        HIPDNN_PLUGIN_LOG_INFO("Removing detached buffer at address: {:p}", ptr);
+        HIPDNN_PLUGIN_LOG_INFO("Removing detached buffer at address: " << ptr);
 
         auto it = _engineDetailsBuffers.find(ptr);
         if(it != _engineDetailsBuffers.end())
@@ -58,10 +58,11 @@ struct HipdnnEnginePluginHandle
         else
         {
             HIPDNN_PLUGIN_LOG_WARN(
-                "No detached buffer found at address: {:p}. Could not remove engine "
-                "details. Ensure you "
-                "are using the same hipdnn handle you used for engine details creation",
-                ptr);
+                "No detached buffer found at address: "
+                << ptr
+                << ". Could not remove engine "
+                   "details. Ensure you "
+                   "are using the same hipdnn handle you used for engine details creation");
         }
     }