Repository navigation
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
1 task
amd-vkale
force-pushed
the
cursor/kernel-replay-att-pass-d7b8
branch
from
October 9, 2026 07:21
101e8a3 to
101b092
Compare
amd-vkale
marked this pull request as ready for review
October 9, 2026 20:58
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Environment-driven options and input-file counter groups can bypass newly added incompatibility checks.
2 open findings
What changed in this PR
Adds a dedicated ATT pass to rocprofv3 kernel replay, separating thread tracing from counter collection.
Changes:
- Plans dispatch filtering and ATT/counter passes together.
- Allows
--attwith kernel replay while rejecting incompatible modes. - Adds validation, integration tests, and documentation.
| File | Description |
|---|---|
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-tool/tool.cpp |
Implements replay planning and per-pass context switching. |
projects/rocprofiler-sdk/source/bin/rocprofv3.py |
Updates replay compatibility validation. |
projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/CMakeLists.txt |
Adds ATT replay integration tests. |
projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/conftest.py |
Adds validator options and fixtures. |
projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/validate.py |
Validates trace files, dispatches, and counters. |
projects/rocprofiler-sdk/tests/rocprofv3/kernel-replay/test_kernel_replay_cli.py |
Tests revised CLI compatibility rules. |
projects/rocprofiler-sdk/source/docs/how-to/using-rocprofv3.rst |
Documents ATT-enabled replay. |
projects/rocprofiler-sdk/source/docs/how-to/using-kernel-replay-rocprofv3.rst |
Details ATT pass behavior and limitations. |
projects/rocprofiler-sdk/CHANGELOG.md |
Records the new behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+100
to
+104
| Adding ``--att`` gives each profiled dispatch one more pass, ahead of its counter passes, that | ||
| runs the dispatch thread trace with counter collection switched off. The counter groups then run | ||
| on the following passes with the thread trace switched off. Each dispatch is traced once, and its | ||
| counters are collected against the same restored inputs but without the trace's memory traffic | ||
| and serialization perturbing them. |
Comment on lines
+316
to
+318
| --passes 2 --expected-dispatch-count 4 --common-counters SQ_WAVES SQ_INSTS_VALU | ||
| --thread-trace -k | ||
| "dispatch_id_constant_across_replay_passes or expected_logical_dispatch_count or each_pass_collects_distinct_batch or one_thread_trace_per_dispatch" |
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:
This PR will be automatically closed in 5 days if no further activity occurs. |
…CONFIG The kernel filters (--kernel-include-regex, --kernel-exclude-regex, --kernel-iteration-range) were only consulted from the counter dispatch callback, after the SDK had already been told to replay the dispatch once per counter group. A dispatch no filter selects was still drained, snapshotted and re-executed N times, collecting nothing on any pass. Consult the filter once per logical dispatch in the CONFIG callback instead, when the SDK asks for the pass count. A dispatch that is not selected reports one pass and runs once on the ordinary path. The decision is kept per thread, keyed by the dispatch id the SDK reserves before CONFIG, and every later callback for that id -- each replay pass, or the single run of a dispatch that is not replayed -- reuses it, so the stateful iteration filter still advances exactly once per application launch. This replaces the pass-0 memo in is_targeted_kernel, which could only take effect once a pass had started. Counter collection's iteration counts move to namespace scope so CONFIG and the counter callback number launches the same way. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
Co-authored-by: Vivek Kale <vivek.kale@amd.com>
rocprofv3 rejected --att with --replay-mode kernel because only counter collection was pass-aware: the thread trace would stay on for every pass and report each dispatch once per pass. The SDK has let a tool switch services per pass since kernel replay landed (replay_start_context/replay_stop_context, and the att_client and local-context samples); rocprofv3 simply never used it. With --att, each targeted dispatch now gets one extra pass ahead of its counter passes. Pass 0 runs the dispatch thread trace with counter collection locally stopped, pass 1 locally stops the trace and restarts counter collection, and counter group g runs on pass g + 1. The toggles are sticky for the rest of the loop and the SDK restores both contexts afterwards. The trace never shares a pass with counter collection: SQTT traffic and serialization perturb the counters, and both services write the one per-dispatch user_data slot the SDK hands out, so whichever dispatch callback runs last overwrites the other's state. Under replay the dispatch thread trace moves to its own context so a pass can switch it without dropping that pass's kernel dispatch, memory copy and API records from the client context. It only traces a dispatch the CONFIG plan selected, on the thread trace pass, or on the single run of a dispatch with no counter group to collect; a replay that falls back to a single run, and a dispatch replay never planned (HIP graph, multi-packet submission), keep counter collection instead. Counter records still number their groups 0..N-1 in replay_pass. Device-mode thread trace (--att-consecutive-kernels, --att-no-intercept, --selected-regions) and --collection-period cannot be confined to one pass, so rocprofv3 rejects them together with --replay-mode kernel and --att. PC sampling and --spm stay rejected. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
Two GPU integration tests, disabled when rocprof-trace-decoder is not found, like the other rocprofv3 ATT tests: - --att with two counter groups over the kernel-replay app. validate.py checks the usual replay invariants (counter records carry replay_pass 0..1 once per dispatch, sanity counters constant across passes, one distinct batch per pass), and a new test_one_thread_trace_per_dispatch requires each dispatch to be traced exactly once and the traced dispatches to match the ones with counter records. - The same run with three launches per kernel. With --att and no iteration range only each kernel's first launch is profiled, so exactly three logical dispatches must remain, each traced once and carrying both counter passes. The new check is opt-in through --thread-trace so the counters-only runs are unaffected. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
Describe how --att combines with --replay-mode kernel, how passes map to counter groups, which thread trace options are rejected and why, that replay_pass still numbers counter groups, and that dispatches the kernel filters do not select are no longer replayed. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
mi325 CI ran the new thread trace replay tests for the first time. The behavior they exist for held -- every replayed dispatch was traced exactly once, the traced dispatches matched the ones with counter records, and replay_pass still ran 0..1 -- but two expectations were wrong: - test_expected_counters_present checks a fixed counter list written for the five-group run. A --expected-counters option now names the union of a run's groups, and the thread trace runs pass their two-group list. - With first-launch-only targeting the app yields four profiled dispatches, not three: besides its three kernels, the HIP runtime dispatches a copy kernel for hipMemcpyToSymbol, and that kernel's first launch is profiled too. Also black-format validate.py; it predates the formatter's current output, and the formatting job checks every touched Python file. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
…ially rocprofv3-test-kernel-replay-att-validate checks that SQ_WAVES and SQ_INSTS_VALU stay constant across a dispatch's replay passes. Those SQ counters count every wave on the device, and the generate test only holds the rocprofiler_kernel_replay lock, so GPU tests that run alongside it add waves and instructions to some of the passes and the check fails intermittently. Run rocprofv3-test-kernel-replay-att-generate with RUN_SERIAL, the same fix #12888 makes for the other kernel replay generate tests. The first-iteration variant keeps running in parallel: its validate step selects only checks that do not compare counter values.
…erially rocprofv3-test-kernel-replay-att-first-iteration-validate compares SQ_WAVES and SQ_INSTS_VALU across passes too, so its generate test needs RUN_SERIAL for the same reason as rocprofv3-test-kernel-replay-att-generate. JIRA ID: AIPROFSDK-1171 Co-authored-by: Vivek Kale <vivek.kale@amd.com>
A disabled thread-trace pass still serializes counter passes, so the option undoes the separation this replay pass exists for. The first-iteration ATT validator now also checks that shared counters stay constant across passes. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
cursor
Bot
force-pushed
the
cursor/kernel-replay-att-pass-d7b8
branch
from
November 5, 2026 08:21
a85bc14 to
84ed986
Compare
Pass att_serialize_all as a mapping to dotdict, and put pytest-packages on PYTHONPATH for the validators that run validate.py directly. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
The CLI already rejects the flag because a stopped thread-trace pass still serializes counter passes. Say that in the changelog and the CLI limitations. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
This branch has not been deployed
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
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.


