fix(rocprofiler-compute): Repair counter grouping policy coalescing - #10912
Open
feizheng10 wants to merge 2 commits into
Open
fix(rocprofiler-compute): Repair counter grouping policy coalescing#10912feizheng10 wants to merge 2 commits into
feizheng10 wants to merge 2 commits into
Conversation
- Pack regular PMCs into _ACCUM buckets during metric-aware coalesce - Use formula-only counter discovery for same-bucket grouping decisions - Resolve gfx115x grouping policy and configs for gfx1151–gfx1153 Co-Authored-By: Auto Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes bugs in rocprofiler-compute’s counter grouping / metric-aware coalescing so priority metrics and accumulator (“_ACCUM”) buckets are handled correctly, and gfx1151–gfx1153 correctly resolve the shared gfx115x grouping policy/config tree (intended to land before #9324 so subsequent HBM/pass-coalescing changes stack on a correct allocator).
Changes:
- Update metric-aware coalescing to consider only counters referenced by a metric’s formula (excluding global
SUPPORTED_DENOMspill) and allow packing into_ACCUMbuckets. - Canonicalize analysis-config and grouping-policy lookup (e.g.,
gfx1151→gfx115x) in the inspector and allocator policy lookup. - Add unit tests covering formula-only extraction,
_ACCUMbucket packing behavior, and gfx115x policy resolution.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| projects/rocprofiler-compute/tools/counter_grouping_inspector.py | Resolve analysis config subdirectory via canonical_config_arch() so gfx115x uses the shared config tree. |
| projects/rocprofiler-compute/tests/unit/utils/test_utils_counter_defs.py | Add unit test ensuring formula-only counter extraction omits SUPPORTED_DENOM spill counters. |
| projects/rocprofiler-compute/tests/unit/rocprof_compute_soc/test_soc_base.py | Add unit tests for metric-aware coalesce behavior (_ACCUM packing, ratio partner co-location) and gfx115x policy activation. |
| projects/rocprofiler-compute/src/utils/utils_counter_defs.py | Add extract_metric_formula_hw_counters() to support formula-only counter extraction for coalescing decisions. |
| projects/rocprofiler-compute/src/rocprof_compute_soc/soc_base.py | Use canonical arch for grouping policy lookup, switch coalesce to formula-only extraction, and remove _ACCUM skip during greedy packing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Fix counter grouping policy bugs called out in #9324 review:
_ACCUMbuckets were skipped during metric-aware coalesce (sparse perfmon YAMLs / extra passes), priority metrics used globalSUPPORTED_DENOMspill when deciding same-bucket partners (e.g. gfx1250 CP Utilization split across passes), and gfx1151–gfx1153 never loaded the sharedgfx115xpolicy block.This PR is intended to land before #9324 so HBM / grouping-policy changes stack on a working allocator.
Technical Details
_ACCUMskip in_metric_aware_coalesce_passso regular PMCs can pack into level/accum buckets when block capacity allows (reservations preserved via_trial_counter_file_with_extra).extract_metric_formula_hw_counters()for metric-aware coalesce; profiling still uses fullextract_counters_and_variables()indetect_counters().profiling_counter_grouping_policy.yamland analysis configs viacanonical_config_arch()for gfx1151–gfx1153 (inspector +soc_base).extract_metric_formula_hw_counters()intoextract_counters_and_variables(..., include_supported_denom=True)to DRY the shared variable-resolution loop once behavior is stable in review.Full-panel perfmon passes (before → after)
Measured with
tools/counter_grouping_inspector.pyfull counter set vsorigin/rocprofiler-compute-develop(bucket count = perfmon passes).CDNA arches (empty grouping policy) unchanged on pass count. gfx1250 drops from 23 → 17 (matches the 17-pass target documented in
profiling_counter_grouping_policy.yaml). gfx115x pass count unchanged; grouping policy now activates (4 priority metric ids) where it was previously a no-op.JIRA ID
AIPROFCOMP-78
Test Plan
PYTHONPATH=src pytest tests/unit/rocprof_compute_soc/test_soc_base.py tests/unit/utils/test_utils_counter_defs.pypython3 tools/counter_grouping_inspector.py --arch gfx1250 --block 17.1.1(CP Utilization single bucket)python3 tools/counter_grouping_inspector.py --arch gfx1151(gfx115x config tree + 10 passes)Test Result
Submission Checklist
Made with Cursor