Skip to content

fix(rocprofiler-sdk): stop replay passes from consuming kernel iterations - #11335

Merged
amd-vkale merged 4 commits into
developfrom
users/vkale/fix-replay-iteration-range-with-cursor-fixes
Oct 9, 2026
Merged

amd-vkale merged 4 commits into
developfrom
users/vkale/fix-replay-iteration-range-with-cursor-fixes

Conversation

@amd-vkale

@amd-vkale amd-vkale commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

--kernel-iteration-range selects application-observed kernel launches. Kernel replay creates
multiple executions of one logical dispatch so profiling can collect a different group on each
pass. Before this fix, counter_dispatch_callback consulted the stateful iteration filter on every
pass. The filter therefore mistook replay passes for new application iterations.

The validation report reproduced this with independent HIP and Kokkos workloads. Both the
application and rocprofv3 exited successfully, with no warning, even though the resulting counter
records were incomplete and appeared valid.

For example, with iteration range 2-3 and two replay passes, the expected output is two selected
logical dispatches, each containing passes [0, 1]: four counter records in total. Instead, the
filter counted each pass as an iteration. Pass 1 from one logical dispatch and pass 0 from the next
were selected separately, leaving both dispatches incomplete. Numeric dispatch IDs may vary; the
missing pass in each dispatch is the failure.

Intuition: one dispatch, several physical executions

Profiling hardware cannot always measure every requested event simultaneously, so tools divide the
request into groups and execute the same dispatch once per group. That creates two different units:

  • A logical dispatch is the launch the application requested. Kernel names, launch filters, and
    iteration ranges describe this unit.
  • A replay pass is one physical execution used to collect one profiling group. Pass selection
    and per-pass configuration describe this unit.

The bug came from applying a logical-dispatch filter at the replay-pass boundary.

flowchart TB
  subgraph beforeFix [Before: filter evaluated per replay pass]
    launch1Before["Logical launch 1"]
    launch1Pass0["Pass 0 calls filter; iteration 1; rejected"]
    launch1Pass1["Pass 1 calls filter; iteration 2; selected"]
    incomplete1["Dispatch D1 contains pass 1 only"]
    launch2Before["Logical launch 2"]
    launch2Pass0["Pass 0 calls filter; iteration 3; selected"]
    launch2Pass1["Pass 1 calls filter; iteration 4; rejected"]
    incomplete2["Dispatch D2 contains pass 0 only"]
    launch1Before --> launch1Pass0 --> launch1Pass1 --> incomplete1
    launch2Before --> launch2Pass0 --> launch2Pass1 --> incomplete2
  end

  subgraph afterFix [After: filter evaluated per logical dispatch]
    launch1After["Logical launch 1; filter iteration 1; rejected"]
    launch2After["Logical launch 2; filter iteration 2; selected"]
    launch2Passes["Passes 0 and 1 reuse selected"]
    complete2["Dispatch D2 contains passes 0 and 1"]
    launch3After["Logical launch 3; filter iteration 3; selected"]
    launch3Passes["Passes 0 and 1 reuse selected"]
    complete3["Dispatch D3 contains passes 0 and 1"]
    launch1After --> launch2After --> launch2Passes --> complete2
    launch2After --> launch3After --> launch3Passes --> complete3
  end
Loading

Expected record shape after the fix:

iteration range 2-3, two profiling groups

logical launch 1  -> rejected
logical launch 2  -> dispatch D2: [pass 0, pass 1]
logical launch 3  -> dispatch D3: [pass 0, pass 1]

total: 2 selected dispatches x 2 passes = 4 counter records

Code map:

  • The stateful iteration filter advances the per-kernel count:
    tool.cpp L391-L409.
  • The replay pass and cached dispatch decision are thread-local:
    tool.cpp L1641-L1657.
  • Pass 0 consults the filter and later passes reuse the result:
    tool.cpp L2023-L2036.
  • CONFIG entry clears the previous dispatch decision; PASS entry/exit publishes and clears the
    authoritative pass index:
    tool.cpp L2561-L2593.

Technical Details

The iteration-range decision now belongs to the dispatch:

  1. CONFIG entry resets tl_replay_dispatch_targeted, preventing the previous dispatch's decision
    from leaking into the next replay loop.
  2. Pass 0 calls is_targeted_kernel(), advancing the per-kernel iteration count exactly once, and
    caches the result.
  3. Later passes reuse the cached result and do not advance the iteration count.
  4. Non-replayed dispatches continue to call the filter normally.

Replay PASS callbacks and the counter dispatch callback execute synchronously and in order on the
enqueuing thread, so the decision can be stored thread-locally without a process-wide map or lock.

Issue Tracking

JIRA ID: AIPROFSDK-1130

Test Plan

Adds an integration test combining kernel replay with --kernel-iteration-range. It selects a
subset of logical launches and verifies that every selected dispatch still contains every expected
replay pass. This combination was previously uncovered: iteration-range tests did not enable
replay, while replay tests did not specify an iteration range.

The regression test mirrors AIPROFSDK-1130 with four launches, range 2-3, and two counter groups.
It asserts exactly two selected logical dispatches, passes [0,1] on each, and a distinct counter
group on each pass. Numeric dispatch IDs are deliberately not pinned.

Regression coverage:
CMakeLists.txt L148-L202.

Test Result

CI restarted for the latest test-only update. As of September 8, 2026: 8 passed, 29 pending,
0 failed, and 8 skipped
.

View the live CI status.

Submission Checklist

--kernel-iteration-range selects host-observed kernel launches. The
filter advances a per-kernel counter every time it is consulted, and
counter_dispatch_callback consulted it on every replay pass, so one
launch was charged as many iterations as there were counter groups.

With --replay-mode kernel and a range of [1], pass 0 matched iteration 1
and passes 1..N-1 were rejected, so their counter groups were never
configured and were silently dropped: the run reported the first group
only. Across launches the counter conflated launches with passes (10
after two launches of a five-group replay instead of 2), so any range
beyond the first selected an arbitrary launch/pass pair.

Consult the filter once per dispatch instead. Pass 0 asks, which
advances the count by the single launch the application made, and the
remaining passes of that dispatch reuse the answer. The decision is
cleared when a replay loop begins so it cannot leak between dispatches.
Non-replayed dispatches are unaffected.

Adds an integration test running replay with --kernel-iteration-range
and asserting every surviving dispatch still carries all of its passes.
The combination was previously untested: the iteration-range tests never
enable replay, and the replay tests never set a range.

Co-authored-by: Vivek Kale <vivek.kale@amd.com>
@therock-pr-bot

therock-pr-bot Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/tool.cpp, projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/conftest.py, projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/validate.py; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@amd-vkale
amd-vkale marked this pull request as ready for review September 8, 2026 20:59
Copilot AI lite review requested due to automatic review settings September 8, 2026 20:59
@amd-vkale
amd-vkale requested review from a team as code owners September 8, 2026 20:59

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.

🟢 Approval recommended

The dispatch-level caching approach aligns with the stated replay/pass execution model in this tool and the added integration test directly guards against the reported regression.

Pull request overview

This PR fixes an interaction bug between kernel replay and --kernel-iteration-range in the rocprofiler-sdk tool: the iteration-range filter was being consulted once per replay pass (instead of once per user-visible dispatch), causing a single kernel launch to consume multiple “iterations” and leading to later replay passes being silently dropped. It also adds an integration regression test to ensure filtered dispatches still include all replay passes.

Changes:

  • Cache the kernel-iteration-range targeting decision per dispatch during kernel replay so only replay pass 0 advances the per-kernel iteration counter.
  • Reset the cached decision at the start of each replay loop to prevent cross-dispatch leakage.
  • Add an integration test covering --replay-mode kernel combined with --kernel-iteration-range, validating pass completeness.
File summaries
File Description
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/tool.cpp Ensures kernel-iteration-range filtering is applied once per dispatch (not per replay pass) by caching the decision in a thread-local during replay.
projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/CMakeLists.txt Adds an integration regression test combining kernel replay with --kernel-iteration-range and validating that surviving dispatches still have all replay passes.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Report

Code Coverage Report

Tests Only

code coverage tests.png

Samples Only

code coverage samples.png

Tests + Samples

code coverage all.png

@amd-vkale amd-vkale self-assigned this Sep 8, 2026
@amd-vkale
amd-vkale requested review from rpathani and a balanced review from Copilot September 8, 2026 22:13

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.

🟢 Approval recommended

The implementation matches replay callback ordering and the integration test covers the reported pass-loss regression.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@amd-vkale amd-vkale changed the title fix(rocprofiler-sdk): stop replay passes consuming kernel iterations fix(rocprofiler-sdk): stop replay passes from consuming kernel iterations Sep 8, 2026
Mirror AIPROFSDK-1130 with four launches, iteration range 2-3, and two counter groups. Assert that exactly two logical dispatches survive and that each carries both replay passes with distinct counter groups. This covers rejected and accepted decisions, resetting between dispatches, and pass completeness without pinning numeric dispatch IDs.

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

@bgopesh bgopesh 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.

Claude flags the position of the fix. worth looking

@amd-vkale

Copy link
Copy Markdown
Contributor Author

Claude flags the position of the fix. worth looking

