[hipDNN] Add reduction operation support to frontend#5489
Open
rsuderman wants to merge 8 commits intoROCm:developfrom
Open
[hipDNN] Add reduction operation support to frontend#5489rsuderman wants to merge 8 commits intoROCm:developfrom
rsuderman wants to merge 8 commits intoROCm:developfrom
Conversation
## Motivation hipDNN lacked support for reduction operations, which are needed for building composite ops (e.g., softmax, SDPA) and to match the cudnn-frontend API surface for portability. ## Technical Details - Add `reduction_attributes.fbs` schema with `ReductionMode` enum (ADD, MUL, MIN_OP, MAX_OP, AMAX, AVG, NORM1, NORM2, MUL_NO_ZEROS); MIN and MAX are renamed to MIN_OP/MAX_OP to avoid flatc name collisions with reserved identifiers, matching the PointwiseMode convention used elsewhere in the schema - Generate `reduction_attributes_generated.h` for both FlatBuffers versions (v24_12_23 and v25_9_23) and regenerate `graph_generated.h` to add `ReductionAttributes` as union member 17 - Add `ReductionAttributes` frontend class with `set_mode()`/`get_mode()` and `X`/`Y` tensor accessors; class and typedef names match cudnn-frontend's `Reduction_attributes` / `ReductionMode_t` - Add `ReductionNode`, `Graph::reduction()` method, JSON utilities, and deserialization case in `Graph::deserializeFromFlatBuffer()` - Wire `ReductionAttributes` into `NodeWrapper`, JSON `Graph.hpp`, and `TestJson` / `TestGraphSerialization` round-trip tests ## Test Plan - Run `ninja unit-check` and verify `TestJson` and `TestGraphSerializationRoundTrip.ReductionNode` pass - Confirm JSON and binary round-trip for a reduction graph Test Result - Not yet run (no GPU / build environment available at commit time) Signed-off-by: Rob Suderman <rob.suderman@gmail.com>
projects/hipdnn/test_sdk/include/hipdnn_test_sdk/utilities/FlatbufferGraphTestUtils.hpp
Outdated
Show resolved
Hide resolved
projects/hipdnn/frontend/include/hipdnn_frontend/attributes/ReductionAttributes.hpp
Show resolved
Hide resolved
projects/hipdnn/data_sdk/include/hipdnn_data_sdk/utilities/json/ReductionAttributes.hpp
Show resolved
Hide resolved
Contributor
|
Heads up when #5356 goes in there will be a slight change you will need to include. |
Codecov Report❌ Patch coverage is ❌ 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 #5489 +/- ##
===========================================
+ Coverage 67.27% 67.29% +0.02%
===========================================
Files 1844 1847 +3
Lines 284014 284289 +275
Branches 39839 39880 +41
===========================================
+ Hits 191044 191300 +256
- Misses 76512 76522 +10
- Partials 16458 16467 +9
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
… is_deterministic ## Motivation Address PR ROCm#5489 review comments to improve correctness, API parity with cudnn-frontend, and test coverage for the reduction operation stack. ## Technical Details - Add TestReductionAttributes.cpp and TestReductionNode.cpp covering mode getters/setters, tensor accessors, pack/fromFlatBuffer round-trips, pre_validate_node error cases, and dim validation - Add output-dimension validation to ReductionNode::pre_validate_node(): X and Y must have the same rank, each reduced Y dim must equal exactly 1, and at least one dim must be reduced - Add Graph::reduction(x, y, attrs) three-param overload for explicit partial-reduction output tensors - Add is_deterministic bool field to ReductionAttributes matching cudnn-frontend API (get_is_deterministic / set_is_deterministic); update schema, both generated headers (v24/v25), JSON utilities, and pack_attributes/fromFlatBuffer - Change JSON ReductionMode enum names to lowercase to match convention ## Test Plan - ninja unit-check (hipdnn_frontend_tests, hipdnn_data_sdk_tests) - Run TestReductionAttributes.* and TestReductionNode.* with --gtest_filter Test Result - Not yet run; changes are build-only at this stage Signed-off-by: Rob Suderman <rob.suderman@gmail.com>
Contributor
|
Heads up, the no-rtti changes landed, and graph changes are landing as well shortly that will require a couple updates. |
|
|
||
| namespace hipdnn_frontend::graph | ||
| { | ||
| class ReductionNode : public BaseNode<ReductionNode> |
Contributor
There was a problem hiding this comment.
This should set enum overload, and an enum added here.
Could wait until we add the lowering / lifting path I suppose, but also small to add now.
BrianHarrisonAMD
approved these changes
Mar 17, 2026
Contributor
BrianHarrisonAMD
left a comment
BrianHarrisonAMD
left a comment
There was a problem hiding this comment.
LGTM.
Make sure to update before merging since a fix for clang-tidy came in that was suppressing some checks, and it can break without flagging as a merge conflict.
| EXPECT_TRUE(restored.get_is_deterministic()); | ||
| } | ||
|
|
||
| TEST(TestReductionAttributes, Reduction_attributesTypedefExists) |
Contributor
There was a problem hiding this comment.
I think this is flagging the test name checker in the codecov tool:
TestReductionAttributes.Reduction_attributesTypedefExists [FAIL]
��� Test name 'TestReductionAttributes.Reduction_attributesTypedefExists' does not match expected PascalCase format 'TestSuite.TestCase' or 'Instance/TestSuite.TestCase' without special characters such as "_;:<>[]," etc..
- Add REDUCTION = 17 to NodeType enum with trailing comma on CUSTOM_OP - Pass NodeType::REDUCTION as template arg to BaseNode in ReductionNode - Rename test case Reduction_attributesTypedefExists to ReductionAttributesTypedefExists Signed-off-by: Rob Suderman <rob.suderman@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
hipDNN lacked support for reduction operations, which are needed for building composite ops (e.g., softmax, SDPA) and to match the cudnn-frontend API surface for portability.
Technical Details
reduction_attributes.fbsschema withReductionModeenum (ADD, MUL, MIN_OP, MAX_OP, AMAX, AVG, NORM1, NORM2, MUL_NO_ZEROS); MIN and MAX are renamed to MIN_OP/MAX_OP to avoid flatc name collisions with reserved identifiers, matching the PointwiseMode convention used elsewhere in the schemareduction_attributes_generated.hfor both FlatBuffers versions (v24_12_23 and v25_9_23) and regenerategraph_generated.hto addReductionAttributesas union member 17ReductionAttributesfrontend class withset_mode()/get_mode()andX/Ytensor accessors; class and typedef names match cudnn-frontend'sReduction_attributes/ReductionMode_tReductionNode,Graph::reduction()method, JSON utilities, and deserialization case inGraph::deserializeFromFlatBuffer()ReductionAttributesintoNodeWrapper, JSONGraph.hpp, andTestJson/TestGraphSerializationround-trip testsTest Plan
ninja unit-checkand verifyTestJsonandTestGraphSerializationRoundTrip.ReductionNodepassTest Result
Submission Checklist