split_single_dyn_dim: add bucket_by_optimals to cut dyn-shape compile…#5028
split_single_dyn_dim: add bucket_by_optimals to cut dyn-shape compile…#5028chun-wan wants to merge 3 commits into
Conversation
|
Thank you for your contribution! Since this is an external pull request, a maintainer must review PR and add the "ok-to-test" label if it is approved for testing. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5028 +/- ##
===========================================
+ Coverage 92.89% 92.91% +0.02%
===========================================
Files 603 603
Lines 32448 32571 +123
===========================================
+ Hits 30140 30261 +121
- Misses 2308 2310 +2
🚀 New features to boost your workflow:
|
|
This is the lower-risk half of #4911, split out per review feedback so it can land independently. It contains only the compile-time bucketing: split_single_dyn_dim{.bucket_by_optimals} (a pass parameter, driven directly in tests) plus select_module's smallest-compatible-bucket dispatch with host padding. The per-instance dispatch cache from #4911 is intentionally not here — it'll be a separate follow-up PR. All CI is green except security_gate (fork-only). @pfultz2 @CharlieL7 could you review? Thanks! |
1da50bb to
bfd2499
Compare
… cost Add a bucket_by_optimals field to split_single_dyn_dim: emit one submodule per (min, optimals..., max) instead of one per integer in [min,max], cutting compile time and .mxr size from O(max-min) to O(|optimals|). select_module dispatches to the smallest compatible bucket at runtime (ref pads inputs on the host; GPU callers pre-pad). GPU/Ref targets read MIGRAPHX_DYN_DIM_BUCKET_BY_OPTIMALS and forward it to the pass field. Adds split_single_dyn_dim and select_module bucket-dispatch tests. Split out of ROCm#4911; the per-instance select_module dispatch cache is a separate follow-up PR.
bfd2499 to
6b9ae29
Compare
|
This is the low-risk, bucket-only split out of #4911: just It collapses dynamic-shape compile from O(max−min) → O(|optimals|) — measured on a customer model: @CharlieL7 @kahmed10 could one of you review when you have a chance? 🙏 |
CharlieL7
left a comment
CharlieL7
left a comment
There was a problem hiding this comment.
The way the padding is handled is questionable with using std::memset and std::memcpy in select_module. I recall Khalique had a fixed_pad operator and that basically solves that issue. What's the reason for this design?
| | (plus the ``min`` / ``max`` endpoints), instead of one | ||
| | per integer in ``[min, max]``. O(|optimals|) compile | ||
| | cost. | ||
| | * Read once per compile in the GPU/Ref target and forwarded |
There was a problem hiding this comment.
| | * Read once per compile in the GPU/Ref target and forwarded | |
| | * Read once per compile in the GPU target and forwarded |
The ref target should not be running the split_single_dyn_dim pass.
| * Added slice squeeze matcher to propogate squeeze downstream and allow for parallel branches to merge together (#5004) | ||
| * Added GPU kernel for ONNX `NonMaxSuppression` operation and redesigned the `nonmaxsuppression` operation to better represent the data-dependent output shape in the MIGraphX IR (#4893). | ||
| * Added mixed length gather fusion in same_table_gather_horizontal_fusion to bundle gather kernels that share the same data (#5044). | ||
| * `split_single_dyn_dim` gains a `bucket_by_optimals` field: when true and `dynamic_dimension::optimals` is non-empty, emit one submodule per (min, optimals..., max) instead of one per integer in `[min, max]` (O(|optimals|) vs O(max-min) compile cost / engine size). `select_module::compute()` always falls back to the smallest compatible bucket on a non-exact runtime shape (ref pads on host, GPU callers pre-pad). GPU/Ref targets read `MIGRAPHX_DYN_DIM_BUCKET_BY_OPTIMALS` once and forward it to the pass field; see `docs/reference/MIGraphX-dev-env-vars.rst`. |
There was a problem hiding this comment.
Same issue, ref doesn't run split_single_dyn_dim.
CharlieL7 review: the ref target does not run split_single_dyn_dim (its target.cpp change in this PR is only a copyright-year bump; only the GPU target reads the env var). Correct the dev-env-vars doc and CHANGELOG wording from "GPU/Ref" to "GPU target".
…eview) CharlieL7 review: express bucket padding as an in-graph op instead of a host-side memset/memcpy in select_module::compute(), so padding is device-agnostic and GPU callers no longer need to pre-pad. split_single_dyn_dim (bucket mode) now builds each dim_<N> submodule with a dynamic input narrowed to (prev, N] and prepends Khalique's fixed_pad op (already in develop) to expand it to a static [N, ...] shape. select_module dispatches to the smallest compatible bucket and forwards the runtime argument unchanged; fixed_pad performs the padding on-device. Legacy (non-bucket) enumeration is unchanged. Updates ref/select_module and split_single_dyn_dim tests to the fixed_pad-based buckets and drops the pre-pad caveat from CHANGELOG / dev-env-vars docs.
2 participants
split_single_dyn_dimgains abucket_by_optimalspass field. When true anda dynamic dimension has non-empty
optimals, it emits one static submoduleper (min, optimals..., max) instead of one per integer in [min, max].
select_module::compute()first tries an exact-shape match, thenfalls back to the smallest compatible submodule (same type/rank, every bucket
dim >= the input dim). On the ref backend the input is zero-padded into the
bucket-sized buffer on the host; GPU callers pre-pad.
MIGRAPHX_DYN_DIM_BUCKET_BY_OPTIMALSonce and forward itto the pass field. The field is a normal pass parameter, so unit tests set it
directly via
run_pass(...)rather than env vars.test/split_single_dyn_dim_test.cpp(bucketed enumeration) andtest/ref/select_module.cpp(exact match / round-up-with-host-pad /too-big-throws).
select_moduledispatch cache (a pureruntime optimization) is a separate follow-up PR.