feat(hipsr): add pool-alloc liveness analysis - #591
Merged
Conversation
Computes each pool domain's alloc live ranges: start is the first op writing the buffer as a DPS `outs`, end its last use, both in indices of the domain block's own ops. Users nested in a region fold to their enclosing block-level op, so a lifetime is never cut short by an index the block does not know. The pass still rewrites nothing. A new `emit-pool-report` option reports each range as a remark so LIT can assert it; grouping, sizing, pool emission and the full report land in the follow-up issues. Upstream issue: wcy123/onnx-hipdnn-ep#109 Co-authored-by: Cursor <cursoragent@cursor.com>
zz002
requested review from
amd-mingw,
edelaye,
fhanuman,
qianglin-amd and
wcy123
as code owners
July 30, 2026 07:55
|
Thanks for opening a PR! This project follows LLVM's incremental-development and AI-tool-use Before requesting review, please check that:
Reviewers are assigned through |
L2 Accuracy Results (EP vs CPU)
Threshold: 0.01 | Run: 3398 - Commit: |
MorphiZen EP Performance Results
EPContext Export Performance
EPContext Import Performance
OGA Benchmark Results
OGA Wheel Smoke (Python benchmark_e2e.py)
Run: 3398 - Commit: |
7 tasks
zz002
added a commit
that referenced
this pull request
Jul 31, 2026
## Summary `-hipsr-pool-alloc` now groups each pool domain's allocs by lifetime: an alloc joins the first group whose members are all disjoint from it, otherwise it opens a new one. The pass still rewrites nothing; the `emit-pool-report` remark gained the group each alloc landed in. ## Related issue or design Upstream [`wcy123/onnx-hipdnn-ep#110`](wcy123/onnx-hipdnn-ep#110) (greedy grouping utility), a step of [`wcy123/onnx-hipdnn-ep#19`](wcy123/onnx-hipdnn-ep#19), which links the HIPSR Pool Allocation Pass design page. Follows [#591](#591) (`#109`, liveness). ## Why Group numbering ends up in the report, and `#117` will turn it into pool offsets, so it has to be reproducible. Two things make it so: allocs are collected by walking the block rather than iterating the `Value` -> `Lifetime` map, whose order is pointer-hash dependent; and the sort by lifetime start is stable, because allocs feeding several `outs` of one DPS op share a start and an unstable sort would order them arbitrarily. Touching endpoints count as overlapping. The op that first writes the later buffer is still reading the earlier one, so `[1,3]` and `[3,5]` cannot share space. ## What - `greedyGrouping(Block &, const DenseMap<Value, Lifetime> &)` returning groups as vectors of allocs, next to `computeLiveness` in the same anonymous namespace. - `emitPoolingReport` takes the groups and reports `lifetime [a,b] group N`. Per-domain totals (alloc and group counts, reuse ratio) belong to `#118`. - `pool_alloc_report.mlir`: existing ranges gained their group, plus the two boundary topologies. ## Test plan - [x] Full `check-hip-mlir-lit` — 361 passed, 3 unsupported, 0 failed. - [x] Mutation check: flipping one expected `group N` fails the run, so the new assertions are not vacuous. - [x] `pre-commit run --all-files` clean. Grouping is covered by chromatic number of the interval graph: `coalesce_static` (four disjoint allocs of differing sizes, one group), `interleaved_allocs` and `hoisted_allocs` (one alloc over two disjoint ones, two groups), `split_three_groups` (pairwise overlap, three groups). ## Notes for reviewers - `coalesce_static` and `split_three_groups` are lifted verbatim from the reference implementation in [#572](#572), minus its `CHECK` lines, which assert the pooled IR that does not exist yet. Keeping the fixtures identical means `#117` adds assertions instead of rewriting tests. They live in `pool_alloc_report.mlir` for now because a remark is the only thing to assert before the rewrite is wired; `#117` moves them to `pool_alloc.mlir`. - Tests are lit rather than gtest: the inputs are a `Block` and its `Value`s, which gtest would have to build by hand, and the RUN line's `--implicit-check-not=hipsr.get_pool --implicit-check-not=memref.view` doubles as proof the pass is still a no-op. Happy to add a unit test if reviewers prefer one. - `greedyGrouping` runs unconditionally and its result is only read under `emit-pool-report`, matching how `computeLiveness` landed in `#109`. `#117` consumes it for real. ## Checklist - [x] The change is focused, or links a design/series explaining its scope. - [x] Relevant tests were added or updated and the results are documented. - [x] User-facing or design documentation was updated when needed. - [x] Substantial AI assistance is disclosed, and I reviewed and understand the result. AI assistance: Cursor implemented `greedyGrouping`, ported the two fixtures, and ran the lit and pre-commit validation above; I reviewed the result and hand-checked every expected lifetime and group against the fixture IR.
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
2 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.
Summary
-hipsr-pool-allocnow computes each pool domain's alloc live ranges. The pass still rewrites nothing; a newemit-pool-reportoption reports each range as a remark so lit can assert it.Related issue or design
Upstream
wcy123/onnx-hipdnn-ep#109(liveness analysis utility), a step ofwcy123/onnx-hipdnn-ep#19, which links the HIPSR Pool Allocation Pass design page. Follows #582 (#108, skeleton).Why
A live range starts at the first write, not at the alloc. After materialize-init-tensors the allocs sit together at the top of the domain, so ranges keyed on the alloc index all overlap and nothing can ever share pool space. Taking the first DPS
outswrite as the start keeps disjoint ranges disjoint, which is what makes the grouping in the follow-ups worth doing.Indices are those of the domain block's own ops, so a user nested in a region has to fold to its enclosing block-level op —
Block::findAncestorOpInBlock, asPartitionPoolDomains.cppalready does. Looking the nested op up directly returns index 0 and silently shortens the range, which would later let a live buffer be reused.hipsr.pool_domainisIsolatedFromAbove, so failing to find an ancestor means the IR broke that invariant: an assert, not a fallback branch.emit-pool-reportis the option name the whole series uses, not test scaffolding:#110-#118add group, size and reuse figures to the same report.What
Passes.td:emit-pool-reportbool option, default false, worded after the legacyhip-pool-allocsemit-fragmentation-report.HipsrPoolAllocPass.cpp:LifetimepluscomputeLiveness(Block &)returning aValue->Lifetimemap, andemitPoolingReport()which walks the block so remark order is block order. An alloc with no DPSoutswrite has no range and is absent from the map; the warning for that case belongs to#118.test/lit/Dialect/Hipsr/pool_alloc_report.mlir: one RUN line,--verify-diagnosticsfor the ranges andFileCheck --implicit-check-notfor the unchanged module.Not done: alias-aware last use. The legacy
BufferUtils.htracks view-like aliases throughBufferViewFlowAnalysisbecausehipdomains contain them; hipsr domain bodies do not yet. This has to be revisited before#117wires the rewrite into a bufferized pipeline, otherwise an aliased use shortens the range.Test plan
lit --filter pool_alloc—pool_alloc_report.mlirandpool_alloc.mlirPASS.check-hip-mlir-lit— 356 passed, 2 unsupported, 0 failed.hip-mlir-opt --helplists--emit-pool-reportunder--hipsr-pool-alloc.hipsr.get_poolinto a domain trips--implicit-check-not, so neither half of the RUN line is vacuous.pre-commit run --all-filesclean.Notes for reviewers
Lifetimecarries onlystart/end. The endpoint ops are only needed by#118's debug trace, so they land with it.#110-#118, each on top of the previous one.Checklist