Skip to content

Add sliding-window attention tuning support#2439

Open
umangyadav wants to merge 1 commit into
developfrom
users/umayadav/sliding-window-tuning
Open

Add sliding-window attention tuning support#2439
umangyadav wants to merge 1 commit into
developfrom
users/umayadav/sliding-window-tuning

Conversation

@umangyadav

Copy link
Copy Markdown
Member

Summary

  • Include sliding_window_size in Rock and Python attention tuning identities, reports, and quick-tuning grouping.
  • Propagate current_seq_len through runtime rocmlir-gen commands while deliberately excluding it from tuning keys.
  • Exercise sliding-window configurations in attention sweeps and add rocmlir-gen, MIGraphX, legacy tuning-DB, and mixed-TSV regression coverage.

Bugs fixed and error signatures

Sliding-window tuning-key collisions

The Rock tuning key previously omitted slidingWindowSize. A windowed problem therefore emitted the same sequence around -split_kv 1 -num_heads_q ... as a non-windowed problem, and different window sizes also shared one identity. Tuning databases could consequently return a stale or suboptimal perf configuration for a materially different attention kernel.

The key now conditionally emits -sliding_window_size <N> after -split_kv. A disabled window remains omitted, so existing non-windowed tuning keys are unchanged.

Python tuning-key and report mismatch

Once the C++ key contains the window, the old Python parser fails with:

ValueError: Unknown Attention config argument -sliding_window_size -> <N>

Python reports and quick-tuning grouping also lacked a window column, which merged distinct window sizes into one problem identity. Older TSVs would either raise KeyError: 'SlidingWindowSize', or—when concatenated with newer TSVs—receive NaN and be silently dropped by pandas groupby.

AttentionConfiguration, report columns, and quick-tuning columns now carry SlidingWindowSize; missing and NaN legacy values normalize to the disabled value 0.

Lost runtime sequence positions

current_seq_len is a runtime KV-cache position, not a tuning dimension. Previously it was appended only inside parameterSweeps.test_config; commands and repro strings produced directly by AttentionConfiguration omitted it. A sliding-window command reconstructed through that path could fail Rock verification with:

slidingWindowSize requires currentSeqLen to be set

The configuration now owns runtime argument emission, so current_seq_len appears exactly once in generated driver commands. It remains absent from to_command_line() and C++ tuning keys, preventing per-position tuning-key fragmentation.

Missing sweep and fusion coverage

Attention parameter sweeps never generated sliding-window cases, and the MIGraphX problem-key path did not verify that a folded sliding-window mask contributes its window—but not its runtime sequence position—to the key. The sweep now covers both plain and sliding-window KV-cache problems, and focused regression tests pin both key behavior and legacy compatibility.

Python lint gate

Changing perfRunner.py activates the repository's full-file flake8 check. Two pre-existing inline dictionary expressions triggered E201/E231; they are normalized without changing behavior so the Python lint gate remains clean.

Compatibility

  • Non-sliding attention keys remain byte-for-byte unchanged.
  • Legacy tuning TSVs without SlidingWindowSize continue to load as window size 0.
  • current_seq_len remains runtime-only and never enters tuning DB identity or reports.

Test plan

  • ninja -C build rocmlir-gen rocmlir-driver
  • Focused lit tests: rocmlir-gen problem key, MIGraphX sliding-window problem key, and attention tuning-DB compatibility
  • python3 -m pytest -q mlir/utils/performance/tests (126 passed)
  • C++ premerge clang-format and clang-tidy
  • Python flake8, YAPF, and syntax compilation
  • git diff --check origin/develop

Made with Cursor

Include window size in tuning identity and performance tooling while keeping current sequence length runtime-only.

Co-authored-by: Cursor <[email protected]>
@umangyadav
umangyadav requested a review from causten as a code owner July 23, 2026 20:33
@umangyadav umangyadav self-assigned this Jul 23, 2026
@umangyadav umangyadav added the claude-review Trigger automated PR review by claude[bot]; auto-removed after the run. label Jul 23, 2026
@rocmlir-pr-reviewer rocmlir-pr-reviewer Bot removed the claude-review Trigger automated PR review by claude[bot]; auto-removed after the run. label Jul 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class sliding-window attention support to rocMLIR tuning identities and the Python performance tooling, ensuring sliding-window kernels are keyed distinctly while keeping current_seq_len runtime-only.

Changes:

  • Extend Rock (C++) and Python attention tuning identities/reporting to include sliding_window_size, preserving legacy non-window keys.
  • Plumb current_seq_len into runtime rocmlir-gen command generation while excluding it from tuning keys/DB identities.
  • Add/extend regression coverage across rocmlir-gen tuning keys, MIGraphX fusion keys, tuning-DB compatibility, and quick-tuning TSV handling.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
mlir/utils/performance/reportUtils.py Adds SlidingWindowSize to attention report/test parameter columns.
mlir/utils/performance/perfRunner.py Extends AttentionConfiguration with sliding_window_size + current_seqlen; emits them in runtime driver args (but not tuning keys).
mlir/utils/performance/parameterSweeps.py Removes manual current_seq_len append since runtime arg emission moves into configs.
mlir/utils/performance/attentionSweeps.py Generates and propagates sliding-window samples into attention sweep configurations.
mlir/utils/performance/analysis/quickTuningGen.py Adds SlidingWindowSize to grouping columns and defaults legacy/mixed TSVs to 0.
mlir/test/rocmlir-gen/problem-key.mlir Pins tuning-key behavior: window participates in key; current_seq_len does not.
mlir/test/perf-scripts/attention-tuning-db-compat.py Expands compatibility tests for sliding-window keys and runtime-only current_seq_len.
mlir/test/fusion/mixr-attention-sliding-window-problem-key.mlir Adds MIGraphX fusion tuning-key regression for sliding-window attention.
mlir/lib/Dialect/Rock/Tuning/RockTuningImpl.cpp Adds conditional -sliding_window_size <N> emission to Rock attention tuning keys.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mlir/utils/performance/perfRunner.py
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mlir/lib/Dialect/Rock/Tuning/RockTuningImpl.cpp 66.67% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2439      +/-   ##
===========================================
+ Coverage    82.57%   83.60%   +1.04%     
===========================================
  Files          120      120              
  Lines        42852    42936      +84     
  Branches      7110     7140      +30     
===========================================
+ Hits         35381    35896     +515     
+ Misses        4815     4482     -333     
+ Partials      2656     2558      -98     
Flag Coverage Δ
gfx950 83.36% <66.67%> (+1.01%) ⬆️
mfma 83.38% <66.67%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
mlir/lib/Dialect/Rock/Tuning/RockTuningImpl.cpp 60.81% <66.67%> (+1.49%) ⬆️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants