Various component: Replace PAPI_NTV_ENUM_UMASKS with PAPI_NTV_ENUM_DEFAULT_QUALIFIERS#495
Open
Treece-Burgess wants to merge 2 commits intoicl-utk-edu:masterfrom
Open
Conversation
Contributor
|
I am reviewing this PR. |
Contributor
Author
|
For, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Background:
In the master branch the modifier
PAPI_NTV_ENUM_UMASKSis implemented throughout many of the components such ascuda,rocp_sdk, androcm. This modifier is then used in various PAPI utilities and ctests.Issue:
Only the CPU components have umasks. Due to this, the use/implementation of
PAPI_NTV_ENUM_UMASKSfor the non-cpu components is not accurate.Resolution:
The PR introduces a new modifier called
PAPI_NTV_ENUM_DEFAULT_QUALIFIERS, which serves the same purpose asPAPI_NTV_ENUM_UMASKS, but is specifically for non-cpu components that have qualifiers implemented.Testing
The following list of files that mention
PAPI_NTV_ENUM_UMASKSwere not changed (reason included):papi_libpfm3_events.c- CPU relatedpe_libpfm4_events.c- CPU relatedeventstock.c- CPU relatedperfmon_ia64.c- CPU relatedpapi.h- BothPAPI_NTV_ENUM_UMASKSandPAPI_NTV_ENUM_DEFAULT_QUALIFIERSneed to be supportedpapi.c- BothPAPI_NTV_ENUM_UMASKSandPAPI_NTV_ENUM_DEFAULT_QUALIFIERSneed to be supportedtest_utils.c- The functionenum_add_native_events.cutilizesPAPI_NTV_ENUM_UMASKSand is only called inoverflow_allcounters.cwhere the call has the component index 0 hardcoded which corresponds to theperf_eventcomponentpapi_hybrid_native_avail.c- Unable to test as we do not have access to an Intel Xeon Phi as standsTesting was done on Illyad at Oregon which has the following setup:
CPU: AMD EPYC 7402
GPUs: 1 * H100 and 1 * MI210
OS: RHEL 8.10
ROCm version: 6.1.3
Cuda Toolkit verison: 12.6.3
./papi_native_availand./papi_native_avail -e: ✅-eflag I tested withCYCLESfrom theperf_eventcomponent anddram__bytesfrom thecudacomponent. For thecudacomponent, I had to add a custom implementation underPAPI_NTV_ENUM_DEFAULT_QUALIFIERSto properly test and will add this as a feature in a separate PR:./papi_availand./papi_avail -e: ✅papi_native_avail -e../papi_xml_event_info: ✅./papi_event_chooser: ✅./failed_events: ✅perf_eventandrocmcomponents../all_native_events: ✅perf_eventandrocmcomponents.NOTE:
papi_hybrid_native_avail.cwas not updated in this PR to account forPAPI_NTV_ENUM_DEFAULT_QUALIFIERS. This is due to the PAPI team lacking the necessary hardware and software to perform the proper testing as this time.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