Skip to content

[hipDNN][ALMIOPEN-917] CPU Matmul reference implementation#3965

Merged
a-sidorova merged 12 commits intoROCm:developfrom
a-sidorova:feature/hipdnn/matmul/cpu_ref
Jan 22, 2026
Merged

[hipDNN][ALMIOPEN-917] CPU Matmul reference implementation#3965
a-sidorova merged 12 commits intoROCm:developfrom
a-sidorova:feature/hipdnn/matmul/cpu_ref

Conversation

@a-sidorova
Copy link
Copy Markdown
Contributor

Motivation

The CPU Matmul reference is needed for quick signal integration tests within the ticket ALMIOPEN-917.

Technical Details

Implemented the class CpuFpReferenceMatmul with reference implementation, classes MatmulParams, MatmulPlan and MatmulPlanBuilder with MatmulSignatureKey for CPUGraphExecutor. Added test helpers for tensor initialization and matmul graph building.

Updated the documentation OperationSupport-ReferenceImpl.md

Test Plan

Added unit tests for each added class. Reference implementation is covered by functional tests.

Test Result

Tests are successfully passed

Submission Checklist

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 90.94650% with 22 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...tilities/cpu_graph_executor/MatmulSignatureKey.hpp 86.05% 7 Missing and 5 partials ⚠️
...hipdnn_test_sdk/utilities/CpuFpReferenceMatmul.hpp 94.06% 3 Missing and 3 partials ⚠️
...st_sdk/utilities/cpu_graph_executor/MatmulPlan.hpp 92.00% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #3965       +/-   ##
============================================
+ Coverage    65.41%   81.28%   +15.87%     
============================================
  Files          186      159       -27     
  Lines        27307    16124    -11183     
  Branches      3433     2022     -1411     
============================================
- Hits         17862    13106     -4756     
+ Misses        8200     2226     -5974     
+ Partials      1245      792      -453     
Flag Coverage Δ
hipBLASLt ?
hipDNN 81.28% <90.95%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...clude/hipdnn_test_sdk/utilities/TestTolerances.hpp 100.00% <100.00%> (ø)
...s/cpu_graph_executor/CpuReferenceGraphExecutor.hpp 85.07% <100.00%> (+0.46%) ⬆️
...ilities/cpu_graph_executor/PlanBuilderRegistry.hpp 100.00% <ø> (ø)
...es/cpu_graph_executor/PlanRegistrySignatureKey.hpp 100.00% <ø> (ø)
...st_sdk/utilities/cpu_graph_executor/MatmulPlan.hpp 92.00% <92.00%> (ø)
...hipdnn_test_sdk/utilities/CpuFpReferenceMatmul.hpp 94.06% <94.06%> (ø)
...tilities/cpu_graph_executor/MatmulSignatureKey.hpp 86.05% <86.05%> (ø)

... and 30 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
Copy link
Copy Markdown
Contributor

Codecov Report

❌ Patch coverage is 72.01646% with 68 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...hipdnn_test_sdk/utilities/CpuFpReferenceMatmul.hpp 61.39% 31 Missing and 8 partials ⚠️
...st_sdk/utilities/cpu_graph_executor/MatmulPlan.hpp 66.00% 9 Missing and 8 partials ⚠️
...tilities/cpu_graph_executor/MatmulSignatureKey.hpp 86.05% 7 Missing and 5 partials ⚠️
Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #3965       +/-   ##
============================================
+ Coverage    55.17%   80.55%   +25.38%     
============================================
  Files           14      159      +145     
  Lines         4035    16111    +12076     
  Branches       618     2012     +1394     
============================================
+ Hits          2226    12977    +10751     
- Misses        1597     2344      +747     
- Partials       212      790      +578     

Flag Coverage Δ
hipDNN 80.55% <72.02%> (?)
hipFFT ?
Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...clude/hipdnn_test_sdk/utilities/TestTolerances.hpp 100.00% <100.00%> (ø)
...s/cpu_graph_executor/CpuReferenceGraphExecutor.hpp 85.07% <100.00%> (ø)
...ilities/cpu_graph_executor/PlanBuilderRegistry.hpp 100.00% <ø> (ø)
...es/cpu_graph_executor/PlanRegistrySignatureKey.hpp 100.00% <ø> (ø)
...tilities/cpu_graph_executor/MatmulSignatureKey.hpp 86.05% <86.05%> (ø)
...st_sdk/utilities/cpu_graph_executor/MatmulPlan.hpp 66.00% <66.00%> (ø)
...hipdnn_test_sdk/utilities/CpuFpReferenceMatmul.hpp 61.39% <61.39%> (ø)
... and 166 files with indirect coverage changes

🚀 New features to boost your workflow:

Looks like the coverage for this new ref impl has dropped a bit below target (72% vrs 80%).

I think these two files are the main cause:

I think a couple tests that check error conditions will resolve this, and get above the coverage target.
Sorry if this was mostly copied from existing reference implementations.
I think this is an area we don't have the best coverage currently, but ideally we can improve!

Copy link
Copy Markdown
Contributor

@adickin-amd adickin-amd left a comment

Choose a reason for hiding this comment

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

Overall looking really good. Just need to add the coverage Brian requested and 1 more test for the ref impl covering a 4d broadcast.

@a-sidorova
Copy link
Copy Markdown
Contributor Author

@BrianHarrisonAMD @adickin-amd hi guys! Thank you for the recommendations.

I have added new test cases for better test coverage:

Codecov Report

❌ Patch coverage is 90.94650% with 22 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...tilities/cpu_graph_executor/MatmulSignatureKey.hpp 86.05% 7 Missing and 5 partials ⚠️
...hipdnn_test_sdk/utilities/CpuFpReferenceMatmul.hpp 94.06% 3 Missing and 3 partials ⚠️
...st_sdk/utilities/cpu_graph_executor/MatmulPlan.hpp 92.00% 0 Missing and 4 partials ⚠️
Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #3965       +/-   ##
============================================
+ Coverage    55.17%   80.83%   +25.67%     
============================================
  Files           14      159      +145     
  Lines         4035    16090    +12055     
  Branches       618     2014     +1396     
============================================
+ Hits          2226    13006    +10780     
- Misses        1597     2305      +708     
- Partials       212      779      +567     

Flag Coverage Δ
hipDNN 80.83% <90.95%> (?)
hipFFT ?
Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...clude/hipdnn_test_sdk/utilities/TestTolerances.hpp 100.00% <100.00%> (ø)
...s/cpu_graph_executor/CpuReferenceGraphExecutor.hpp 85.07% <100.00%> (ø)
...ilities/cpu_graph_executor/PlanBuilderRegistry.hpp 100.00% <ø> (ø)
...es/cpu_graph_executor/PlanRegistrySignatureKey.hpp 100.00% <ø> (ø)
...st_sdk/utilities/cpu_graph_executor/MatmulPlan.hpp 92.00% <92.00%> (ø)
...hipdnn_test_sdk/utilities/CpuFpReferenceMatmul.hpp 94.06% <94.06%> (ø)
...tilities/cpu_graph_executor/MatmulSignatureKey.hpp 86.05% <86.05%> (ø)
... and 166 files with indirect coverage changes

🚀 New features to boost your workflow:

May I ask you to review one more time please?

@a-sidorova a-sidorova force-pushed the feature/hipdnn/matmul/cpu_ref branch from 7532603 to b4f03a1 Compare January 21, 2026 11:09
Copy link
Copy Markdown
Contributor

@adickin-amd adickin-amd left a comment

Choose a reason for hiding this comment

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

Thanks for adding those tests, LGTM!

@a-sidorova a-sidorova force-pushed the feature/hipdnn/matmul/cpu_ref branch from 4415f9b to 91cca24 Compare January 22, 2026 05:10
@a-sidorova a-sidorova enabled auto-merge (squash) January 22, 2026 05:26
@a-sidorova a-sidorova merged commit b6de351 into ROCm:develop Jan 22, 2026
45 of 49 checks passed
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.

5 participants