Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/rocp_sdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,4 @@ Example:
## Known Limitations

* In dispatch mode, PAPI may read zeros if reading takes place immediately after the return of a GPU kernel. This is not a PAPI bug. It may occur because calls such as hipDeviceSynchronize() do not guarantee that ROCprofiler has been called and all counter buffers have been flushed. Therefore, it is recommended that the user code adds a delay between the return of a kernel and calls to PAPI_read(), PAPI_stop(), etc.
* If an application is linked against the static PAPI library libpapi.a, then the application must call PAPI_library_init() before calling any hip routines (e.g. hipInit(), hipGetDeviceCount(), hipLaunchKernelGGL(), etc). If the application is linked against the dynamic library libpapi.so, then the order of operations does not matter.
* If an application is linked against the static PAPI library libpapi.a, then the application must call PAPI_library_init() through PAPI_add_named_event()/PAPI_add_event() before calling any hip routines (e.g. hipInit(), hipGetDeviceCount(), hipLaunchKernelGGL(), etc). If the application is linked against the dynamic library libpapi.so, then the order of operations does not matter.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the added "through PAPI_add_named_event()/PAPI_add_event()" was added due to the PAPI_EDELAY_INIT support and doing this workflow would initialize the component.

However, you can also call PAPI_enum_cmp_event(int *EventCode, PAPI_ENUM_FIRST, int cidx) to initialize the component. I would make that aware to users as well.

6 changes: 3 additions & 3 deletions src/components/rocp_sdk/rocp_sdk.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ rocp_sdk_init_component(int cid)
return papi_errno;
}

// This component needs to be fully initialized from the beginning,
// because interleaving hip calls and PAPI calls leads to errors.
return check_n_initialize();
sprintf(_rocp_sdk_vector.cmp_info.disabled_reason, "Not initialized. Access component events to initialize it.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change sprintf to snprintf and then check the returned value, i.e.

int strLen = snprintf(_rocp_sdk_vector.cmp_info.disabled_reason, PAPI_HUGE_STR_LEN, "%s",  "Not initialized. Access component events to initialize it.");
if (strLen < 0 || strLen >= PAPI_HUGE_STR_LEN) {
    SUBDBG(...)
    return PAPI_EBUF;
}

_rocp_sdk_vector.cmp_info.disabled = PAPI_EDELAY_INIT;
return PAPI_EDELAY_INIT;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my testing, if I have the following workflow:

PAPI_library_init(PAPI_VER_CURRENT);
int cidx = API_get_component_index("rocp_sdk");

int EventCode = 0 | PAPI_NATIVE_MASK;
PAPI_enum_cmp_event(&EventCode, PAPI_ENUM_FIRST, cidx);

const PAPI_component_info_t *cmpinfo = PAPI_get_component_info(cidx);

The member variable of PAPI_component_info_t disabled_reason will still show "Not initialized. Access component events to initialize it." This should not be the case anymore as I have initialized rocp_sdk.

To resolve this you need to set disabled_reason to an empty string inside rocp_sdk_init_private (this is what I did in the cuda component see here).


int
Expand Down