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

BrianHarrisonAMD · 2026-02-09T16:45:59Z

Motivation

Address non-blocking feedback, and clean-up from #4312.

Technical Details

Summary of changes:

Fix improper logging namespaces in several places
Improve env handling for logging to be more forgiving (remove whitespace, and ignore capitalization)
Remove duplicated files / functions for logging across components
Swap to enum instead of string compares everywhere
Improve locking for shared state

Test Plan

Build & tests are working for hipDNN + provider components

Test Result

Build & tests are passing locally, and waiting on CI

… installed without spdlog

…from #4312

CMiservaAMD

LGTM.
Just holding-off on setting the PR to approved until I have a chance to see how difficult it might be to add changes in https://github.com/ROCm/rocm-libraries/compare/users/cmiserva/almiopen-1048/single-backend-spdlog?expand=1 to this PR.

codecov-commenter · 2026-02-09T18:01:13Z

Codecov Report

❌ Patch coverage is 91.57895% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
projects/hipdnn/backend/src/logging/Logging.hpp	88.57%	0 Missing and 4 partials ⚠️
projects/hipdnn/backend/src/logging/Logging.cpp	90.91%	1 Missing and 2 partials ⚠️
...in_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp	0.00%	1 Missing ⚠️

❌ Your project status has failed because the head coverage (76.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4423      +/-   ##
===========================================
- Coverage    65.93%   65.35%   -0.58%     
===========================================
  Files         1600     1581      -19     
  Lines       254848   242154   -12694     
  Branches     35899    33925    -1974     
===========================================
- Hits        168021   158250    -9771     
+ Misses       71603    69886    -1717     
+ Partials     15224    14018    -1206

Flag	Coverage Δ		*Carryforward flag
hipBLAS	`90.67% <ø> (ø)`		Carriedforward from 2c08d63
hipBLASLt	`43.62% <ø> (ø)`		Carriedforward from 2c08d63
hipDNN	`81.83% <91.58%> (+0.24%)`	⬆️
hipFFT	`56.68% <ø> (+0.42%)`	⬆️	Carriedforward from 2c08d63
hipRAND	`?`
hipSOLVER	`?`
hipSPARSE	`84.70% <ø> (ø)`		Carriedforward from 2c08d63
rocBLAS	`47.97% <ø> (ø)`		Carriedforward from 2c08d63
rocFFT	`48.57% <ø> (-3.64%)`	⬇️	Carriedforward from 2c08d63
rocSOLVER	`76.83% <ø> (ø)`		Carriedforward from 2c08d63
rocSPARSE	`71.53% <ø> (ø)`		Carriedforward from 2c08d63

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines	Coverage Δ
...a_sdk/include/hipdnn_data_sdk/logging/LogLevel.hpp	`95.71% <100.00%> (+20.30%)`	⬆️
...k/include/hipdnn_data_sdk/logging/LoggingUtils.hpp	`100.00% <100.00%> (ø)`
...k/include/hipdnn_data_sdk/utilities/StringUtil.hpp	`94.19% <100.00%> (+0.68%)`	⬆️
...in_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp	`0.00% <0.00%> (ø)`
projects/hipdnn/backend/src/logging/Logging.cpp	`82.54% <90.91%> (+14.97%)`	⬆️
projects/hipdnn/backend/src/logging/Logging.hpp	`86.11% <88.57%> (-6.20%)`	⬇️

... and 65 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

BrianHarrisonAMD · 2026-02-09T18:53:16Z

Codecov Report

❌ Patch coverage is 82.97872% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
projects/hipdnn/backend/src/logging/Logging.cpp 65.00% 4 Missing and 3 partials ⚠️
...in_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp 0.00% 1 Missing ⚠️
❌ Your project status has failed because the head coverage (76.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4423      +/-   ##
===========================================
+ Coverage    65.32%   65.33%   +0.01%     
===========================================
  Files         1579     1579              
  Lines       242137   242162      +25     
  Branches     33920    33922       +2     
===========================================
+ Hits        158163   158195      +32     
+ Misses       69952    69947       -5     
+ Partials     14022    14020       -2     
Flag Coverage Δ *Carryforward flag
hipBLAS 90.67% <ø> (ø) Carriedforward from f45bbd9
hipBLASLt 43.62% <ø> (ø) Carriedforward from f45bbd9
hipDNN 81.49% <82.98%> (+0.09%) ⬆️
hipFFT 56.68% <ø> (ø) Carriedforward from f45bbd9
hipSPARSE 84.70% <ø> (ø) Carriedforward from f45bbd9
rocBLAS 47.97% <ø> (ø) Carriedforward from f45bbd9
rocFFT 48.57% <ø> (ø) Carriedforward from f45bbd9
rocSOLVER 76.83% <ø> (ø) Carriedforward from f45bbd9
rocSPARSE 71.53% <ø> (-<0.01%) ⬇️ Carriedforward from f45bbd9
*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
.../hipdnn/backend/src/logging/ComponentFormatter.hpp 63.16% <ø> (ø)
projects/hipdnn/backend/src/logging/Logging.hpp 92.31% <ø> (ø)
...ojects/hipdnn/backend/src/logging/LoggingUtils.hpp 100.00% <ø> (ø)
...a_sdk/include/hipdnn_data_sdk/logging/LogLevel.hpp 95.71% <100.00%> (+20.30%) ⬆️
...k/include/hipdnn_data_sdk/logging/LoggingUtils.hpp 100.00% <100.00%> (ø)
...k/include/hipdnn_data_sdk/utilities/StringUtil.hpp 94.19% <100.00%> (+0.68%) ⬆️
...in_sdk/include/hipdnn_plugin_sdk/PluginLogging.hpp 0.00% <0.00%> (ø)
projects/hipdnn/backend/src/logging/Logging.cpp 68.21% <65.00%> (+0.64%) ⬆️
... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

The code files flagged are either covered by plugins, or are error handling from HIP errors that aren't covered by tests.

I think coverage looks good.

mousdahl-amd

LGTM

mousdahl-amd

LGTM

adickin-amd · 2026-02-09T19:13:41Z

PR Review Summary

This is a well-executed refactoring PR that enhances the logging infrastructure across hipDNN components. All changes are consistent, properly tested, and improve code quality.

Changes Summary

1. Namespace Cleanup

Corrected logging namespace references from hipdnn::logging to hipdnn_plugin_sdk::logging
Fixed backend logging namespace from hipdnn::backend::logging to hipdnn_backend::logging

2. Environment Variable Handling Improvements

Log level parsing is now case-insensitive (e.g., INFO, Info, info all accepted)
Leading and trailing whitespace is automatically trimmed from log level values
Added stringToSeverity() returning std::optional for explicit invalid input handling
Added stringToSeverityOrOff() convenience wrapper for backward compatibility

3. Code Consolidation

Removed duplicate CallbackSink.hpp from backend (functionality already exists in plugin_sdk)
Replaced string comparisons with enum comparisons for improved type safety and performance

4. Performance Optimization

Upgraded from std::mutex to std::shared_mutex for component name accessor, improving read concurrency

5. Test Coverage

Added comprehensive tests for the trim function
Added tests for both stringToSeverity() and stringToSeverityOrOff() functions
Validated case-insensitivity and whitespace handling behavior

Recommendation

✅ Approve - The implementation is clean, consistent, and well-tested. No issues requiring action were identified.

Generated by Claude Code

These empty calls served no purpose and were leftover from previous refactoring. Removing them cleans up the CMake files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…OCm/rocm-libraries into users/bharriso/logging-changes-cleanup

SamuelReeder

LGTM!

…nce. (#4428)

BrianHarrisonAMD · 2026-02-09T19:41:07Z

Auto-merge enabled.

Please disable if blocking comments are added!

…ng (#4440) ## Motivation Remove spdlog/fmt dependency from hipblaslt-provider. This change is part of removing spdlog/fmt from plugin_sdk. ## Technical Details Changes: - Convert logging to stream based style - Remove hipdnn_enable_spdlog() calls from CMakeLists.txt files - Remove spdlog includes and shutdown calls from test main.cpp files ## Test Plan Build and tests are working locally (CI not enabled for hipblaslt-provider yet). Logs are properly output (unchanged). ## Test Result Build and tests are continuing to pass. Logs are printing properly. --- ### Note Off the logging clean-up branch (PR #4423) that is about to be merged. This ensures we have the latest namespaces, and other clean-up changes for this work. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

) ## Motivation Remove spdlog/fmt dependency from miopen-provider. This change is part of removing spdlog/fmt from plugin_sdk. ## Technical Details Changes: - Convert logging to stream based style - Remove hipdnn_enable_spdlog() calls from CMakeLists.txt files - Remove spdlog includes and shutdown calls from test main.cpp files ## Test Plan Build and tests are working locally and in CI. Sanity check logging to ensure it's functioning same as before for samples, and tests. ## Test Result Build and tests are continuing to pass. Logs are printing properly. Waiting on CI signal. --- ### Note Off the logging clean-up branch (PR #4423) that is about to be merged. This ensures we have the latest namespaces, and other clean-up changes for this work. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

…ging-changes-cleanup

adickin-amd · 2026-02-11T15:38:24Z

Issuing gardener override, the windows CI issue is known and waiting for another full CI run could take another day leading to more postponed work.

BrianHarrisonAMD and others added 8 commits February 6, 2026 16:01

Add nolint ignores for false positives, and handle cases where fmt is…

70b2252

… installed without spdlog

Update installed cmake as well

5ebf688

Fix stable_sort warning

51e1b16

Swap to lint suppress since it's not compiler

9a1d21e

Remove stable sort for custom stable behavior

af61d69

Merge branch 'develop' into users/bharriso/fix-code-cov-target

cea0d40

Code review feedback clean-up

f45bbd9

Address clean-up and feedback for logging dependency removal changes …

c1ff904

…from #4312

BrianHarrisonAMD self-assigned this Feb 9, 2026

Merge branch 'develop' into users/bharriso/logging-changes-cleanup

9f3a295

github-actions bot added project: hipdnn project: miopen-provider project: hipblaslt-provider labels Feb 9, 2026

BrianHarrisonAMD marked this pull request as ready for review February 9, 2026 16:51

BrianHarrisonAMD requested a review from a team as a code owner February 9, 2026 16:51

assistant-librarian bot added the organization: ROCm label Feb 9, 2026

CMiservaAMD reviewed Feb 9, 2026

View reviewed changes

Merge branch 'develop' into users/bharriso/logging-changes-cleanup

796f173

mousdahl-amd approved these changes Feb 9, 2026

View reviewed changes

BrianHarrisonAMD and others added 2 commits February 9, 2026 14:14

Remove empty target_compile_definitions from provider CMakes

3626e63

These empty calls served no purpose and were leftover from previous refactoring. Removing them cleans up the CMake files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Merge branch 'users/bharriso/logging-changes-cleanup' of github.com:R…

83e4f3c

…OCm/rocm-libraries into users/bharriso/logging-changes-cleanup

SamuelReeder approved these changes Feb 9, 2026

View reviewed changes

CMiservaAMD and others added 2 commits February 9, 2026 12:38

[hipDNN] Consolidate backend logger into a single spdlog logger insta…

5f2dbee

…nce. (#4428)

Merge branch 'develop' into users/bharriso/logging-changes-cleanup

33c9bcf

BrianHarrisonAMD enabled auto-merge (squash) February 9, 2026 19:40

This was referenced Feb 9, 2026

[hipblaslt-provider] Convert hipblaslt-provider to stream-style logging #4440

Merged

[miopen-provider] Convert miopen-provider to stream-style logging #4442

Merged

BrianHarrisonAMD and others added 4 commits February 10, 2026 10:41

Merge branch 'develop' into users/bharriso/logging-changes-cleanup

2c08d63

Merge remote-tracking branch 'origin/develop' into users/bharriso/log…

3aa1bc2

…ging-changes-cleanup

adickin-amd disabled auto-merge February 11, 2026 15:39

adickin-amd merged commit 65f68fa into develop Feb 11, 2026
38 of 43 checks passed

adickin-amd deleted the users/bharriso/logging-changes-cleanup branch February 11, 2026 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

BrianHarrisonAMD commented Feb 9, 2026

Uh oh!

CMiservaAMD left a comment

Uh oh!

codecov-commenter commented Feb 9, 2026 •

edited

Loading

Uh oh!

BrianHarrisonAMD commented Feb 9, 2026

Codecov Report

Uh oh!

mousdahl-amd left a comment

Uh oh!

mousdahl-amd left a comment

Uh oh!

adickin-amd commented Feb 9, 2026

Uh oh!

SamuelReeder left a comment

Uh oh!

BrianHarrisonAMD commented Feb 9, 2026

Uh oh!

adickin-amd commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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

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

Conversation

BrianHarrisonAMD commented Feb 9, 2026

Motivation

Technical Details

Test Plan

Test Result

Uh oh!

CMiservaAMD left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

BrianHarrisonAMD commented Feb 9, 2026

Codecov Report

Uh oh!

mousdahl-amd left a comment

Choose a reason for hiding this comment

Uh oh!

mousdahl-amd left a comment

Choose a reason for hiding this comment

Uh oh!

adickin-amd commented Feb 9, 2026

PR Review Summary

Changes Summary

Recommendation

Uh oh!

SamuelReeder left a comment

Choose a reason for hiding this comment

Uh oh!

BrianHarrisonAMD commented Feb 9, 2026

Uh oh!

adickin-amd commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Feb 9, 2026 •

edited

Loading