Skip to content

Commit 36e247e

Browse files
committed
docs(internal): hipGraph decode-capture track handoff README
Internal handoff for PR ROCm#5019: thesis, per-file implementation w/ code, verification, architecture note, and the maintainer review (approach superseded by ROCm#4956; fp16/int4 gate is the likely-additive piece). Lives on the non-PR branch only; not for upstream.
1 parent f8b7c6c commit 36e247e

1 file changed

Lines changed: 255 additions & 0 deletions

File tree

HIPGRAPH_DECODE_README.md

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
# hipGraph (GPU graph capture) for LLM decode — track README (handoff)
2+
3+
**Purpose of this doc:** hand off the hipGraph decode-capture work, PR
4+
**[ROCm/AMDMIGraphX#5019](https://github.com/ROCm/AMDMIGraphX/pull/5019)**, including the maintainer
5+
review that lands on it. Single reference for whoever takes this over while I'm out. NOT in the PR
6+
branch — lives on the internal
7+
[`amd/dev/adilohia/hipgraph-decode-capture`](https://github.com/aditya-dl/AMDMIGraphX/tree/amd/dev/adilohia/hipgraph-decode-capture)
8+
branch so it never reaches upstream reviewers.
9+
10+
**Fork:** https://github.com/aditya-dl/AMDMIGraphX · **PR:**
11+
[#5019](https://github.com/ROCm/AMDMIGraphX/pull/5019) · **superseding PR:**
12+
[#4956](https://github.com/ROCm/AMDMIGraphX/pull/4956) · full branch/commit table in §6.
13+
14+
> ⚠️ **Read §5 first if you're triaging the PR.** A maintainer (pfultz2) has indicated our approach is
15+
> superseded by an in-flight implementation (#4956). The mechanism below works and is validated, but
16+
> the *placement* (`program::eval`) is the thing under review.
17+
18+
---
19+
20+
## 0. Background (read this first if you're new to the area)
21+
22+
Plain-language orientation so the rest is readable without deep GPU/MIGraphX knowledge. Skip if you
23+
already work in this stack.
24+
25+
- **MIGraphX** is AMD's inference engine: it takes a model, compiles it into a list of GPU operations,
26+
and runs them. It's the layer being changed here.
27+
- **LLM inference has two phases.** *Prefill* processes the prompt once; *decode* then generates the
28+
answer one token (word-piece) at a time, re-running the model per token. Decode dominates latency
29+
for chat-style use, so it's where we optimize. **"per-token" = once per generated token.**
30+
- **The problem we attacked — dispatch overhead.** To run the model for one token, the engine tells
31+
the GPU to run ~50+ small operations ("kernels"), issuing them **one at a time** from the CPU. Each
32+
hand-off has CPU cost, and the gaps between them let the GPU clock throttle down. On fast GPUs this
33+
per-launch overhead — not the actual math — is a big chunk of per-token time.
34+
- **hipGraph (the fix) — this is GPU "graph capture."** If you already know graph capture (NVIDIA
35+
CUDA Graphs / DirectML command-list replay), that's exactly what this is; hipGraph is HIP/AMD's
36+
version. For everyone else: it lets you **record** a sequence of GPU operations once and **replay**
37+
the whole sequence with a single command, so instead of ~50 hand-offs per token you do one. This is
38+
"capture/replay."
39+
- **fp16 vs int4 ("quantization").** Model weights can be stored at full precision (**fp16**, 16-bit)
40+
or compressed to **int4** (4-bit) to save memory/bandwidth. int4 needs an extra on-the-fly
41+
"unpack/dequantize" step. Our change **only** turns on hipGraph for fp16 models — we measured that
42+
it makes int4 *slower*, so int4 is deliberately left on the normal path (the "gate," §2d).
43+
- **The review situation.** This work is an open pull request (#5019). A senior maintainer has said
44+
the *idea* is fine but it's built in the wrong place, and there's already another in-progress
45+
version (#4956) he prefers. So the open question isn't "does it work" (it does) — it's "which
46+
implementation lands, and does our int4 safeguard get carried over." See §5.
47+
- **A few names you'll see:** *develop* = the upstream main branch the PR targets; *rel-2608* = the
48+
internal release branch our other work is built on; *EP* = "execution provider," the plug-in that
49+
lets the inference runtime use MIGraphX; *pass* / *op* = MIGraphX's two normal ways to add
50+
functionality (a compile-time transform, and a runtime operation) — the maintainer wants the
51+
feature expressed as those rather than by editing core engine code.
52+
53+
---
54+
55+
## 1. Original thesis
56+
57+
LLM decode is bottlenecked on discrete GPUs by **host dispatch overhead**, not kernel compute.
58+
`program::eval` issues the per-token kernel sequence one launch at a time (`generic_eval` → one
59+
`hipExtModuleLaunchKernel` per op, ~50+ launches/token). The per-launch host cost plus the GPU-clock
60+
throttle from the resulting dispatch bubbles is a measurable fraction of per-token latency. Other
61+
backends already avoid this (CUDA Graphs; DirectML D3D12 command-list replay). **Thesis: capture the
62+
decode kernel loop into a hipGraph once and replay it with one launch per token.**
63+
64+
## 2. Implementation (what was built)
65+
66+
Commit `f8b7c6c17` — 4 files, +198/−2. The mechanism has three parts: **capture/replay primitives**
67+
on the GPU context, **routing** through eval, and a **fp16-only gate**.
68+
69+
### 2a. GPU context — capture/replay primitives
70+
`src/targets/gpu/include/migraphx/gpu/context.hpp`
71+
72+
RAII handles + the capture/replay entry point. `execute()` is the single entry: off → run eagerly;
73+
graph already built → replay; first eval → capture, instantiate, replay.
74+
75+
```cpp
76+
using hip_graph_ptr = MIGRAPHX_MANAGE_PTR(hipGraph_t, hipGraphDestroy);
77+
using hip_graph_exec_ptr = MIGRAPHX_MANAGE_PTR(hipGraphExec_t, hipGraphExecDestroy);
78+
79+
bool is_graph_enabled() const // opt-in, not cross-compiling, AND capturable (see gate)
80+
{ return enabled(MIGRAPHX_ENABLE_HIPGRAPH{}) and not is_cross_compile() and graph_capturable; }
81+
82+
void execute(const std::function<void()>& run_kernels)
83+
{
84+
if(not is_graph_enabled()) { run_kernels(); return; } // eager (default)
85+
if(has_graph()) { replay_graph(); return; } // steady-state: 1 launch
86+
begin_graph_capture(); // first eval: capture...
87+
run_kernels();
88+
if(end_graph_capture()) replay_graph(); // ...instantiate + run once
89+
else run_kernels(); // capture failed -> eager
90+
}
91+
```
92+
**What the code does, line by line:**
93+
- The two `using` lines wrap the raw HIP graph handles (`hipGraph_t`, `hipGraphExec_t`) in
94+
smart-pointer types that auto-destroy them — so we never leak a captured graph.
95+
- `is_graph_enabled()` is the master on/off check, true only when **all three** hold: the user set the
96+
env flag, we're not cross-compiling (compiling for a GPU that isn't present), and this program is
97+
capturable (the fp16 gate — §2d). If any is false, hipGraph is skipped entirely.
98+
- `execute()` is the one function the rest of the engine calls to run the kernel loop. It has three
99+
cases: (1) feature off → just run the kernels normally ("eager"); (2) a graph was already captured
100+
on a previous token → replay it with one call and return; (3) first token → start capture, run the
101+
loop once (which *records* the kernels rather than running them), finalize the graph, and replay it
102+
once to actually produce this token. If capture fails for any reason, fall back to running eagerly so
103+
the result is always correct.
104+
105+
`begin/end_graph_capture()` wrap `hipStreamBeginCapture` (ThreadLocal) … `hipStreamEndCapture` +
106+
`hipGraphInstantiate`; `replay_graph()` is `hipGraphLaunch`. State held on the context:
107+
`captured_graph`, `graph_exec`, and `bool graph_capturable = true` (set false by the gate).
108+
109+
### 2b. program::eval — routing ⚠️ THIS is the contested change
110+
`src/program.cpp` (the single-context branch)
111+
112+
```cpp
113+
else if(contexts.size() == 1)
114+
{
115+
contexts.front().execute([&] {
116+
ret = generic_eval(*this, contexts, params, [&](auto&&, auto f) { return f(); });
117+
impl->graph_cached_results = ret; // captured eval produces no output; cache it
118+
});
119+
if(ret.empty())
120+
ret = impl->graph_cached_results; // replay path: reuse cached output args
121+
}
122+
```
123+
**What the code does:** `program::eval` is the engine's run function. The `contexts.size() == 1`
124+
branch is the common single-GPU case (which decode hits). Instead of running the kernel loop directly,
125+
it now hands the loop to `context::execute()` (from §2a) as a callback — so the context can decide to
126+
run it eagerly *or* capture/replay it. The callback runs `generic_eval` (the actual per-op loop) and
127+
stashes its result in `graph_cached_results`. After `execute()` returns, if `ret` is empty (which
128+
happens on a replay, because replaying a captured graph doesn't go through the callback) we substitute
129+
the cached result.
130+
131+
Why the cache: under `hipStreamBeginCapture` the launches are *recorded, not executed*, so the
132+
capture eval returns empty; on replay we return the cached output arguments. Valid because static-
133+
shape decode reuses fixed device buffers. Flag off → `execute()` just runs the loop → byte-identical
134+
to the prior path. **This core-file change is what the maintainer objects to (§5).**
135+
136+
### 2c. Type-erased context — the hook
137+
`src/include/migraphx/context.hpp`
138+
139+
Adds an `execute(run_kernels)` method to the type-erased `context` whose **default just runs the
140+
loop** (`execute_context` free function), so non-GPU targets are unaffected; the GPU context overrides
141+
it with 2a. (This adds a method to a widely-implemented interface — also part of what review flags.)
142+
143+
### 2d. fp16-only gate
144+
`src/targets/gpu/fuse_mlir.cpp` + the `graph_capturable` flag in 2a
145+
146+
hipGraph capture **regresses int4/fp4 decode (measured up to ~2× slower on discrete GPUs)**. The
147+
mechanism behind the regression was **not root-caused** — it was measured (interleaved A/B on navi31
148+
/ STX-Halo, quantized vs fp16) and gated on empirically. So this is a measured-fact gate, not a
149+
mechanistic one; if the gate is ported elsewhere, the regression should be re-confirmed rather than
150+
assumed from a theory. `fuse_mlir::apply` scans the module (pre-lowering, while op names are intact)
151+
and marks the context non-capturable if any quantized/low-bit op is present:
152+
153+
```cpp
154+
static const std::array<std::string,4> low_bit_ops =
155+
{{"unpack_int4","unpack_fp4","dequantizelinear","quant_dot"}};
156+
for(const auto& ins : mpm.get_module())
157+
if(contains(low_bit_ops, ins.name())) { ctx->set_graph_not_capturable(); break; }
158+
```
159+
**What the code does:** during compilation, `fuse_mlir::apply` walks every instruction in the model
160+
(`mpm.get_module()`). If it finds any op whose name is one of the four quantization/low-bit markers
161+
(`unpack_int4`/`unpack_fp4`/`dequantizelinear`/`quant_dot`), it flips the context's `graph_capturable`
162+
flag to false and stops scanning. That flag is the third condition in `is_graph_enabled()` (§2a), so a
163+
quantized model can never enter the capture path — it always runs eagerly. A pure fp16 model contains
164+
none of those ops, so the flag stays true and capture is allowed.
165+
166+
Allowlist-by-absence → every int4 variant (AWQ/RTN, block-32/128) + int8/fp4 stays eager; only fp16
167+
(none of these ops) captures. `is_graph_enabled()` enforces it. Cheap pre-lowering scan; no per-token
168+
or compile cost, none when the feature is off. **This gate is the part of the work most likely to be
169+
additive even if the capture mechanism is replaced (§5).**
170+
171+
### Data flow (end to end)
172+
1. compile: `fuse_mlir` sets `graph_capturable` (false if quantized) on the context.
173+
2. first decode eval: `is_graph_enabled()` true (fp16+flag) → capture loop → instantiate → cache
174+
output args → replay once.
175+
3. subsequent evals: `has_graph()` → single `hipGraphLaunch`, host-side loop skipped.
176+
4. flag off / quantized / cross-compile / capture failure → eager loop, byte-identical to baseline.
177+
178+
## 3. Verification done
179+
180+
| Check | Result |
181+
|---|---|
182+
| Greedy decode output, off vs on (fp16) | byte-identical |
183+
| Gate: int4 program, flag on | marked non-capturable → eager (no regression); confirmed via a temporary capture-engaged stderr marker (since removed) |
184+
| Gate: fp16 program, flag on | captures + replays |
185+
| Builds on the PR base (`develop`) | clean full-target build, exit 0 |
186+
| Default off | byte-identical to prior path |
187+
| New external dependencies | none |
188+
189+
**Measured fp16 steady-state decode throughput, off→on, interleaved same-session A/B, md5-verified
190+
builds** (internal numbers — keep OUT of the public PR; generalized ranges only there):
191+
192+
| Platform | Off→On |
193+
|---|---|
194+
| RX 9070 XT (gfx1201, RDNA4) | Llama-1B ~+24%; DeepSeek-1.5B / Qwen-1.5B ~+18–20% |
195+
| RDNA3 dGPU (navi31) | all four lead models ~+6 to +16% |
196+
| Strix Halo APU (gfx1150) | all four ~+2–3% (memory-bound → less dispatch headroom; expected) |
197+
198+
Win scales with how dispatch-bound the platform is (largest on the highest-bandwidth dGPU).
199+
200+
## 4. Architecture note (why the placement is contested)
201+
202+
The implementation puts capture/replay in **`program::eval`** (core, target-agnostic) + a hook on the
203+
**type-erased context** — for a **GPU-only** feature. It works, but it's the wrong *layer*: capture is
204+
GPU-specific and shouldn't live in core that every target (CPU/ref/gpu) runs through. The MIGraphX-
205+
idiomatic way to express "rewrite the program for the GPU" is a **pass** + a **custom op** (e.g.
206+
`mlss_conv` in this same tree is capture-as-op). See the learning note
207+
`docs/learning-notes/ask-layer-question-before-touching-core.md` in the optimization repo for the full
208+
analysis.
209+
210+
## 5. Maintainer review — current status (READ THIS)
211+
212+
pfultz2 on PR #5019: *"there is no reason to modify `program::eval` as you can just write an op to do
213+
the execution. … see #4956 which implements hip graph and it handles when the pointer change."*
214+
215+
**[#4956 "Add support for HipGraph"](https://github.com/ROCm/AMDMIGraphX/pull/4956)** (pfultz2, DRAFT,
216+
~1483 additions) does the same feature with **zero core/eval changes**:
217+
- a GPU pass **`hipgraphify`** that partitions the module into maximal runs of capturable
218+
instructions and rewrites each into…
219+
- a **`hip::graph` op** (`hip_graph.cpp`) holding the captured `hipGraphExec`, with an
220+
**`exec::update()`** that re-points the graph when buffer pointers change (the invariant we worked
221+
around by caching output args + assuming fixed buffers).
222+
223+
**Implication:** the capture *mechanism* in #5019 is likely superseded by #4956. The PR is unlikely to
224+
merge as-structured. Realistic outcomes:
225+
1. Close #5019 in favor of #4956; OR
226+
2. Contribute #5019's **distinct value — the fp16/int4-gate (§2d)** — onto #4956 (open question:
227+
does #4956's `is_capturable` predicate already exclude quantized runs? needs checking before
228+
claiming the gate is additive); OR
229+
3. Rework #5019 to the pass+op pattern (large, duplicates #4956 — not recommended).
230+
231+
**What's left / next actions for the merge owner:**
232+
1. Get the timeline + intent for #4956 from pfultz2 (a Teams message was being drafted: ask ETA,
233+
whether to close #5019, and whether any piece is worth keeping separate).
234+
2. Verify whether #4956 already handles the quantized-regression case (read its `is_capturable`).
235+
If not, the §2d gate is the thing to land — onto #4956, not as #5019.
236+
3. Upstream CI on #5019 only matters if #5019 stays alive; deprioritize until (1).
237+
238+
## 6. Branches & artifacts
239+
240+
| Item | Value |
241+
|---|---|
242+
| **PR (ours)** | https://github.com/ROCm/AMDMIGraphX/pull/5019 (base `develop`) |
243+
| Superseding PR | https://github.com/ROCm/AMDMIGraphX/pull/4956 (pfultz2, draft) |
244+
| Fork | `aditya-dl/AMDMIGraphX` — https://github.com/aditya-dl/AMDMIGraphX |
245+
| PR branch (fork) | [`amd/dev/adilohia/hipgraph-decode-capture-develop`](https://github.com/aditya-dl/AMDMIGraphX/tree/amd/dev/adilohia/hipgraph-decode-capture-develop) — commit `0647ae162`, 1 commit on `develop`, includes CHANGELOG. This is what PR #5019 is opened from. |
246+
| Internal branch (this README) | [`amd/dev/adilohia/hipgraph-decode-capture`](https://github.com/aditya-dl/AMDMIGraphX/tree/amd/dev/adilohia/hipgraph-decode-capture) — rel-2608 base; holds this README + the code; NOT PR'd. README at [`HIPGRAPH_DECODE_README.md`](https://github.com/aditya-dl/AMDMIGraphX/blob/amd/dev/adilohia/hipgraph-decode-capture/HIPGRAPH_DECODE_README.md) (visible once the latest commit is pushed). |
247+
| Enable flag | `MIGRAPHX_ENABLE_HIPGRAPH=1` (default off) |
248+
| Code commit | `f8b7c6c17` (on both branches) |
249+
250+
## 7. How to reproduce the A/B (internal env)
251+
252+
Same dll for both arms; only the env var differs. Build vs **stock** rocMLIR (the GEMV rocMLIR breaks
253+
int4 compile — unrelated track). Run an fp16 decode model flag-off then flag-on, interleaved; compare
254+
steady-state decode throughput. Quantized models should show ~no change (gated → eager). Output must
255+
be byte-identical off vs on.

0 commit comments

Comments
 (0)