feat(hipsr): add pool-alloc greedy grouping - #605
Merged
Conversation
Place each alloc into the first group whose members all have lifetimes disjoint from it, opening a new group when none fits. Touching endpoints count as overlapping: the op that first writes the later buffer is still reading the earlier one. Allocs are collected in block order and stable-sorted by lifetime start, so group numbering does not depend on DenseMap iteration order. Grouping stays analysis only; the pass still rewrites nothing. The emit-pool-report remark now reports "lifetime [a,b] group N".
zz002
requested review from
amd-mingw,
edelaye,
fhanuman,
qianglin-amd and
wcy123
as code owners
July 31, 2026 03:27
|
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: 3452 - Commit: |
MorphiZen EP Performance Results
EPContext Export Performance
EPContext Import Performance
OGA Benchmark Results
OGA Wheel Smoke (Python benchmark_e2e.py)
Run: 3452 - Commit: |
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
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 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; theemit-pool-reportremark gained the group each alloc landed in.Related issue or design
Upstream
wcy123/onnx-hipdnn-ep#110(greedy grouping utility), a step ofwcy123/onnx-hipdnn-ep#19, which links the HIPSR Pool Allocation Pass design page. Follows #591 (#109, liveness).Why
Group numbering ends up in the report, and
#117will 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 theValue->Lifetimemap, whose order is pointer-hash dependent; and the sort by lifetime start is stable, because allocs feeding severaloutsof 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 tocomputeLivenessin the same anonymous namespace.emitPoolingReporttakes the groups and reportslifetime [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
check-hip-mlir-lit— 361 passed, 3 unsupported, 0 failed.group Nfails the run, so the new assertions are not vacuous.pre-commit run --all-filesclean.Grouping is covered by chromatic number of the interval graph:
coalesce_static(four disjoint allocs of differing sizes, one group),interleaved_allocsandhoisted_allocs(one alloc over two disjoint ones, two groups),split_three_groups(pairwise overlap, three groups).Notes for reviewers
coalesce_staticandsplit_three_groupsare lifted verbatim from the reference implementation in #572, minus itsCHECKlines, which assert the pooled IR that does not exist yet. Keeping the fixtures identical means#117adds assertions instead of rewriting tests. They live inpool_alloc_report.mlirfor now because a remark is the only thing to assert before the rewrite is wired;#117moves them topool_alloc.mlir.Blockand itsValues, which gtest would have to build by hand, and the RUN line's--implicit-check-not=hipsr.get_pool --implicit-check-not=memref.viewdoubles as proof the pass is still a no-op. Happy to add a unit test if reviewers prefer one.greedyGroupingruns unconditionally and its result is only read underemit-pool-report, matching howcomputeLivenesslanded in#109.#117consumes it for real.Checklist
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.