Skip to content

[hipDNN] Pre-populate operation type enum and dispatch scaffolding for op lifting#5570

Open
SamuelReeder wants to merge 2 commits intodevelopfrom
users/sareeder/hipdnn-lifting-scaffolding
Open

[hipDNN] Pre-populate operation type enum and dispatch scaffolding for op lifting#5570
SamuelReeder wants to merge 2 commits intodevelopfrom
users/sareeder/hipdnn-lifting-scaffolding

Conversation

@SamuelReeder
Copy link
Contributor

@SamuelReeder SamuelReeder commented Mar 18, 2026

Motivation

Every lifting PR needs to add entries to three shared files: HipdnnOperationType.h (enum values), OperationUnpacker.hpp (dispatch cases), and NodeFactory.cpp/.hpp (factory dispatch). Without pre-populating these files, each lifting PR touches the same lines and causes merge conflicts.

This PR front-loads all necessary enum values and dispatch stubs so that each individual lifting PR only needs to implement unpack_from_descriptor() on its frontend node and fromNode() on its backend descriptor — no shared file conflicts.

Technical Details

HipdnnOperationType.h — Extended the hipdnnOperationType_t enum with 11 new values (7–17): BATCHNORM, POINTWISE, MATMUL, RMSNORM, LAYERNORM, SDPA_FORWARD, BLOCK_SCALE_QUANTIZE, SDPA_BACKWARD, BLOCK_SCALE_DEQUANTIZE, CUSTOM_OP, and REDUCTION.

OperationUnpacker.hpp — Added #include directives for all 17 frontend node headers and added active dispatch cases in createNodeForType() for all 15 operation types that have existing node classes (everything except REDUCTION). Each case instantiates the node with default-constructed attributes. Since INode::unpack_from_descriptor() returns "not implemented" by default, these cases are safe: they produce a clear error until each lifting PR overrides the method on its specific node class.

NodeFactory.cpp / NodeFactory.hpp — Added commented-out dispatch cases and #include directives for all 15 remaining operation descriptors. Each block is labeled with a comment to uncomment when fromNode() is implemented in the corresponding lifting PR.

BackendEnumStringUtils.hpp — Added #include "HipdnnOperationType.h", a new hipdnnGetOperationTypeString() function covering all 18 enum values, and two previously missing attribute name string cases: HIPDNN_ATTR_OPERATION_TYPE_EXT and HIPDNN_ATTR_OPERATION_NAME_EXT.

TestBackendEnumStringUtils.cpp — Added a GetOperationTypeString test covering all 18 enum values plus the unknown sentinel, tests for the two newly added attribute name strings, and an unknown-attribute sentinel test.

Note: DescriptorFactory, GraphUnpacker, Graph.hpp, CMakeLists, and test_sdk were already fully populated during the lowering PRs and are not conflict points for lifting.

Test Plan

  • Build hipdnn_backend and hipdnn_frontend
  • TestBackendEnumStringUtils passes (all 10 test cases)
  • TestOperationUnpacker passes (all 2 test cases)
  • Full hipdnn test suite passes (9 test executables)

Submission Checklist

@SamuelReeder SamuelReeder self-assigned this Mar 18, 2026
@SamuelReeder SamuelReeder changed the title [hipDNN] Pre-populate operation type enum and dispatch scaffolding for all lifting tickets [hipDNN] Pre-populate operation type enum and dispatch scaffolding for op lifting Mar 18, 2026
@SamuelReeder SamuelReeder force-pushed the users/sareeder/hipdnn-lifting-scaffolding branch from 5f54dfc to eb76fe5 Compare March 18, 2026 17:49
@SamuelReeder SamuelReeder marked this pull request as ready for review March 18, 2026 18:03
@SamuelReeder SamuelReeder requested a review from a team as a code owner March 18, 2026 18:03
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

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (77.21%) 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    #5570      +/-   ##
===========================================
+ Coverage    67.27%   67.27%   +0.01%     
===========================================
  Files         1844     1844              
  Lines       284014   284061      +47     
  Branches     39839    39860      +21     
===========================================
+ Hits        191044   191092      +48     
+ Misses       76512    76509       -3     
- Partials     16458    16460       +2     
Flag Coverage Δ *Carryforward flag
hipBLAS 90.67% <ø> (ø) Carriedforward from 02f625e
hipBLASLt 43.49% <ø> (ø) Carriedforward from 02f625e
hipCUB 82.21% <ø> (ø) Carriedforward from 02f625e
hipDNN 85.15% <100.00%> (+0.02%) ⬆️
hipFFT 55.59% <ø> (ø) Carriedforward from 02f625e
hipRAND 76.12% <ø> (ø) Carriedforward from 02f625e
hipSOLVER 68.81% <ø> (ø) Carriedforward from 02f625e
hipSPARSE 84.70% <ø> (ø) Carriedforward from 02f625e
rocBLAS 47.97% <ø> (ø) Carriedforward from 02f625e
rocFFT 53.24% <ø> (ø) Carriedforward from 02f625e
rocRAND 57.07% <ø> (ø) Carriedforward from 02f625e
rocSOLVER 77.21% <ø> (ø) Carriedforward from 02f625e
rocSPARSE 71.48% <ø> (ø) Carriedforward from 02f625e

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

Files with missing lines Coverage Δ
...ects/hipdnn/backend/src/BackendEnumStringUtils.hpp 100.00% <100.00%> (+0.27%) ⬆️
...cts/hipdnn/backend/src/descriptors/NodeFactory.cpp 100.00% <ø> (ø)
...clude/hipdnn_frontend/detail/OperationUnpacker.hpp 100.00% <ø> (ø)

... and 2 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.

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