search

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

Units missing from L2 per-channel breakdown #133

Closed skyreflectedinmirrors closed 1 year ago

skyreflectedinmirrors commented 1 year ago

image

I think we just have to copy the units from the base metric into, e.g.:

https://github.com/AMDResearch/omniperf/blob/8af6efc0be5288dadd2e758556154e52f7406e5d/src/omniperf_analyze/configs/gfx90a/1800_L2_cache_per_channel.yaml#L207

coleramos425 commented 1 year ago

Could you elaborate @arghdos. Per your screenshot, I'm reading metrics 18.1.1 - 18.1.7 all as "...req per $normUnit". Which seems correct to me

Am I missing something?

skyreflectedinmirrors commented 1 year ago

I guess it's sort of implied by the metric being "L1-L2 read req(uests)", but the unit would actually be "requests per wave" or "cycles per wave" (e.g., this one: https://github.com/AMDResearch/omniperf/blob/8af6efc0be5288dadd2e758556154e52f7406e5d/src/omniperf_analyze/configs/gfx90a/1800_L2_cache_per_channel.yaml#L921) not just "per wave". At a glance, it looks like all the request ones have this. Low priority issue tho

coleramos425 commented 1 year ago

Patch merged into dev. Closing issue.