[hipDNN] Add CustomOp descriptor lifting#5885
Merged
SamuelReeder merged 21 commits intodevelopfrom Mar 30, 2026
Merged
Conversation
Implement the lifting (deserialization) path for the CustomOp operation descriptor, enabling round-trip serialization through FlatBuffer and C-API routes. Backend changes: - Add fromNode() static factory to CustomOpOperationDescriptor for FlatBuffer-based reconstruction - Add HIPDNN_ATTR_OPERATION_NAME_EXT support to setAttribute/getAttribute - Add HIPDNN_ATTR_OPERATION_TYPE_EXT support to getAttribute - Uncomment CustomOp case in NodeFactory Frontend changes: - Create CustomOpUnpacker.hpp with helpers for tensor array unpacking, byte array unpacking, and full custom op operation unpacking - Wire unpack_from_descriptor() override in CustomOpNode - Uncomment CustomOp case in OperationUnpacker Tests: - Add TestCustomOpOperationFromNode unit tests (13 test cases) - Add TestGraphDescriptorCustomOp name preservation tests (3 test cases) - Add IntegrationCustomOpDescriptorLifting tests (4 test cases) - Uncomment TestOperationUnpacker CustomOp test case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Move unpackAndRegisterTensorArray and getDescriptorAttrByteArray from CustomOpUnpacker.hpp to DescriptorUnpackHelpers.hpp so they can be reused by other unpackers. Replace the inline peer_stats unpacking in BatchnormUnpacker.hpp with a call to the shared helper. Fix OperationNamePreservedInGraph test to actually test name preservation through the C-API path (setAttribute before finalize, then verify via serialized graph). Add OperationNameRoundTripThroughLifting test that verifies names survive FlatBuffer serialize/deserialize and NodeFactory rebuild. Remove fromNode-based tests from the graph descriptor test file (those belong in the fromNode test file). Add fromNode test variations for different tensor array sizes: FromNodeWithSingleInputAndOutput (1 in, 1 out) and FromNodeWithTwoOutputs (2 in, 2 out). Add SingleInputTwoOutputsRoundTrip integration test to verify the full lower/lift cycle with non-standard tensor counts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Add tensor dims/strides constants to CustomOpConstants.hpp and use them throughout the test fixture instead of inline values. Add SetsTensorReferencesWithFullValues test to verify dims, strides, and data_type on descriptor objects. Enhance GetAttributeWorksAfterFromNode to retrieve and verify input/output tensor descriptors via getAttribute using ScopedDescriptor and verifyTensorDescriptor utility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rray Cover both new helper functions that were moved to DescriptorUnpackHelpers.hpp. Six test cases for unpackAndRegisterTensorArray (new tensors, existing reuse, empty array, count failure, null descriptor, UID query failure) and five for getDescriptorAttrByteArray (success, not-supported, zero count, count failure, data failure). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (77.72%) 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 #5885 +/- ##
===========================================
+ Coverage 67.23% 67.52% +0.29%
===========================================
Files 1876 1877 +1
Lines 291079 291201 +122
Branches 41055 41070 +15
===========================================
+ Hits 195690 196620 +930
+ Misses 78592 77733 -859
- Partials 16797 16848 +51
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove the _inputDescs.empty() check from CustomOpOperationDescriptor::finalize() so that custom operations with zero inputs (output-only generators) are valid. Add backend tests for zero-input fromNode round-trip and zero-input graph descriptor serialization, plus an integration test verifying the full lower-lift cycle with zero inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…llowed Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
SamuelReeder
commented
Mar 26, 2026
Extract the repeated 7-call expectFullTensorMocksForDesc mock sequence into a single free function in an anonymous namespace. Both TestUnpackTensorAttributes and TestUnpackAndRegisterTensorArray now delegate to this shared helper, and the large inline duplicate in UnpackAndRegisterTensorNewTensor is gone. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…operation name Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
BrianHarrisonAMD
approved these changes
Mar 27, 2026
Contributor
BrianHarrisonAMD
left a comment
BrianHarrisonAMD
left a comment
There was a problem hiding this comment.
LGTM.
We should fix the couple issues noted before merge.
…-port tests, use toVec helper Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…iopen-1625/add-custom-op-lifting # Conflicts: # projects/hipdnn/backend/src/descriptors/NodeFactory.hpp
…d static-in-anon-namespace warning Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…iopen-1625/add-custom-op-lifting # Conflicts: # projects/hipdnn/frontend/include/hipdnn_frontend/detail/DescriptorUnpackHelpers.hpp
…by-value tests Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…skip behavior Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
SamuelReeder
deleted the
users/sareeder/almiopen-1625/add-custom-op-lifting
branch
vidyasagar-amd
pushed a commit
that referenced
this pull request
Apr 9, 2026
## Motivation Adds descriptor-based lifting support for the CustomOp operation, enabling reconstruction of frontend graph attributes from serialized FlatBuffer data. This completes the round-trip path: frontend graph → descriptor lowering → serialize → deserialize → descriptor lifting → frontend graph. ## Technical Details Added `fromNode()` static factory method to `CustomOpOperationDescriptor` that reconstructs descriptor state from FlatBuffer `NodeT` data, resolving tensor UIDs from the shared tensor map. Added `HIPDNN_ATTR_OPERATION_NAME_EXT` and `HIPDNN_ATTR_OPERATION_TYPE_EXT` attribute support to `setAttribute()`/`getAttribute()`. Created `CustomOpUnpacker.hpp` that unpacks tensor arrays, byte arrays (opaque buffer), compute data type, and operation name from the C-API descriptor. **Note:** updated the CustomOpOperationDescriptor to allow finalization with 0 inputs or outputs on the descriptor. This matches the frontend, but there was inconsistency in the backend. ## Test Plan - [x] 13 fromNode round-trip unit tests (lifecycle, tensor references, data fields, name preservation, missing tensor failures) - [x] 3 name preservation tests in `TestGraphDescriptorCustomOp` - [x] 4 integration lifting tests including `AutoAssignedUidsPreservedInRoundTrip` - [x] `TestOperationUnpacker` case for CustomOp - [x] Shared test constants used throughout ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.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
Adds descriptor-based lifting support for the CustomOp operation, enabling reconstruction of frontend graph attributes from serialized FlatBuffer data. This completes the round-trip path: frontend graph → descriptor lowering → serialize → deserialize → descriptor lifting → frontend graph.
Technical Details
Added
fromNode()static factory method toCustomOpOperationDescriptorthat reconstructs descriptor state from FlatBufferNodeTdata, resolving tensor UIDs from the shared tensor map. AddedHIPDNN_ATTR_OPERATION_NAME_EXTandHIPDNN_ATTR_OPERATION_TYPE_EXTattribute support tosetAttribute()/getAttribute(). CreatedCustomOpUnpacker.hppthat unpacks tensor arrays, byte arrays (opaque buffer), compute data type, and operation name from the C-API descriptor.Note: updated the CustomOpOperationDescriptor to allow finalization with 0 inputs or outputs on the descriptor. This matches the frontend, but there was inconsistency in the backend.
Test Plan
TestGraphDescriptorCustomOpAutoAssignedUidsPreservedInRoundTripTestOperationUnpackercase for CustomOpSubmission Checklist