Thanks for that. Working on fixing and then testing again.

…geted_kernel

Review feedback: the replay/iteration conflation is a property of the
filter being stateful, not of the counter callback, so guarding a single
caller left the invariant enforced by that call site's local convention
rather than by the function owning the state.

is_targeted_kernel() is the shared chokepoint for four callers:
att_dispatch_callback, att_dispatch_consecutive_kernel_callback,
counter_dispatch_callback and spm_dispatch_callback. Only the third was
guarded. Replay does not drive the other three today -- rocprofv3
rejects ATT, PC sampling and SPM alongside --replay-mode kernel -- so
there was no live bug, but any path that later gained replay would have
silently reacquired the original one.

Move the memoization into is_targeted_kernel() so the rule travels with
the state it protects. counter_dispatch_callback returns to its original
form and the behavioural change is now five lines in one function.

No functional change to the fix: pass 0 still consults the filter and
advances the count by the single launch the application made, later
passes of that dispatch still reuse the answer, the cache is still
cleared when a replay loop begins, and non-replayed dispatches still
take the unchanged path.

Co-authored-by: Vivek Kale <vivek.kale@amd.com>
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Oct 5, 2026
@amd-vkale

Copy link
Copy Markdown
Contributor Author

@bgopesh can you please re-review?

@github-actions github-actions Bot removed the Stale PR has no activity for 25+ days label Oct 7, 2026
The checks on this branch last ran on Sep 9, against a develop that has
moved on since, and the required "RCCL PR gate" check was added to the
develop ruleset after that run, so it has never been reported here. This
commit changes no files; it only starts a fresh CI run.
amd-vkale added a commit that referenced this pull request Oct 9, 2026
Both are outputs of the same kernel-replay run, so the option reads better beside --json-input
than at the end of pytest_addoption. It also keeps this change clear of the option that the
kernel-iteration-range fix (#11335) appends after --common-counters, so the two PRs merge in
either order without a conflict in conftest.py.
@amd-vkale
amd-vkale merged commit a5c51ce into develop Oct 9, 2026
148 of 151 checks passed
@amd-vkale
amd-vkale deleted the users/vkale/fix-replay-iteration-range-with-cursor-fixes branch October 9, 2026 05:12
cursor Bot pushed a commit that referenced this pull request Oct 9, 2026
rocprofv3-test-kernel-replay-iteration-range-generate arrived on develop
with #11335 after the other generate tests got RUN_SERIAL. Its validate
step compares SQ_WAVES and SQ_INSTS_VALU across passes as well, so it
needs to run alone for the same reason.

JIRA ID: AIPROFSDK-1171

Co-authored-by: Vivek Kale <vivek.kale@amd.com>
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

anujshuk-amd pushed a commit that referenced this pull request Oct 12, 2026
…ions (#11335)

--kernel-iteration-range numbers the launches the application made. Kernel
replay runs one launch once per counter group, and every pass consulted the
stateful iteration filter, so one launch was counted once per pass. Passes
past the end of the range were rejected and those counter groups were dropped.
The run still exited successfully, with records that looked complete.

is_targeted_kernel() now decides once per logical dispatch: pass 0 consults
the filter and later passes reuse that answer. Each replay loop clears the
cached decision when it starts. A dispatch that is not replayed still consults
the filter every time.

The regression test is the reported case, AIPROFSDK-1130: four launches, range
2-3, two counter groups. It requires exactly two selected dispatches, each
with passes [0, 1], and does not pin dispatch ids.
---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
amd-vkale added a commit that referenced this pull request Oct 15, 2026
…CHANGELOG

Record the #11335 kernel-replay --kernel-iteration-range / multi-group --pmc fix under Unreleased Resolved issues for ROCm 10.2.
amd-vkale added a commit that referenced this pull request Oct 15, 2026
…released

Record already-merged remove-callbacks (#11967–#11970), SPM set_agents, and
the #11335 kernel-iteration-range fix under Unreleased for ROCm 10.2, alongside
the rocpd replay-pass aggregation note from this PR.
amd-vkale added a commit that referenced this pull request Oct 15, 2026
Drop the remove-callbacks / set_agents / #11335 Unreleased bullets from this
PR so they land via the dedicated docs PR instead of conflicting here.
bgopesh added a commit that referenced this pull request Oct 21, 2026
Resolve the CHANGELOG conflict by keeping the rocprofv3 --doctor (#12889)
and hsa_amd_queue_create (#13720) entries from develop alongside this
PR's Unreleased entries.

Replace the queue-interposition, SPM agent-scoping, and kernel replay
iteration-range entries with more detailed versions (#11967-#11970,
#11335).

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants