Skip to content

[rocprofiler-sdk] : collect SPM TA and VCA counters for gfx12 and gfx1250 - #13449

Open
sauverma93 wants to merge 4 commits into
developfrom
users/sauverma/spm-ta-metrics
Open

sauverma93 wants to merge 4 commits into
developfrom
users/sauverma/spm-ta-metrics

Conversation

@sauverma93

@sauverma93 sauverma93 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

This follows up #12212 for gfx1250 and gfx12.

gfx1250 (MI455) reports TA activity as TX_VCA_VCA_BUSY and TX_VCA_TOTAL_WAVEFRONTS. The JSON tool requested TA_TA_BUSY and TA_TOTAL_WAVEFRONTS, so those TA counters never appeared in the gfx1250 samples.

gfx1200 and gfx1201 do use the TA counters. TA_TA_BUSY was already defined. TA_TOTAL_WAVEFRONTS was not, so the counter-value tests compared TA_TA_BUSY with a missing wavefront counter.

The SQC I-cache check is the other failure, and it does not behave the same on every architecture. On MI300, SQC_ICACHE_REQ stays populated and hits and misses stay within that request count, so a miss-rate ratio is well defined. On gfx12 and gfx1250 these counters are nondeterministic per SQ and per bank. Early dispatches of a kernel still see I-cache traffic. Later dispatches of the same kernel can report no requests, hits, or misses, because the instruction cache is already warm. Dividing by SQC_ICACHE_REQ then fails.

Technical Details

The JSON tool requests TX_VCA_VCA_BUSY and TX_VCA_TOTAL_WAVEFRONTS alongside TA_TA_BUSY and TA_TOTAL_WAVEFRONTS. It collects only the counters the agent supports. validate.py reads the agent recorded on each dispatch:

  • gfx1250: TX_VCA_VCA_BUSY and TX_VCA_TOTAL_WAVEFRONTS
  • other agents, including gfx12: TA_TA_BUSY and TA_TOTAL_WAVEFRONTS

The VCA counters are already defined for gfx1250. TA_TOTAL_WAVEFRONTS is now defined for gfx12, gfx1200, and gfx1201 on the same TA event used by gfx908 and gfx90a.

The SQC check no longer divides by SQC_ICACHE_REQ. It requires SQC_ICACHE_HITS and SQC_ICACHE_MISSES to be no larger than SQC_ICACHE_REQ. A dispatch with no I-cache traffic passes. A hit or miss reported with no requests still fails. A missing counter fails with its name.

The tests this addresses:

  • test-spm-dispatch-collection-validate.test_spm_counter_values
  • test-spm-dispatch-collection-validate.test_spm_counter_consistency
  • test-spm-dispatch-buffer-collection-validate.test_spm_counter_values

Issue Tracking

ISSUE ID: AIPROFSDK-1151

Follow-up to #12212.

Test Plan

gfx1250

  • Re-run the SPM integration tests on gfx1250. The consistency checks pass.

  • Confirm the gfx1250 samples include TX_VCA_VCA_BUSY and TX_VCA_TOTAL_WAVEFRONTS, and that the busy counter is greater than the wavefront counter.

  • Re-run the two counter-value tests after the SQC comparison change:

    • test-spm-dispatch-collection-validate.test_spm_counter_values
    • test-spm-dispatch-buffer-collection-validate.test_spm_counter_values

    The VCA comparison already passed. The remaining failure was a warm I-cache: requests, hits, and misses were all absent, and the old check divided by the request count. That case is now accepted.

gfx12

  • TA_TOTAL_WAVEFRONTS is collected, and TA_TA_BUSY is greater than it.
  • These pass:
    • test-spm-dispatch-collection-validate.test_spm_counter_values
    • test-spm-dispatch-collection-validate.test_spm_counter_consistency
    • test-spm-dispatch-buffer-collection-validate.test_spm_counter_values
    • test-spm-dispatch-buffer-collection-validate.test_spm_counter_consistency
  • SQC_ICACHE_* matches the expected gfx12 pattern. The first launches of a kernel still report requests, with hits and misses no larger than that request count. Repeat launches of the same kernel report no I-cache traffic. Both pass.
  • Not changed here: rocprofv3-test-spm-json-validate.test_validate_spm still requires SQ_WAVES to match the PMC total exactly. SPM can come in slightly under that total, so this shader-clock check can still fail. The reference-clock run of the same check passed.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Oct 14, 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/tests/spm/validate.py, projects/rocprofiler-sdk/tests/tools/json-tool.cpp; 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?

@sauverma93
sauverma93 marked this pull request as draft October 14, 2026 11:42
@sauverma93 sauverma93 changed the title [rocprofiler-sdk] : define gfx12 and gfx1250 TA counters for the SPM tests [DO NOT REVIEW or MERGE][rocprofiler-sdk] : define gfx12 and gfx1250 TA counters for the SPM tests Oct 14, 2026
@sauverma93
sauverma93 force-pushed the users/sauverma/spm-ta-metrics branch from 9f51767 to fb67bd9 Compare October 22, 2026 13:51
@sauverma93 sauverma93 changed the title [DO NOT REVIEW or MERGE][rocprofiler-sdk] : define gfx12 and gfx1250 TA counters for the SPM tests [DO NOT REVIEW or MERGE][rocprofiler-sdk] : collect VCA counters for gfx1201 and gfx1250 SPM tests Oct 23, 2026
@sauverma93 sauverma93 changed the title [DO NOT REVIEW or MERGE][rocprofiler-sdk] : collect VCA counters for gfx1201 and gfx1250 SPM tests [rocprofiler-sdk] : collect VCA counters for gfx1201 and gfx1250 SPM tests Oct 23, 2026
@sauverma93
sauverma93 force-pushed the users/sauverma/spm-ta-metrics branch from fb67bd9 to 6a8dad6 Compare October 23, 2026 12:42
@sauverma93 sauverma93 changed the title [rocprofiler-sdk] : collect VCA counters for gfx1201 and gfx1250 SPM tests [rocprofiler-sdk] : collect VCA counters for gfx1250 SPM tests Oct 23, 2026
@sauverma93 sauverma93 changed the title [rocprofiler-sdk] : collect VCA counters for gfx1250 SPM tests [rocprofiler-sdk] : collect SPM TA and VCA counters for gfx12 and gfx1250 Oct 26, 2026
@sauverma93
sauverma93 force-pushed the users/sauverma/spm-ta-metrics branch 2 times, most recently from 44814c2 to 0daff31 Compare October 29, 2026 16:45
@github-actions

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

@sauverma93
sauverma93 force-pushed the users/sauverma/spm-ta-metrics branch 3 times, most recently from 178a2cd to dff662a Compare October 30, 2026 13:24

@bing-ma bing-ma 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.

Looks good to me; the gfx12/gfx1250 counter selection and the warm-I-cache SQC validation change both make sense.

sauverma93 and others added 4 commits November 2, 2026 15:48
TA_TA_BUSY is TA event 15 on gfx1250, matching gfx1200 and gfx1201. TA_TOTAL_WAVEFRONTS is TA event 32 on gfx1200, gfx1201, and gfx1250.
MI455 (gfx1250) reports TA activity as TX_VCA_VCA_BUSY and TX_VCA_TOTAL_WAVEFRONTS. The JSON tool requests those counters, and the validator selects them when the dispatch agent is gfx1250.
…unt.

gfx1200 and gfx1201 count TA_TOTAL_WAVEFRONTS as TA event 32. Without that definition the SPM counter-value tests compare TA_TA_BUSY with a missing wavefront counter. The SQC hit and miss check divided by SQC_ICACHE_REQ. That ratio is valid on MI300, where the request count stays nonzero. On gfx12 and gfx1250 later dispatches report zero requests, hits, and misses once the instruction cache is warm, so the division fails. Compare hits and misses with the request count instead.
The rocprofiler-sdk formatting check rejects the previous line breaks.
@sauverma93
sauverma93 force-pushed the users/sauverma/spm-ta-metrics branch from 64973f2 to 67178eb Compare November 2, 2026 10:18
@sauverma93
sauverma93 marked this pull request as ready for review November 2, 2026 10:33
@peymanr

peymanr commented Nov 2, 2026

Copy link
Copy Markdown

PR Pundit review

(Testing PR Pundit Code Review agent. Not all comments may be relevant. This is a one-time automated pass: please address what is useful, no need to request a re-review. Please reply to each point, whether it applies or not: we harvest these replies, and both kinds are enormously helpful for tuning the agent. Reach out to @peymanr for suggestions.)

# Finding Where
1 🟠 gfx1250 VCA busy/wavefronts assert fails with bare AssertionError: this PR reuses message-less TA heuristic projects/rocprofiler-sdk/tests/spm/validate.py:128
2 🟠 SPM test passes with zero I-cache traffic on all dispatches: this PR replaced division check with value <= requests projects/rocprofiler-sdk/tests/spm/validate.py:131-141

1. 🟠 Should fix: gfx1250 VCA busy/wavefronts assert fails with bare AssertionError: this PR reuses message-less TA heuristic

projects/rocprofiler-sdk/tests/spm/validate.py:128

assert float(busy) > float(wavefronts) in _validate_dispatch_counters has no failure message, unlike the other new asserts, so a CI failure does not show the dispatch id, counter names or values. The PR now also applies this strict relation to TX_VCA_VCA_BUSY (VCA busy cycles) vs TX_VCA_TOTAL_WAVEFRONTS (vec32 packets processed) on gfx1250, and nothing shows that relation holds for those counters. The message-less strict assert and its failure when both values are zero were already on the base for the TA counters. Consider adding a message with the names and values.

Who is affected: Developers reading SPM test failures, mainly on gfx1250 where the VCA counters are now compared.

Checked against the code by an independent pass.

2. 🟠 Should fix: SPM test passes with zero I-cache traffic on all dispatches: this PR replaced division check with value <= requests

projects/rocprofiler-sdk/tests/spm/validate.py:131-141

The PR now accepts SQC_ICACHE_REQ == 0; the base crashed with ZeroDivisionError on that input. Nothing requires any dispatch to have seen I-cache traffic, so a fault that zeroes only the SQC counters (while TA/TX_VCA busy stays non-zero) would pass. Bounding SQC_ICACHE_HITS and SQC_ICACHE_MISSES separately is unchanged from the base, which used the equivalent 100*x/REQ <= 100 form. Consider asserting that at least one dispatch reports SQC_ICACHE_REQ > 0.

Who is affected: Only the SPM validate.py test; a regression limited to the SQC I-cache counters would not fail it.

Checked against the code by an independent pass.

Shared from a pilot of PR Pundit, an agentic code reviewer that builds and tests the change and reads the project's past reviews. Reviewed at 67178eb4; findings checked and posted by @peymanr; points already raised on this PR are left out.

assert float(busy) > float(wavefronts)

assert (
100

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.

Why do we remove this altogether, why not check if Req != 0 and compute the ratio in that case

This branch has not been deployed

No deployments
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.

4 participants