ROCm / rocprofiler-compute

Advanced Profiling and Analytics for AMD Hardware
https://rocm.docs.amd.com/projects/omniperf/en/latest/
MIT License
135 stars 49 forks source link

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

Closed skyreflectedinmirrors closed 1 year ago

skyreflectedinmirrors commented 1 year ago

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 commented 1 year ago

Vote for removing it from LDS section.