Skip to content
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

Wave Cycles metric in LDS section seems out of place #130

Closed
skyreflectedinmirrors opened this issue May 11, 2023 · 1 comment
Closed

Wave Cycles metric in LDS section seems out of place #130

skyreflectedinmirrors opened this issue May 11, 2023 · 1 comment
Assignees

Comments

@skyreflectedinmirrors
Copy link
Contributor

https://github.com/AMDResearch/omniperf/blob/f8fe039aa57422bb5036d647699f1f97298bed5d/src/omniperf_analyze/configs/gfx90a/1200_lds.yaml#LL46C1-L49C30

This is a (somewhat) duplicated version of the Wave Cycles metric in the wavefront section:

https://github.com/AMDResearch/omniperf/blob/f8fe039aa57422bb5036d647699f1f97298bed5d/src/omniperf_analyze/configs/gfx90a/0700_wavefront-launch.yaml#LL107C1-L109C54

with the added restrict that it is only cycles per wave.

It's unclear what use this serves here, as the same information is in wavefront launch stats. It is somewhat useful for comparison against e.g., the Index Accesses metrics (and others that are potentially in cycles / wave, if norm_unit == wave), but is more constrained because it only ever reports per_wave.

I suggest we either remove it from the LDS section, or enable normalization on it if we find this useful to keep in LDS. My vote would be the former, otherwise we need to document that it's the same as in wavefront.

@feizheng10
Copy link
Contributor

Vote for removing it from LDS section.

coleramos425 added a commit that referenced this issue May 15, 2023
Signed-off-by: coleramos425 <colramos@amd.com>
@coleramos425 coleramos425 added this to the v1.0.8 milestone May 15, 2023
@coleramos425 coleramos425 self-assigned this May 15, 2023
feizheng10 pushed a commit to feizheng10/omniperf that referenced this issue Dec 6, 2023
Signed-off-by: coleramos425 <colramos@amd.com>
Signed-off-by: fei.zheng <fei.zheng@amd.com>
feizheng10 pushed a commit to feizheng10/omniperf that referenced this issue Dec 20, 2023
Signed-off-by: coleramos425 <colramos@amd.com>
Signed-off-by: fei.zheng <fei.zheng@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants