Repository navigation
docs(hipdnn): RFC 0017 Universal Kernel Descriptors (UKD/UMD/UDD/UED/UHD) - #9533
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
mousdahl-amd
left a comment
There was a problem hiding this comment.
High-Level Review
Highest-signal non-blocking items:
- Framing gap — the cost of the status quo is asserted, not quantified; the whole complexity budget rests on kernel-authoring volume that isn't given, and the nearest
alternative (a build-time codegen path) isn't weighed. - Observability is unaddressed — "drop in a file and it works" makes "why wasn't my kernel selected/loaded?" the dominant new failure mode, with no diagnostics surface named.
- No fuzz/test strategy for the descriptor parsers and the safe interpreter, which on the drop-in path parse untrusted input.
- Composition (§9) carries ~half the design weight but open question #3 leaves its v1 inclusion undecided — candidate to split into its own follow-up.
Security Review
Threat model (inferred, RFC states none explicitly): in-process GPU provider; the realistic attacker is a malicious/tampered runtime drop-in package — the drop-in path loads
author code objects and JIT-compiles author HIP source, all running as the host process. Secondary: hostile serialized graph/descriptor hitting the matcher + "safe
interpreter."
The load-bearing decision is made correctly — no inline code in descriptors (symbol names + typed args only), drop-in opt-in/off-by-default/built-in-predicates-only. Top
non-blocking gaps:
- Drop-in folder is a code-exec boundary defined by filesystem perms hipDNN doesn't own — needs explicit path resolution, refuse world/group-writable dirs, reject symlinks.
- Signing is deferred but is the actual enforcement — sequence it as a prerequisite before drop-in ships enabled.
- Three new hostile-input parsers (binary descriptor loader, matcher, expression interpreter) — no stated recursion/step/size bounds → stack-exhaustion + truncated-input
surface. - Shape/workspace integer-overflow — grid/workspace arithmetic over user-controlled dims → under-alloc + OOB device write; needs checked-width fail-closed.
- Installed AOT bundles are trusted by filesystem location alone — integrity verification, not just compat gating.
2c9d60f to
d2fe03e
Compare
Proposes a data-driven kernel ingestor for the hip-kernel-provider that replaces per-kernel hand-written C++ (plan builder, engine, static registration, bespoke launch, selection heuristic) with declarative descriptor files a single generic provider loads and runs: - UKD (Universal Kernel Descriptor): one launchable kernel (graph criteria, engine/heuristic membership, launch spec, kernel source). - UED (Universal Engine Descriptor): one engine (identity, knobs, tags). - UHD (Universal Heuristic Descriptor): one kernel-selection model. - UCD (Universal Composite Descriptor): pipelines, described as future work. Covers graph-criteria matching, a symbolic launch/workspace expression language, declarative argument binding, native-predicate and custom-plan escape hatches, build-time (AOT) and runtime drop-in ingestion, observability, testing and performance gates (reusing hipDNN's existing testing strategy), risks, open questions, and an SDPA worked example. Generalizes the rocKE SDPA launch prototype (PR #9207). Intentionally high-level, with per-subsystem follow-up RFCs. Documentation only; no code changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d2fe03e to
6a12657
Compare
Decompose the UKD into reusable descriptors bound per kernel: - UMD (match), UDD (dispatch), UED (engine), UHD (heuristic) - Launch = kernel source + UDD; UKD = 1 UMD + 1 UED + 1 UHD + N Launches - KDP (KernelDescriptorPack) batches many UKDs into one file - Add section 8 Adapters and Extensibility (source + heuristic adapters; JIT future) - Clarify fusion is a day-one capability; general pattern matching deferred to JIT - New ukd_descriptor_decomposition.svg; update all diagrams to the new model Documentation only; no code changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g, reorder) Second revision pass on the UKD RFC, incorporating two adversarial review rounds and a final QA pass: - Descriptors carry an id (references, by ID) plus a mandatory name; drop schema from diagrams, keep it in worked examples with a version callout. - Add section 8 diagrams (descriptor formats, dispatch dataflow, adapters, JIT seam); fix concepts/decomposition/composition/before-after/flows. - Reframe section 1 (quantified scale, staleness cost, author-owned E2E vision); reframe rollout as a platform stand-up with a follow-up-RFC map. - Multi-kernel launch UKD (SDPA-BWD) is distinct from composition (UCD); add a CBA single-fused-kernel worked example and a C++-to-field table. - Qualify UMD/UDD symbol closure (checked at load), arbitration, and the no-ABI-under-composition workspace/stream contract. - Move Multiple Kernels and Composition after Phased Delivery so committed content leads; purge em-dashes and AI-tells; add glossary terms. Documentation only; no code changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Third revision pass from review feedback: - Rename Sprawl->Maintainability, Slow features->Feature velocity; tighten Staleness; name conv algorithm families; reframe the SDPA prototype and Vision (author-owned E2E delivery); simplify Scope. - Descriptor ids are opaque int64 (name stays the human label); references lined up. UED uses behavior_notes/numerical_notes (drop tags). Kernel source hip_source -> hip. - Replace the section 9 end-to-end flow + diagram with an Observability and Diagnostics section (remove ukd_execution_flow.svg). - Runtime drop-in: opt-in with a dedicated location (drop the env-var). - Custom plan owns workspace sizing; heuristic adapters are always runtime. - Section 5 leads with the SDPA-fwd UMD; CBA becomes a fused-match demo at the end. Phased delivery starts with the follow-up RFC series (format + code bundled). Trim intermediate-buffer sizing; drop the drop-in-identity open question. - Simplify arbitration and the out-of-scope note. Documentation only; no code changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Realign code-example comments (numeric ids left them ragged).
- Add a device/arch constraint to the UMD ('this works on gfx942'), a
constraint-table row, and a note reconciling match-time arch gating with
the kpack manifest; make the symbol lifecycle (declare in UMD -> bind at
match -> use in UDD) explicit and show a workspace formula over bound dims.
- Deep E2E review fixes: make the SDPA-BWD example self-contained (describe
the bwd UMD's bound tensors/dims); relabel the section 1.1 'execution
composition' row as multi-kernel launch; fix a stale (see 10.3)->(13.3)
cross-ref; expand rocKE and JIT on first use.
Documentation only; no code changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Staleness: 'until the re-port is prioritized' (drop the awkward 'funds'). - Maintainability: cover the full carrying cost of each hand-written variant (testing/coverage, keeping it building, updates, fixes), not just bug fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The descriptor family is already shown by ukd_concepts.svg in Section 2, and the UCD pipeline by ukd_composition.svg in Section 13. Remove the decomposition diagram and its Section 2 reference, which duplicated both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ukd_flows: route the runtime-drop-folder arrow into a separate entry on the provider so it no longer overlaps the provider-to-core downflow and read as a two-way link. Each is now a distinct one-way arrow. - Section 4: drop the JSON Schema / FlatBuffer callout; defer the concrete serialization and schema to each format's follow-up RFC. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace stacked acronyms in flowing text with their functional names (match / dispatch / engine / heuristic / kernel / composite descriptor). Acronyms are kept in headings, the Section 2 table, code comments, diagrams, phase-deliverable labels, and the glossary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Repeat the SDPA-forward match descriptor (from Section 5) inline in the worked example so all three descriptors appear together and a reader does not have to jump back to track the referenced UMD. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reframe Section 12: the system is built to full support first, then real engines migrate onto it (coexistence notes kept). Reorder phases to the natural build sequence: AOT packaging (lifting the POC) -> matching -> engine registry -> dispatch (running end to end here) -> kernel selection. Fold runtime drop-in into the packaging phase, and reorder the follow-up RFC table to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase 1 is now migrating the generic build and bundling pipeline out of the rocKE SDPA POC; the full data-driven build pipeline and integration (arbitrary descriptor sets, drop-in, validation) becomes the phase just before heuristics. Dispatch remains where the POC first runs end to end. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the 20 unresolved review threads on PR #9533 plus the author directives that came out of them. Structural additions: - New Section 8 End-to-End Flow: the nine-step path from applicability request to kernel launch, named against hipDNN's real interfaces, with a step/descriptor/interface table, a sequence diagram, and the descriptor/data dependency table. States that the catalog and bound token state are provider-owned on the provider's own handle, since hipDNN passes no stable graph identity across calls today. - Rewrote the SDPA worked example as a two-stage accept/reject grounded in the 21 decline conditions the rocKE dispatcher enforces, including the classifyMaskMode encoding and the catalog-stage decline. Policy and lifecycle: - KDP schema/SDK version tag, mirroring the existing per-graph minimum required engine API version mechanism; per-file-type major.minor versioning with reject-newer and accept-older-within-major. - Additive versus breaking KMD change classification, replacing the unconditional retrain statement. - Knobs restricted to the values present in the loaded catalog, with the current front-end ordering named as an open dependency. - Provider boundary: a new runtime dependency means a new provider, plus the minimum provider surface and the no-Find-mode statement. Corrections where the text contradicted commitments made in review: - Fail-closed is a base-path invariant, not composite-only. - Fusion is day-one for matching; the fuse decision is the host's. - MIOpen and hipBLASLt are carved out of provider normalization. - Engine selection is an ordered-policy loop, not first-claim-wins; corrected the ONNX Runtime prior-art row accordingly. - Duplicate-id caveat for in-place retunes, deferred to RFC 0009. Also adds HIPDNN_DISABLE_UKDS, the umbrella-matcher constraint, the metadata unique-key boundary, and fixes select.candidates to reference KDP or UED ids.
## Summary Remove the standalone rocKE client C++ api (the plugin shell under `dnn-providers/hip-kernel-provider/rocke/library/api/`). We are moving rocKE kernel bundles over to a generic ingestion path in `hip-kernel-provider`, so the client, which is a non-functional shell today, is no longer needed. JIRA ID: AICK-1737 ## Risk Low (2). Deletes dead code plus its CMake wiring; no behavior change. The build-time Python beside it (`kernels`/`builders`/`dispatch`/`tests`) is kept and still wheeled. ## Related - AICK-1704: migrate the real ingestion logic from the POC branch (out of scope here) - POC / source: #9533 ## Testing `hipdnn-providers-all` superbuild configures cleanly with rocKE enabled after the removal; `rocke-pyenv`/`rocke-wheels`/`rocke_library` and the platform engine tests all survive, and no `rocke_client` target remains. - [x] Superbuild configure (rocKE on) - Status: Passed - [ ] PR CI - Status: Pending ## Changes - Delete `rocke/library/api/` and `rocke/library/CMakeLists.txt`. - `rocke/CMakeLists.txt`: drop `add_subdirectory(library)`; the Python tree is now wheeled directly by the existing pyenv/wheels blocks. - `hip-kernel-provider/CMakeLists.txt`: drop the now-unused `ROCKE_CLIENT_PARENT_PROJECT`. - Update `ROCKE_LAYOUT_RESTRUCTURE.md` to drop the removed `api/` layer.
## Summary Draft PR **against the RFC branch** (`users-bharriso-rfc-ukd-kernel-ingestor`), not `develop`, so the delta from review feedback is reviewable in isolation before it folds into [#9533](#9533). Addresses the **20 unresolved review threads** on #9533 plus the author directives that came out of them. Resolved threads were explicitly excluded. ### Structural additions - **New Section 8, End-to-End Flow.** The largest gap. Covers the nine-step path from applicability request to kernel launch, named against hipDNN's real interfaces (`IEngine::isApplicable`, `initializeExecutionContext`, `getMaxWorkspaceSize`, `IPlan::execute`), with a step/descriptor/interface table, a sequence diagram, and the descriptor/data dependency table reviewers asked for. States plainly that the catalog and bound token state are **provider-owned**. (Second round: the key is now `(engine id, graph id, device id)` against a graph identity this RFC adds, rather than a provider-computed hash. See below.) - **Rewrote the SDPA worked example (§13).** *Superseded in the second round: that rewrite was grounded in the wrong kernel family and has been replaced. See below.* The first-round version was a two-stage accept/reject grounded in the 21 decline conditions `SdpaGraphAdapter` enforced. Shows all three decline shapes (graph-level flag, absence-across-23-optional-tensors, all three tri-state flavors), encodes `classifyMaskMode`, drives geometry from `$kernel.*` instead of a hard-coded `block.x: 256`, and includes a catalog-stage decline where the matcher accepts but no kernel declares that mask mode. ### Policy and lifecycle - KDP schema/SDK version tag, framed as mirroring hipDNN's existing per-graph minimum-required-API-version mechanism rather than as a new invention. - Per-file-type major.minor versioning: reject newer than understood, accept older within the major. - Additive vs. breaking KMD change classification, replacing the previously unconditional retrain statement. - Knobs restricted to the values actually present in the loaded catalog. - Provider boundary: a new runtime dependency means a new provider, plus the enumerated minimum provider surface and the no-Find-mode statement. - `HIPDNN_DISABLE_UKDS`, completing the engine/pack/kernel disable ladder. ### Corrections where the RFC contradicted commitments made in review - Fail-closed is a **base-path** invariant, not composite-only. - Fusion is day-one for *matching*; the fuse decision is the host's and there is no fusion cost model. - MIOpen and hipBLASLt explicitly carved out of provider normalization. - Engine selection is an ordered-policy loop, **not** first-claim-wins; corrected the ONNX Runtime prior-art row accordingly. - Duplicate-id caveat for in-place retunes, deferred to RFC 0009. ## Second round: review-driven corrections A deep review of the first round produced eight blocking findings, and verification passes over the corrections themselves found more. All are addressed here. - **§8's applicability cache is rebuilt on a graph identity.** The first-round design hashed the serialized graph bytes and confirmed a hit with `memcmp`. That cannot be implemented at the `IEngine` layer, which receives a deserialized `IGraph` with no buffer accessor, and it obliged the provider to retain a copy of every cached graph. The key is now `(engine id, graph id, device id)`. Adding the engine id fixes a real aliasing defect: the catalog is per-engine, so the old key let one engine's catalog answer for another in the same provider. - **A drop-in pack appearing now invalidates the cache.** The first round covered removal only, so a dropped-in kernel would never have been picked up for a graph shape the process had already seen, which is the common case in a long-running server. An inventory generation counter is folded into the key. - **A UED's identity is reconciled with hipDNN's engine-id space.** A UED carries a name; the engine id is that name hashed, as for any hand-written engine. Names must be globally unique and should be namespace-scoped (`rocke:SDPA`), and because descriptor engines register at load rather than at build, collision detection moves to a load-time check. - **Autotuning is a single self-measure lever, not per-candidate enumeration.** Modelled on the MIOpen provider's existing `global.benchmarking` knob: the engine measures its own catalog and caches the winner. This is what makes "search within one engine's full catalog" true, since addressing kernels from outside through knob settings would only ever reach the fields an engine chose to expose. - **Adding a kernel is stated as the two-step it is.** A new UKD under an unchanged KMD is loaded, catalogued, and immediately measurable, but it is not ranked against its peers until the engine's UHD is retrained, unless it is the only kernel matching the graph. The first round implied a new kernel was free; §13.8 said "no heuristic". Retraining is self-serve and is now named in §11. - **The UHD scorer interface is pointwise by construction.** A scorer sees one kernel at a time and is never handed the catalog, so knob filtering and ranking commute structurally rather than by author discipline. Set-aware selection is named as future work in the UHD follow-up. - **§5's operator table is normative and closed**, and every operator used in the document is in it. This round added `value_or_default`, `ceil_div`, `rsqrt`, `min`, and `max`, each justified by real dispatch code. - **§14 states the real dependency order.** "No strict build order" was not credible: six of the ten follow-ups are a chain, KMD first, and nothing runs until all six land. ### §13 regrounded on gfx950 dense attention The first-round worked example was built against the wrong kernel family. It cited `attention_unified`'s geometry while the adapter it claimed to encode targets the FMHA-fwd-MFMA family, which is f16-only and has no `num_warps`/`tile_size` axis, so the example's headline **accept** case (bf16) would in reality have **declined**. §13 is now grounded on `rocke/library/kernels/gfx950/attention_dense.py` ([#9480](#9480)), the current dense flash-attention prefill kernel: - Its own spec is already split into functional compile-time fields and validated performance knobs, which maps almost one-to-one onto matcher criteria and KMD fields. - Its real ABI is **five arguments** (four pointers plus `scale`; seven with varlen), not the eight of the first round, because every shape quantity is baked at compile time. Nothing in it needs a source kind this design does not have. - Its two real, measured cohorts (`persistent` vs default) differ in **grid formula shape** rather than argument slots, which surfaced a genuine gap in the one-UDD-per-KDP rule. §6 now names formula shape as a second trigger for a separate pack, distinct from one kernel needing several dispatches, which remains a multi-launch pack. - Where the example depends on something that does not exist yet it says so: there is no AOT artifact for this family today, and the build-only rocKE adapter of §9.1 is the named bridge. - §13.8 now also counts the **new-family** case honestly, which is what this example actually is: ten descriptors, of which eight are authored once and shared by every kernel that later joins. ## Changes this RFC asks of hipDNN core The first round claimed no changes outside the provider. That was too strong. Four are now named in §3 rather than left to be discovered. Each is additive, and none changes an interface an engine implements: | Addition | Why | Section | |---|---|---| | A graph identity, minted at graph-descriptor finalize | so a provider can cache per-graph work; mirrors the existing serialized-buffer cache and its invalidation | §8 | | An extended `$device.*` property set | the expression vocabulary reads device facts hipDNN does not carry today | §5 | | UED names in the engine-id space | a descriptor-backed engine needs an id the host already understands | §4 | | A workspace entry point for custom plans | the workspace query arrives before a plan exists, so a custom-plan handler must answer it | §6 | ## Open decisions for reviewers The seven questions the first round raised (Q1-Q7) are answered in-text by this round. Two decisions remain genuinely open and are the ones worth a reviewer's attention: | # | Question | Where | |---|---|---| | 1 | Whether a future UHD may reason over the candidate set as a whole, giving up the commutativity the pointwise scorer buys | UHD follow-up; scorer interface in §9.2 | | 2 | Whether an operation's conventional attribute defaults, such as SDPA's `1/sqrt(head_size)` scale, are settled once in the schema or left to each pack | §17, Open Question 6 | Two earlier answers are worth re-reading because this round changed them: Q1 (one UDD per KDP) now has a **second** trigger, grid/block/workspace formula shape, not just argument-slot presence; and Q7 (pinning a specific UKD) is now clearly optional rather than load-bearing, because the self-measure lever reaches the whole catalog without per-kernel addressing. ## Third round: rocKE integration and four review findings The second round's §13 described what did not exist yet: a `kernel_source` pointing at an `hsaco` file nobody builds, a `custom_library` UHD wrapping a hardcoded host threshold, and a §13.8 leading with the missing build step. This round describes the integration instead, and fixes four findings a scoped review pass raised against §5, §6, and §13. ### rocKE becomes a real kernel source - **New `rocke` kernel-source kind** (§7, §9.1). A rocKE kernel is a Python builder that emits IR from a frozen spec, so it is neither directly loadable nor one artifact: one builder yields a different code object per spec. The source therefore carries both halves, `source` + `entry` plus the explicit `build` values for one instance, and the build-only adapter calls the builder, runs the rocKE AOT compile, and packs the result. - **A pack's kernel vector is its AOT build list.** Every UKD names the same builder and differs only in its build values, so what gets compiled is derived from what is catalogued rather than maintained beside it. No descriptor names an artifact file; the adapter owns the name. - **§13.6's UHD is a real UHD**: a model ranking the engine's whole catalog, with `persistent` and `num_persistent` as ordinary features and the work term the retired host threshold branched on as a derived feature. The hardcoded `work >= num_persistent` rule is one decision boundary in the training data, not a rule the descriptors carry. - **§13.8 no longer narrates absence.** Step 1 is picking build values, not waiting for tooling. Ragged, varlen, and sliding-window stay as extension points. - **rocKE is never called a DSL.** Generic DSL usage elsewhere is unchanged. ### Review findings - **`batch` was baked but never pinned** (blocking). `AttentionDenseSpec` bakes batch (`attention_dense.py:107`); it sizes the K/V buffer-resource bounds and, in the persistent cohort, the grid-stride trip count. §13's KMD had no `batch` field and the matcher no `$kernel.batch` pin, so it accepted any batch and could hand a graph to a kernel built for another, the exact silent-wrong-kernel class §5's pinning rule exists to prevent. Added the field and the pin. - **`$scale.value` was not expressible** (blocking). It named an eight-arm flatbuffers union with no generic numeric accessor, and it was passed as the second argument to `value_or_default`, whose §5 definition took a literal. Resolved by adding `$q.value_f32` as a **precomputed field** (§5's existing mechanism, the same one `$q.packed` and `$q.stride_order` use), generalizing `value_or_default`'s fallback to any type-compatible expression including a field reference, and gating the scale tensor's dtype so a non-float value cannot narrow silently into an `f32` kernarg. - **§13.7 contradicted §13.5** (blocking). The table claimed `args_signature` needs "no computed sources" while §13.5's `scale` is an explicit `expr` source. The row now says what is there. - **Two ungrounded `$graph.*` tokens.** `$graph.node_kind` is not a field in `graph.fbs` and is redundant with the structural pattern, so it is deleted. `$graph.override_shape` mis-named the real `is_override_shape_enabled`, now renamed and declared in §5's Graph namespace. - **Three baked tuning axes were missing from the KMD**, found while verifying the round above. `waves_per_eu`, `lazy_rescale`, and `interleave` are baked into the IR by both builders but were not KMD fields, so two kernels differing only in one of them would collide on the catalog key. Making the `build` values explicit is what exposed this; the artifact filenames had hidden it. All three added with the spec's own defaults. - **`persist_decode` carries the resolved decode, never `"auto"`.** `"auto"` is not a third configuration but a request to compute one (`resolved_persist_decode`), and the KMD is both the catalog key and the heuristic's feature space: two UKDs carrying `"auto"` but resolving differently would collide on the key and rank on a string rather than the decode they were built with. This answers the reviewer's open question on the point. - **Residual drafting artifacts** removed: three places narrated the edit history ("is the fix") rather than stating the design. Verified correct by two reviewers against source and left unchanged: the 27-name operand map vs `sdpa_attributes.fbs`; the 23-of-24 refusal list vs `AttentionDenseSpec`; §13.3's mask classifier vs `getMaskType`; all three §13.4 cases re-derived numerically; grid/block/args vs `attention_dense_grid`/`_block`/`_signature`; `shared_mem_bytes: 0`; the dtype pin spelling; and every §13 prose count. ## Risk Assessment Risk 1 (docs only). No product code, schemas, plugin ABI, or build wiring changes in this PR; blast radius is one file under `projects/hipdnn/docs/rfcs/`. Note that the design it describes now asks for four additive changes to hipDNN core (see above) which are not implemented here. ## Testing Summary - Anchors and TOC: every in-document reference resolves; TOC matches all 19 headings. - All complete JSONC descriptor blocks parse; multi-object blocks verified object by object. - Descriptor cross-references resolve: every `matchers`, `engine`, `dispatch`, `heuristic`, and `metadata` id resolves to a descriptor defined in the document, except the two in §15.2's explicitly-labelled UCD sketch, which are pre-existing. - Every `$kernel.*` field referenced by §13's matcher and both UDDs is declared in its KMD; the two UKDs' metadata tuples are distinct; both `args_signature` lists match the real five-argument ABI. - Operator closure: every operator used anywhere in the document appears in §5's normative table. - §13's factual claims verified against `attention_dense.py` and `dispatch/attention.py` on `origin/develop`, including the accept/decline arithmetic. - Code fences balanced; no em-dashes, tabs, CRLF, or trailing whitespace; final newline present. - `pre-commit run --files` on the changed file: passed. ## Testing Checklist - [x] Doc validation (links/anchors, TOC, fences, whitespace/EOL) - Status: Passed - [x] `pre-commit` - Status: Passed - [ ] PR CI - GitHub PR checks (`ci:docs-only`) - Status: Pending ## Technical Changes `projects/hipdnn/docs/rfcs/0017_UniversalKernelDescriptor.md`: +164 / -89 this round (+1570 / -341 against the base branch). §13 rewritten and regrounded on `attention_dense`; §8 cache rebuilt on a graph identity with an inventory generation; §5 operator table made normative and closed; §4 UED identity; §2 self-measure lever; §16 dormancy and lifecycle; §6 formula-shape pack trigger and the custom-plan workspace obligation; §3 core-additions table; §11 retraining tooling; §14 dependency order; §17 two new open questions; §19 glossary entries for the new terms. Third round: §7 and §9.1 gain the `rocke` source kind; §13.6's UKDs and UHD are rebuilt on it; §5 declares `value_f32`, `is_runtime_pass_by_value`, and `is_override_shape_enabled`, and generalizes `value_or_default`; §13.2 pins `batch` and gates the scale tensor's dtype.
|
Building on Brian's note in the HIP-macros thread — "the JIT specification will be slightly different from the current UKD's, since you could have 1 definition generate many possible variants that need to be picked between" I have been playing around with this in ruka and have been implementing the JIT kernel path: porting Triton and FlyDSL kernels onto the KDP format, analogous to the §9.3 path. With some thanks to Claude for summarizing it for me:
Otherwise great work so far! I really like this whole idea to standardize the way to describe every kernel's capabilities etc. Looking forward to the actual implementation 👍 |
SamuelReeder
left a comment
There was a problem hiding this comment.
LGTM! Let me know if further fundamental changes are made and I'd be happy to review again.
Thanks for the review, and feedback @amd-bartgips! For JIT, we need capabilities that allow us to express all the different ways variants can be made for populating the catalog that the heuristic ranks. Decently likely we will also need to expand heuristics to support two-stages. Once JIT exists, the catalog of possible entries explodes, and it becomes unlikely you can run a full heuristic estimation to rank each entry. Instead we would likley need a quick running heuristic that can pick the candidate list, and then run the ranking heuristic to determine the final winner. All that being said, these are just early thoughts on how this can work as we extend to support JIT, and future RFCs will be made that can make this more concrete. |
Move the SDPA worked example into its own companion document and apply four clarifications from review. - Split Section 13 (575 lines, 21% of the RFC) into 0017_UniversalKernelDescriptor_WorkedExample.md, following the existing 0012_CuDNN_Shim / _Reference precedent. Section 13 stays in place as a summary that states the three load-bearing results and links to the companion, so no section renumbering is needed and the ~24 in-document anchors and prose references still resolve. The author-cost table and the ten-descriptor ratio stay in the RFC, where the general-audience reader needs them. - Fill in the two elisions the length limit had forced: the mask-mode classifier is now spliced into the matcher in full rather than referenced by a placeholder string, and both kernel packs carry complete ids. - Collapse the duplicated descriptor definitions. The Section 2 table is now the single reference definition, absorbing the detail from the Section 1 bullet list, which is removed. - Give the KDP example an id and name, matching the rule Section 4 already states for every descriptor, and note that the id is what HIPDNN_DISABLE_KDPS accepts. - State when the drop-in discovery scan runs: not on the applicability path, so a cached verdict is accepted against the current generation, and packs are expected to be staged before process start. - Require heuristic training-set provenance so a kernel's dormant or ranked state is observable, add it to the diagnostics surface, and track the format in the heuristic follow-up RFC. Documentation only; no code changes. JIRA ID: AICK-1668
The rfcs directory is a flat numbered list, so a second file for one RFC reads as a stray entry in it. Move the SDPA worked example into rfcs/examples/, keeping the 0017_ prefix so it still sorts with the RFC it belongs to, and fix the relative links in both directions. Documentation only; no code changes. JIRA ID: AICK-1668
"This table is the reference definition of each one; the rest of the document builds on it rather than restating it" describes how the document is organized, which is useful to a reviewer and noise to a reader. The preceding sentence already introduces the table. Documentation only; no code changes. JIRA ID: AICK-1668
The table led with UKD, whose entry refers to the KMD it fills in, the pack it inherits from, and that pack's engine, none of which the reader has met yet. Order the rows so every descriptor is defined before it is cited: KMD, UHD, UED, UMD, UDD, UKD, then KDP, which binds the rest. Two rows referred to a pack as a KDP before that row existed; they now say "pack", which reads the same and removes the last forward reference. No row content otherwise changes. Documentation only; no code changes. JIRA ID: AICK-1668
## Summary Adds a single opt-in build flag that gates the generic kernel ingestor build-time logic (dynamic engine loading, kpack bundling/packaging, dynamic engine registration) so that work tracked under the hipDNN generic kernel ingestor epic can land incrementally without affecting default builds. See [RFC 0017: Universal Kernel Descriptors](https://github.com/ROCm/rocm-libraries/blob/develop/projects/hipdnn/docs/rfcs/0017_UniversalKernelDescriptor.md) ([rocm-libraries#9533](ROCm/rocm-libraries#9533)) for the platform this flag gates. JIRA ID : ALMIOPEN-2399 ## Risk Assessment Minimal risk. This declares an inert, default-OFF build flag in `FLAGS.cmake` with no accompanying source changes; no consuming code currently reads the propagated variable, so default builds and installed artifacts are byte-for-byte unaffected. ## ASIC Coverage No ASIC impact. The flag is plumbing only (default OFF, no gated logic exists yet in `hipDNN`/`hipkernelprovider` to exercise); standard PR CI is sufficient. ## Testing Summary - CMake configure smoke test confirming the flag registers and reports correctly with its default value. - CMake configure smoke test confirming the flag can be toggled on via the command line and propagates through `therock_finalize_flags()`/`therock_report_flags()` without error. ## Testing Checklist - [x] CMake configure (flag default) - `cmake -S . -B build -DTHEROCK_ENABLE_ALL=OFF -DTHEROCK_ENABLE_HIPDNN=OFF` - Status: Passed (reports `HIPDNN_ENABLE_KERNEL_INGESTOR = OFF`) - [x] CMake configure (flag enabled) - `cmake -S . -B build -DTHEROCK_ENABLE_ALL=OFF -DTHEROCK_ENABLE_HIPDNN=OFF -DTHEROCK_FLAG_HIPDNN_ENABLE_KERNEL_INGESTOR=ON` - Status: Passed (reports `HIPDNN_ENABLE_KERNEL_INGESTOR = ON`) - [ ] PR CI - GitHub PR checks - Status: Pending ## Technical Changes - Declares `THEROCK_FLAG_HIPDNN_ENABLE_KERNEL_INGESTOR` (default `OFF`) in `FLAGS.cmake`. - Scopes `CMAKE_VARS HIPDNN_ENABLE_KERNEL_INGESTOR=ON` to the `hipDNN` and `hipkernelprovider` subprojects, matching the epic's plan to gate plugin-sdk and hip-kernel-provider dynamic-engine logic behind one flag.
JIRA ID : AICK-1699 Close date: August 21st ## Summary Adds **RFC 0019: Universal Heuristic Descriptor (UHD)** — the "UHD + kernel selection" follow-up named in RFC 0017 §12.2 ([PR #9533](#9533), open). It designs the data-driven, per-engine kernel-selection model for the descriptor-based provider, plus the tooling that generates one. Docs-only: one new file, `projects/hipdnn/docs/rfcs/0019_UniversalHeuristicDescriptor.md`. ## What it covers - **UHD schema & adapters** — one discriminant (`adapter`) over `static_order` / `table` / `tree_data` (default) / `onnx` / `custom_library`; the model ships **as data with the engine**, read by an in-tree interpreter (no `liblightgbm` runtime dependency). - **Ownership** — the UED (engine) owns one UHD and one KMD; a KDP joins the engine. Knobs are a view onto KMD fields; the KMD↔UHD coupling is conditional (only a *breaking* KMD change forces a retrain — additive changes and dispatch-only fields do not). - **Feature contract** — an inline `features_signature` in JsonLogic (the language pinned by the sibling UMD RFC), sourced from the matcher's bound symbols; a three-part fail-closed contract check. - **Efficient evaluation** — the `features_signature` lowers to a compiled expression at load (reusing the UMD PoC's compile-once `JsonLogic.hpp`), with a shared per-graph feature prefix and a per-candidate suffix, so the hot path stays tight while the data contract stays extensible. - **Engine-selection integration** — applicability bubbles up before ranking; the tooling produces **two** heuristics (a cheap engine-level performance estimate and the fine-grained config UHD) consumed by **two** policies (quick vs. thorough), with a rank-ordering fallback. Cardinal (calibrated-TFLOPS) scoring enables cross-engine comparison. - **Generation pipeline** — a standalone tool over hipDNN's public **autotune** API ([RFC 0013](https://github.com/ROCm/rocm-libraries/blob/develop/projects/hipdnn/docs/rfcs/0013_Autotune.md)); ships a trivial `static_order` pack, then generates and drops in a `tree_data` UHD — usable by any package author, no provider code changes. ## Relationship to concurrent RFCs - **Parent: RFC 0017 (Universal Kernel Descriptors)** — [PR #9533](#9533) (open into `develop`; not yet merged there). This RFC is aligned with the review-feedback updates that landed on the RFC 0017 branch via [PR #10039](#10039): knob-as-exposed-KMD-field, cache key `(engine id, graph id, device id)` + inventory generation counter, lazy-load-on-ranking (not on winning), and the conditional KMD↔UHD coupling / KMD-superset-of-features relationship. - **Sibling: RFC 0018 Universal Match Descriptor (UMD) & graph matcher** — [PR #10341](#10341), with a working implementation. Its `JsonLogic.hpp` (compile-once / evaluate-many, `$`-sigil convention) is the concrete evaluator this RFC's feature extractor reuses; this RFC also answers that RFC's open question by making the matcher's bindings the canonical feature source. **Numbering note:** the UMD RFC holds "0018", so this doc is **0019**; final numbers reconcile at review across the RFC 0017 §12.2 follow-up series. ## Test plan - [ ] Docs-only change — no build/test impact. - [ ] Markdown renders; internal section anchors resolve (audited: 99 refs, 0 broken). - [ ] RFC review by hipDNN descriptor-series owners. - [ ] Confirm the 0018/0019 numbering with the UMD RFC author (dahawkin, PR #10341). --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Christopher Erb <christopher.erb@amd.com>
Update All review threads are now addressed. Finalizing, and merging shortly. TY for the feedback!
Summary
RFC proposing a data-driven kernel ingestor for the hipDNN
hip-kernel-provider. Today, adding a kernel or fused graph means hand-writing C++ for each one: a plan builder that pattern-matches the graph, an engine and its registration, bespoke launch code, and a selection heuristic, i.e. a code change and recompile per kernel. As kernels multiply per architecture, data type, problem shape, and fusion, this creates four compounding problems: scale, staleness (hand-ported copies fall behind upstream authors), maintainability sprawl, and slow feature velocity.This RFC replaces that per-kernel C++ with a small family of declarative descriptors that one generic provider loads and runs. The unit an author ships is a KDP (Kernel Descriptor Pack): one file that binds a shared matcher set, one engine, and one dispatch descriptor over a vector of child kernels, so a family of kernels is a few shared descriptors plus one small kernel entry each, not hundreds of near-duplicate files. Every descriptor is referenced by ID:
$kernel.<field>.A Launch is one dispatch step inside a UDD (grid, block, shared memory, argument signature) with a named source slot, paired at runtime with the UKD source that fills it: a simple kernel is a one-Launch UDD, and a multi-launch kernel such as SDPA backward is a several-Launch UDD run in order.
It generalizes the existing rocKE SDPA launch prototype (PR #9207) into data and covers: graph-criteria matching (UMD) with a declare/bind/use symbol lifecycle; a symbolic launch/workspace expression language and declarative argument binding (UDD); kernel sources plus an adapter model for extending supported source and heuristic formats; observability and diagnostics; AOT packaging and opt-in runtime drop-in; a worked SDPA example; and a phased rollout that starts by publishing a per-format follow-up RFC before implementation. Multi-kernel launch and composition (UCD, Universal Composite Descriptor) are framed as fast-follows, and JIT as a future follow-on that reuses the same descriptors.
Documentation only; no code changes.
JIRA ID: AICK-1668
Risk Assessment
Risk 1 (docs only). No product code, schemas, plugin ABI, or build wiring changes; the blast radius is limited to files under
projects/hipdnn/docs/. The RFC proposes an approach and is explicitly requesting feedback before any implementation is scoped, so nothing here alters runtime behavior.Testing Summary
features_signature, and UKD metadata agree.pre-commitrun on both changed files: all applicable hooks passed (trailing whitespace, end-of-file, large files).Testing Checklist
pre-commit) - Status: Passedci:docs-only) - Status: PendingTechnical Changes
projects/hipdnn/docs/rfcs/0017_UniversalKernelDescriptor.md: the RFC. It defines the descriptor family (UKD kernel, KDP pack, UMD match, UDD dispatch, UED engine, UHD heuristic, KMD metadata), where a KDP binds a matcher set, one engine, and one dispatch over a vector of child UKDs and each engine owns its one heuristic and one metadata schema. Covers matching with a declare/bind/use lifecycle, the symbolic launch/workspace expression language and declarative argument binding, the Launch concept, kernel-source and heuristic adapters/extensibility, observability and diagnostics, AOT packaging and opt-in runtime drop-in, a worked SDPA example, phased delivery (starting with per-format follow-up RFCs), and multi-kernel launch plus composition (UCD) as future work.projects/hipdnn/docs/rfcs/examples/0017_UniversalKernelDescriptor_WorkedExample.md: the long-form SDPA worked example, split out of the RFC per review. Follows the existing0012_CuDNN_Shim/0012_CuDNN_Shim_Referencecompanion-document precedent. Section 13 of the RFC remains as a summary linking to it, so no sections are renumbered and all existing anchors still resolve. The split also let two prior elisions be filled in: the mask-mode classifier is now spliced into the matcher in full, and both kernel packs carry complete ids.projects/hipdnn/docs/images/ukd_*.svg: before/after, concepts, criteria match, dispatch flow, ingestion flows, adapters, composition, and JIT seam.