Skip to content

[hipdnn] Add explicit cmake dependency on fmtlib#4889

Merged
SamuelReeder merged 2 commits intodevelopfrom
users/sareeder/hipdnn-fmt-dependency
Feb 26, 2026
Merged

[hipdnn] Add explicit cmake dependency on fmtlib#4889
SamuelReeder merged 2 commits intodevelopfrom
users/sareeder/hipdnn-fmt-dependency

Conversation

@SamuelReeder
Copy link
Contributor

@SamuelReeder SamuelReeder commented Feb 25, 2026

Summary

  • Links fmt::fmt in the backend when external fmt is used (not transitive through spdlog)
  • The backend uses fmtlib (via spdlog) but was relying on transitive header-only includes rather than proper cmake linkage

Based on work in #4569 from @rsuderman.

Test plan

  • hipDNN cmake configuration succeeds
  • hipDNN build succeeds (all 239 targets)
  • All 7 core test suites pass (data_sdk, backend, frontend, test_sdk, plugin_sdk, public_backend, public_frontend)
  • hipDNN samples build and pass with this install
  • hipDNN install tests

🤖 Generated with Claude Code

The hipdnn backend uses fmtlib (via spdlog) but was missing an explicit
cmake dependency and link for fmt::fmt. Add fmt to the hipdnn dependency
system (hipdnn_add_dependency) so it can be found on the system or
fetched via FetchContent if unavailable, and link fmt::fmt to
hipdnn_backend_private.

Co-Authored-By: Rob Suderman <rob.suderman@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@SamuelReeder SamuelReeder force-pushed the users/sareeder/hipdnn-fmt-dependency branch from f0dccd5 to 86b3baf Compare February 25, 2026 18:27
hipdnn_enable_spdlog() uses an include-only approach (no linking) to
avoid inheriting spdlog compile options incompatible with clang++ on
Windows. However, this also drops the transitive link to fmt::fmt that
spdlog normally provides. When spdlog is built with SPDLOG_FMT_EXTERNAL
and fmt is a compiled static library (not header-only), the missing link
causes linker errors.

Add an explicit target_link_libraries for fmt::fmt in the specific case
where spdlog uses external compiled fmt. This does not affect the
bundled-fmt or header-only-fmt paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ 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    #4889      +/-   ##
===========================================
- Coverage    65.94%   65.94%   -0.00%     
===========================================
  Files         1718     1718              
  Lines       267197   267197              
  Branches     37045    37045              
===========================================
- Hits        176202   176201       -1     
  Misses       75442    75442              
- Partials     15553    15554       +1     
Flag Coverage Δ *Carryforward flag
hipBLAS 90.67% <ø> (ø) Carriedforward from bb5a4dd
hipBLASLt 43.55% <ø> (ø) Carriedforward from bb5a4dd
hipCUB 82.38% <ø> (ø) Carriedforward from bb5a4dd
hipDNN 80.86% <ø> (-0.01%) ⬇️
hipFFT 55.93% <ø> (ø) Carriedforward from bb5a4dd
hipRAND 76.12% <ø> (ø) Carriedforward from bb5a4dd
hipSOLVER 68.81% <ø> (ø) Carriedforward from bb5a4dd
hipSPARSE 84.70% <ø> (ø) Carriedforward from bb5a4dd
rocBLAS 47.97% <ø> (ø) Carriedforward from bb5a4dd
rocFFT 52.91% <ø> (ø) Carriedforward from bb5a4dd
rocRAND 57.06% <ø> (ø) Carriedforward from bb5a4dd
rocSOLVER 76.83% <ø> (ø) Carriedforward from bb5a4dd
rocSPARSE 71.53% <ø> (ø) Carriedforward from bb5a4dd

*This pull request uses carry forward flags. Click here to find out more.
see 1 file 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.

Copy link
Contributor

@BrianHarrisonAMD BrianHarrisonAMD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@SamuelReeder SamuelReeder marked this pull request as ready for review February 25, 2026 22:48
@SamuelReeder SamuelReeder requested a review from a team as a code owner February 25, 2026 22:48
@SamuelReeder SamuelReeder enabled auto-merge (squash) February 25, 2026 22:48
@SamuelReeder SamuelReeder merged commit 9cdbc5c into develop Feb 26, 2026
28 of 30 checks passed
@SamuelReeder SamuelReeder deleted the users/sareeder/hipdnn-fmt-dependency branch February 26, 2026 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants