-
Notifications
You must be signed in to change notification settings - Fork 65
Fix MI100 counter names #848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
* Do not use custom definition for MI100 counters since rocprofiler-sdk supports MI100 counters
@@ -274,13 +274,13 @@ Panel Config: | |||
pop: ((100 * AVG((((TCC_EA_WRREQ_64B_sum * 64) + ((TCC_EA_WRREQ_sum - TCC_EA_WRREQ_64B_sum) | |||
* 32)) / (End_Timestamp - Start_Timestamp)))) / $hbmBandwidth) | |||
L2-Fabric Read Latency: | |||
value: AVG(((TCC_EA_RDREQ_LEVEL_sum / TCC_EA_RDREQ_sum) if (TCC_EA_RDREQ_sum | |||
value: AVG(((TCC_EA0_RDREQ_LEVEL_sum / TCC_EA0_RDREQ_sum) if (TCC_EA0_RDREQ_sum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TCC_EA0_RDREQ_sum
doesn't have gfx908 in: https://github.com/AMD-ROCm-Internal/rocprofiler-sdk-internal/blob/3a7ff6c9c86d8490f8dd1e057150ca2cf94a8445/source/share/rocprofiler-sdk/counter_defs.yaml#L6649
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same for
TCC_EA0_WRREQ_sum
TCC_EA0_RDREQ_sum
TCC_EA0_WRREQ
TCC_EA0_RDREQ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does, please check latest staging branch :) https://github.com/AMD-ROCm-Internal/rocprofiler-sdk-internal/blob/amd-staging/source/share/rocprofiler-sdk/counter_defs.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right!
rocprofiler-compute Pull Request
Related Issue
What type of PR is this? (check all that apply)
Technical Details
Do not use custom definition for MI100 counters since rocprofiler-sdk supports MI100 counters in https://github.com/AMD-ROCm-Internal/rocprofiler-sdk-internal/pull/501
Fix MI100 counter names per rocprofiler-sdk definition
Fix metric formulas due to counter name change
Have you added or updated tests to validate functionality?
Added / Updated documentation?
Have you updated CHANGELOG?
Checklist to follow before merge