Commit 84dc26e
authored
fix(hipdnn): fail fast on plan-only override execute without override enabled (#8881)
## Summary
The frontend override `execute()` overload fails fast when override args
are supplied to a graph that did not enable override shapes — but only
for full graphs. The plan-only
[deserialize_compiled_plan](https://github.com/ROCm/rocm-libraries/blob/f1230db5ef05ba6cf6413741a1b58b6a80473a8a/projects/hipdnn/frontend/include/hipdnn_frontend/Graph.hpp#L1571)
branch skipped the override-enabled check, so misuse on a plan-only
object was only caught later at the backend dispatch guard. This makes
the plan-only branch fail fast in the frontend, matching the full-graph
branch. Part of the overridable tensor shapes work ([RFC
0008](https://github.com/ROCm/rocm-libraries/blob/develop/projects/hipdnn/docs/rfcs/0008_OverridableTensorShapesDesign.md)).
JIRA ID : ALMIOPEN-2244
## Risk Assessment
Medium risk. Adds a read-only backend execution-plan attribute (API
addition) and changes the plan-only override-execute path to reject
earlier. The new rejection only fires for misuse the backend dispatch
guard already rejected authoritatively, so no legitimate path changes:
plan-only overrides on an override-enabled plan still execute, and
full-graph behavior is unchanged. No serialization format change. Blast
radius is ASIC-independent (see below), so passing PR CI is sufficient.
## Testing Summary
- Backend unit tests: new attribute round-trips through deserialize →
`getAttribute` for both flag states; existing plan-descriptor,
serialization, and enum-string suites unaffected.
- Frontend GPU integration tests: new plan-only fail-fast regression
(rejects with `INVALID_VALUE` and never dispatches to the provider);
full override-execute suite for no regressions.
- Commit hooks (clang-format, whitespace) on all changed files.
## Testing Checklist
- [x] hipDNN backend unit tests - `hipdnn_backend_tests
--gtest_filter='TestExecutionPlanDescriptor.*:*SerializationApi*:*BackendEnumStringUtils*:*IsOverrideShapeEnabledExt*'`
- ASICs: gfx90a - Status: Passed (74)
- [x] hipDNN frontend override integration tests -
`hipdnn_public_frontend_tests --gtest_filter='*Override*'` - ASICs:
gfx90a - Status: Passed (35)
- [x] Commit hooks - `pre-commit run --files <changed>` - Status: Passed
- [ ] PR CI - GitHub PR checks - Status: Pending
## Technical Changes
- Add read-only
`HIPDNN_ATTR_EXECUTION_PLAN_IS_OVERRIDE_SHAPE_ENABLED_EXT` with a
`getAttribute` case on `ExecutionPlanDescriptor` (mirrors
`ENGINE_GLOBAL_INDEX_EXT`; no serialization change) plus its enum-name
mapping. The attribute is required because the override-enabled flag,
while already preserved across the plan round-trip in the backend
(serialized into the plan flatbuffer, restored on deserialize), was
unreachable from the header-only frontend: `getAttribute` did not
surface it, `deserialize_compiled_plan` hard-coded it to `false`, and no
operation-graph or engine-config descriptor is reconstructed on the
plan-only path. The only no-new-attribute alternative — parsing the
backend plan flatbuffer in the frontend — would couple the frontend to
the backend wire format and version gate.
- Recover the flag in `deserialize_compiled_plan` via a new
`getExecutionPlanOverrideShapeEnabled` frontend helper instead of
forcing `false`.
- Reject override args in the plan-only branch of the override
`execute()` overload with `INVALID_VALUE`, short-circuiting before any
provider dispatch.
- Add backend tests for the attribute (both flag states) and enum-name
mapping, and a frontend GPU regression test for the plan-only fail-fast.1 parent 039eba8 commit 84dc26e
9 files changed
Lines changed: 293 additions & 19 deletions
File tree
- projects/hipdnn
- backend
- include
- src
- descriptors
- tests
- descriptors
- frontend
- include/hipdnn_frontend
- detail
- tests
- tests/frontend
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
163 | 163 | | |
164 | 164 | | |
165 | 165 | | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
166 | 170 | | |
167 | 171 | | |
168 | 172 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
322 | 322 | | |
323 | 323 | | |
324 | 324 | | |
| 325 | + | |
| 326 | + | |
325 | 327 | | |
326 | 328 | | |
327 | 329 | | |
| |||
Lines changed: 9 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
122 | 131 | | |
123 | 132 | | |
124 | 133 | | |
| |||
Lines changed: 5 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
144 | 149 | | |
145 | 150 | | |
146 | 151 | | |
| |||
Lines changed: 48 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
505 | 505 | | |
506 | 506 | | |
507 | 507 | | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
508 | 556 | | |
509 | 557 | | |
510 | 558 | | |
| |||
Lines changed: 33 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1485 | 1485 | | |
1486 | 1486 | | |
1487 | 1487 | | |
1488 | | - | |
| 1488 | + | |
| 1489 | + | |
| 1490 | + | |
| 1491 | + | |
| 1492 | + | |
1489 | 1493 | | |
1490 | 1494 | | |
1491 | 1495 | | |
| |||
1580 | 1584 | | |
1581 | 1585 | | |
1582 | 1586 | | |
1583 | | - | |
| 1587 | + | |
| 1588 | + | |
| 1589 | + | |
| 1590 | + | |
| 1591 | + | |
1584 | 1592 | | |
1585 | 1593 | | |
1586 | 1594 | | |
1587 | | - | |
| 1595 | + | |
| 1596 | + | |
| 1597 | + | |
| 1598 | + | |
| 1599 | + | |
| 1600 | + | |
1588 | 1601 | | |
1589 | 1602 | | |
1590 | 1603 | | |
| |||
2024 | 2037 | | |
2025 | 2038 | | |
2026 | 2039 | | |
2027 | | - | |
2028 | | - | |
| 2040 | + | |
| 2041 | + | |
| 2042 | + | |
| 2043 | + | |
2029 | 2044 | | |
2030 | 2045 | | |
2031 | 2046 | | |
| |||
2054 | 2069 | | |
2055 | 2070 | | |
2056 | 2071 | | |
| 2072 | + | |
| 2073 | + | |
| 2074 | + | |
| 2075 | + | |
| 2076 | + | |
| 2077 | + | |
| 2078 | + | |
| 2079 | + | |
| 2080 | + | |
| 2081 | + | |
| 2082 | + | |
| 2083 | + | |
| 2084 | + | |
2057 | 2085 | | |
2058 | 2086 | | |
2059 | 2087 | | |
| |||
Lines changed: 10 additions & 14 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
106 | 107 | | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
| 108 | + | |
| 109 | + | |
114 | 110 | | |
115 | 111 | | |
116 | 112 | | |
117 | | - | |
| 113 | + | |
118 | 114 | | |
119 | 115 | | |
120 | 116 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
636 | 636 | | |
637 | 637 | | |
638 | 638 | | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
639 | 666 | | |
640 | 667 | | |
641 | 668 | | |
| |||
651 | 678 | | |
652 | 679 | | |
653 | 680 | | |
| 681 | + | |
654 | 682 | | |
655 | 683 | | |
656 | 684 | | |
| |||
838 | 866 | | |
839 | 867 | | |
840 | 868 | | |
| 869 | + | |
841 | 870 | | |
842 | 871 | | |
843 | 872 | | |
| |||
870 | 899 | | |
871 | 900 | | |
872 | 901 | | |
| 902 | + | |
873 | 903 | | |
874 | 904 | | |
875 | 905 | | |
| |||
901 | 931 | | |
902 | 932 | | |
903 | 933 | | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
| 992 | + | |
| 993 | + | |
| 994 | + | |
| 995 | + | |
| 996 | + | |
| 997 | + | |
| 998 | + | |
| 999 | + | |
| 1000 | + | |
| 1001 | + | |
904 | 1002 | | |
905 | 1003 | | |
906 | 1004 | | |
| |||
0 commit comments