Motivation
rocprofv3 rejects
--atttogether with--replay-mode kernel, so a dispatch can't be thread-traced and have its counters collected in the same run. Kernel replay already re-runs each profiled dispatch from restored memory, once per counter group. This PR gives the dispatch thread trace a pass of its own. Each dispatch is traced once, and its counters are collected on the other passes, against the same inputs but without the trace's memory traffic and serialization.Builds on #11335, now merged. The branch is rebased onto develop, so the diff is only the thread-trace pass.
#12729 (passes described by input-file jobs) is stacked on this PR.
Technical Details
Each profiled dispatch runs three times: pass 0 is the thread trace, pass 1 collects
SQ_WAVESandGRBM_COUNT, and pass 2 collectsGRBM_GUI_ACTIVE.tool.cpp):plan_replay_dispatch), and every pass reuses that plan. The trace and the counters therefore agree on which dispatches are profiled. This relies on fix(rocprofiler-sdk): stop replay passes from consuming kernel iterations #11335's memoized iteration filter.--att, the dispatch thread trace is configured on its own context (att_replay_context), so replay can switch it per pass withreplay_start_contextandreplay_stop_context. The trace pass stops counter collection, and the counter passes stop the trace.--att.replay_passin counter records still numbers the counter group,0..N-1.--att, nothing changes.rocprofv3.py):--attis now allowed with--replay-mode kernel. Options that would capture every pass, or serialize the counter passes, are rejected with it:--att-consecutive-kernels,--att-no-intercept,--selected-regions,--collection-period, and--att-serialize-all. A stopped thread-trace context still returnsbSerialize, so--att-serialize-allwould serialize the counter passes this split exists to avoid.RUN_SERIAL, like the other replay generate tests in test(rocprofv3): run the kernel replay generate tests serially #12888. Their validators compare SQ counters across passes, and those count every wave on the GPU, so concurrent tests make them fail intermittently. The validators that invokevalidate.pydirectly getpytest-packagesonPYTHONPATH.using-kernel-replay-rocprofv3.rst, the pass-count rules and limitations there (including the--att-serialize-allrejection),using-rocprofv3.rst, and the CHANGELOG.Issue Tracking
JIRA ID: AIPROFSDK-1171
Test Plan
test_kernel_replay_cli.py(no GPU):--attis accepted with replay, each of the four device-scoped options is rejected with it, and--att-serialize-allis rejected with it.GPU (mi325):
rocprofv3-test-kernel-replay-att-{generate,validate}androcprofv3-test-kernel-replay-att-first-iteration-{generate,validate}. The validator checks that:replay_passstays0..N-1;The tests are skipped where the trace decoder library isn't installed.
Test Result
a4eff9fc6f(Core, Coverage): all four thread-trace replay tests passed, including on the two jobs that failed for other reasons. On Core ubuntu-22.04 both generate and both validate tests passed. The CLI unit test is 33/33 locally.pc_sampling_vs_dispatch_counter_collection,pc_sampling_vs_device_counter_collection, and the two reverse-order variants aborted.device_counting_service_testsync/async counters timed out, and the threeexternal-correlation-id-requestsamples aborted.cecc6c6752is documentation only (changelog and the CLI limitations name the--att-serialize-allrejection). CI is running again for that commit.Submission Checklist