Skip to content

Feat/devalshah/generic splitter - #1026

Merged
kyle-hoffmeyer merged 30 commits into
feat/khoffmey/split_refactor_genericfrom
feat/devalshah/generic_splitter
Sep 15, 2026
Merged

kyle-hoffmeyer merged 30 commits into
feat/khoffmey/split_refactor_genericfrom
feat/devalshah/generic_splitter

Conversation

@devalshahamd

Copy link
Copy Markdown
Contributor

Making some changes to simplify the code and remove incorrect/redundant logic

Pull Request Template

Note to AMDers:
This is a public repository. Please do not upload any confidential or customer data. Make sure all such data has been anonymized or removed before making this PR. If you need to attach any private files or links, please insert a Internal OneDrive Link or a Jira Ticket Link instead.

devalshah-amd and others added 2 commits September 14, 2026 18:43
The merge brought in bookend promotion, which adopts the leading and
trailing regions of a periodic run as iterations when their GPU signature
matches. Nothing exercised it, so a regression would only have surfaced
as a coverage drop on the corpus.

Co-authored-by: Cursor <cursoragent@cursor.com>
Each denoising step is preceded by a to/expand/to prep group on the
timestep tensor, and only the last of the three carries GPU work. The
previous references started an iteration at that last call, so the two
CPU-side calls were credited to the step before the one they prepare.
Blocks now anchor on the first child whose name carries GPU time, which
keeps the group with the step it feeds.

GPU attribution is unchanged: the kernel and memcpy lists are identical
and GPU busy time per split matches to 0.000us. Only ~34us of CPU frames
move, and the manifest still reports 6 iterations with the same warmup
and wrapup roles.

Co-authored-by: Cursor <cursoragent@cursor.com>
@kyle-hoffmeyer

kyle-hoffmeyer commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

@devalshahamd

  • Removed promote_bookends_iterations -> periodic_candidates and grouped_candidates are able to find all iterations.
  • Simplified identification of gpu paths by relying exclusively on non_gpu_path field.
  • Added tests for steady state identification and phase division:
    • Runs steady state identification + phase division on LLM inference traces- with annotations then again with annotations stripped
    • Passes if:
      • Number of splits is the same between each steady state category / phase
      • Number of kernels per split is the same within 5% tolerance
  • Modified batch size identification to only consider non-memory ops when extracting the most common input dimension. This prevents false positives where KV cache initialization and cleanup operations are included in the calculation.
  • Fixed steady state identification for LLM inference without annotations to not require minimum number of steady state splits
  • Steady state identification adds prefill iterations at the end of the window rather than just the ones within the window

@kyle-hoffmeyer

kyle-hoffmeyer commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Given list of candidates:
Iter 0 (prefill): 0 run_batch [GPU] 3 pop_and_process [GPU] 9 get_next_batch_to_run [GPU]
Iter 1: 11 run_batch [GPU] 14 pop_and_process [ ] 20 get_next_batch_to_run [GPU]
Iter 2: 22 run_batch [GPU] 25 pop_and_process [ ] 31 get_next_batch_to_run [GPU]
Iter 3: 33 run_batch [GPU] 36 pop_and_process [ ] 42 get_next_batch_to_run [GPU]
Iter 4: 44 run_batch [GPU] 47 pop_and_process [ ] 53 get_next_batch_to_run [GPU]
Iter 5: 55 run_batch [GPU] 58 pop_and_process [GPU] 64 get_next_batch_to_run [ ]

periodic_candidate in feat/khoffmey filters out the non gpu path events. Meaning iter 0's pop_and_process is included but the iter's pop_and_process gets filtered out. This means the pattern it finds is get_next_batch_to_run -> run_batch and iter 0 is left out (since it starts with run_batch, not get_next_batch_to_run).

periodic_candidate in feat/devalshah branch however works. It only filters out events that are never gpu paths. So since pop_and_process is sometimes a gpu path, all invocations are included, meaning the recurring pattern is very clean: run_batch -> pop_and_process -> get_next_batch_to_run. So grouped_candidate actually isn't needed for this example.
However, lets say we remove pop_and_process altogether for iters 1-5. In this case, periodic_candidate fails but grouped_candidate identifies run_batch as the gpu bearing event and all 6 iters are picked up.

So either way, the bookend enhancement isn't needed. periodic_candidate finds everything and if grouped_candidate works correctly if periodic_candidate were to fail.

@kyle-hoffmeyer

Copy link
Copy Markdown
Collaborator

Regarding the prune_nongpu_path flag, it just adds a non_gpu_path flag like we discussed. But nothing in trace splitter actually checks this. gpu_bearing func just checks if gpu_events is set / kernel_bearing is set (a field assigned in reattach_worker_threads I should delete). It doesn't check non_gpu_path.

So we can just simplify this by making reattach_worker_threads assign non_gpu_path arg to events if prune_nongpu_path is true (instead of kernel_bearing field), and then in trace splitter we just check non_gpu_path arg and nothing else.

@kyle-hoffmeyer
kyle-hoffmeyer merged commit 16085ba into feat/khoffmey/split_refactor_generic Sep 15, 2026
1 check passed
@kyle-hoffmeyer
kyle-hoffmeyer deleted the feat/devalshah/generic_splitter branch September 15, 2026 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants