Minor fixes for ROCm component qualifiers #139
Conversation
|
@bertwesarg I integrated your comments into a new PR. Would you please take a look at it? |
| sprintf(info->symbol, "%s:device=%i:instance=%i", ntv_table_p->events[inf.nameid].name, inf.device, inf.instance); | ||
| sprintf(info->long_descr, "%s", ntv_table_p->events[inf.nameid].descr); | ||
| sprintf(info->long_descr, "%s, masks:Mandatory device qualifier [%s]:Mandatory instance qualifier in range [0-%i]", |
There was a problem hiding this comment.
I was now able to digest how perf_event is creating the mask description, and indeed it looks like it is using : (colon) to separate multiple qualifiers. So we have two which are using a , (comma) and one which is using a : (colon). And you opted for : (colon) too.
There was a problem hiding this comment.
Yes I looked at the perf_event output and found a : qualifier separator. I guess this is an inconsistency that ought to be fixed sooner or later.
There was a problem hiding this comment.
Actually, the components you mentioned that use a , separator are no longer in use and should eventually be removed.
There was a problem hiding this comment.
thanks, old code makes the life harder for a programmer-archaeologist
gcongiu
force-pushed
the
2023.12.14_rocm-qualifier-fixes
branch
from
3162d26 to
d1eacae
Compare
gcongiu
force-pushed
the
2023.12.14_rocm-qualifier-fixes
branch
from
d1eacae to
06a9185
Compare
Pull Request Description
The ROCm component
ntv_code_to_infofunction does not behave consistently with other components (e.g.,perf_event). This PR fixes the function behavior to align it to the CPU component.Author Checklist
Why this PR exists. Reference all relevant information, including background, issues, test failures, etc
Commits are self contained and only do one thing
Commits have a header of the form:
module: short descriptionCommits have a body (whenever relevant) containing a detailed description of the addressed problem and its solution
The PR needs to pass all the tests