feat(hipsr): lifetime-based pool allocation (single group) - #572
Draft
zz002 wants to merge 13 commits into
Draft
Conversation
zz002
requested review from
amd-mingw,
edelaye,
fhanuman,
qianglin-amd and
wcy123
as code owners
July 28, 2026 02:07
|
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: 3352 - Commit: |
MorphiZen EP Performance Results
EPContext Export Performance
EPContext Import Performance
OGA Benchmark Results
OGA Wheel Smoke (Python benchmark_e2e.py)
Run: 3352 - Commit: |
Fill in the hipsr-pool-alloc pass: pool the memref.alloc buffers inside a pool_domain whose lifetimes do not overlap into one hipsr.get_pool, replacing each alloc with a memref.view at offset 0. Lifetime start = first write (DPS outs), end = last use. Multi-group inputs error out for now.
Split processDomain into per-step helpers (collectAllocLifetimes, greedyGroup, findContext, findLastAlloc, emitGroupSize, emitPool, replaceAllocsWithViews) mirroring the design-doc phases; inline single-use size/align emitters and the overlaps predicate. Make a poolable domain without an !hipsr.context operand a hard pass failure instead of a silent no-op. Remove code comments per request.
Drop the single-group bail in processDomain and emit N groups into one device pool: poolSize = sum of aligned group sizes (chained arith.addi), each group i placed at a cumulative offset (off0 = 0, off_i = off_{i-1} + size_{i-1}), members of one group sharing that offset. The single-group path stays byte-for-byte identical (poolSize has no addi, offset is a bare constant 0), so the existing positive cases do not regress.
Add pool_alloc_multi_group.mlir (two-group offset chain, intra-group reuse plus cross-group separation, three-group topology, no size-bucketing) and drop the now-obsolete multi-group expected-error case from pool_alloc_single_group.mlir.
…euse Add multi_dynamic (dynamic alloc size flowing through the poolSize/offset addi chains alongside a static group) and multi_bigroup_mixed (a three-member mixed-dtype group folded by two maxui plus a fourth overlapping group) to pool_alloc_multi_group.mlir, and single_many_members (four disjoint allocs coalescing into one group sharing a single offset) to pool_alloc_single_group.mlir.
zz002
force-pushed
the
feat/hipsr-pool-alloc-skeleton
branch
from
July 28, 2026 07:55
af7bc54 to
69dbc05
Compare
Per-op headers were consolidated into HipsrOps.h; switch the pool-alloc pass to the aggregate header like the rest of the dialect so it builds after rebasing onto main.
emitGroupSize becomes emitGroupSizes returning every group's aligned size, and emitPool accumulates the total pool size itself. findContext keeps the last matching context arg and view replacement uses llvm::zip. No IR/behavior change.
…ed file Merge the single/multi-group files into pool_alloc.mlir ordered by lifetime-interval chromatic number. Rename cases by behavior (coalesce_* for disjoint reuse vs split_* for overlapping groups) and shape (static/dynamic/mixed); split_with_coalesced_group now carries a dynamic member so one multi-group domain crosses dynamic size and mixed dtype.
Gate the per-domain effectiveness remark behind a new emit-pool-report option, following the hip-pool-allocs convention that reports are opt-in while only real anomalies report unconditionally: an alloc with no DPS write is now warned about instead of silently skipped. The remark carries allocation/group/reuse counts, plus pooled vs naive bytes and intra-group slack when every extent is constant; the naive baseline aligns each alloc separately so the ratio compares like with like and never reports a negative saving. Also add DEBUG_TYPE, four STATISTIC counters, and LLVM_DEBUG traces for lifetime intervals and grouping conflicts. Refresh the pass description, which still claimed multiple non-overlap groups error out. New pool_alloc_report.mlir asserts the remark wording and that the option leaves the IR unchanged. pool_alloc.mlir keeps its -verify-diagnostics guard that the default path emits no diagnostics, and gains a dead-alloc case asserting the unpoolable alloc is left alone and excluded from the pool size.
zz002
marked this pull request as draft
July 29, 2026 06:36
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
1 participant
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
Implement the single-group core of the new
hipsr-pool-allocpass: within eachhipsr.pool_domain,memref.allocbuffers whose lifetimes do not overlap collapse into onehipsr.get_poolbyte buffer, each alloc rewritten to amemref.viewat offset 0. Also lands the no-op pass registration (skeleton) and lit coverage.Related issue or design
Design:
pool-allocs-design-cnHIPSR Pool Allocation Pass + PR9 breakdown §9a (single-group). Series: PR 9 (remove SSA-identity bucketing, lifetime-only grouping).Why
The old
hipPoolAllocspre-buckets by SSA identity before grouping. This replaces that with pure lifetime grouping so buffers that are only sequentially live — different SSA values, different sizes — can share the same memory. Lifetimestartuses the first write (DPSouts), not the alloc index, so grouping is robust to allocs being hoisted adjacent.What
-hipsr-pool-alloc(Passes.tddef + pass; auto-registered viaGEN_PASS_REGISTRATION).start= first write (DPSouts),end= last use, by op index.>1group emits an error andsignalPassFailure(single-group bail; multi-group is the follow-up).elemBytes × Π(static dims) × Π(dynamic-size operands),arith.maxuiacross allocs,alignUp(256);hipsr.get_pool(ctx, size); each alloc becomes amemref.viewat offset 0.ctxcomes from thepool_domainblock arg (it isIsolatedFromAbove).shape_ofon the buffer), so it stays valid after the alloc is replaced/erased; views are emitted after the pool to preserve dominance.Test plan
llvm-lit test/lit/Dialect/Hipsr/pool_alloc_single_group.mlirgreen — first-write disjoint → 1 group,alignUp(256), single alloc, dynamic dim (muliby the size operand), static/dynamic + different element type in one group, no-alloc no-op unchanged, and>1group →emitError(-verify-diagnostics).pre-commit run --all-filesclean.Notes for reviewers
kPoolAlignment(256 B) is compiler-owned for GPU coalesced access. Runtimehipdnn_ep_get_pool_basedoes not validate offsets — thehipMallocbase is already 256-aligned, so offset-256 keeps every view hipMalloc-grade aligned. Unlike the oldhippass it is a constant here rather than a pass option (can be lifted to an option later if needed).pool_domainfixtures.Checklist