[rocprofiler-sdk] Add SPM experimental public API header by SrirakshaNag · Pull Request #3681 · ROCm/rocm-systems

SrirakshaNag · 2026-03-03T01:11:30Z

Motivation

Add SPM experimental public API header

Technical Details

The new header defines:

Config types: rocprofiler_spm_counter_config_id_t, rocprofiler_spm_configuration_t (frequency, buffer_size, timeout)
Record types: rocprofiler_spm_record_flag_t, rocprofiler_spm_dispatch_counting_service_data_t, rocprofiler_spm_counter_record_t
Callbacks: rocprofiler_spm_dispatch_counting_record_cb_t, rocprofiler_spm_dispatch_counting_service_cb_t
API functions: rocprofiler_spm_create_counter_config, rocprofiler_spm_destroy_counter_config, rocprofiler_iterate_spm_supported_counters, rocprofiler_configure_callback_spm_dispatch_service, rocprofiler_configure_buffer_spm_dispatch_service

JIRA ID

Test Plan

Existing tests should pass

Test Result

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

bwelton · 2026-03-04T21:07:44Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+ * @retval ROCPROFILER_STATUS_ERROR_AGENT_ARCH_NOT_SUPPORTED agent has no supported SPM counter
+ */
+ROCPROFILER_SDK_EXPERIMENTAL rocprofiler_status_t
+rocprofiler_iterate_spm_supported_counters(rocprofiler_agent_id_t              agent_id,


for this call, i remember there was a discussion about just using the counter collection supported counters call. What was the reasoning behind using this instead?

Yes, that needs a unified API.

bwelton · 2026-03-04T21:08:35Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+ROCPROFILER_SDK_EXPERIMENTAL rocprofiler_status_t
+rocprofiler_iterate_spm_supported_counters(rocprofiler_agent_id_t              agent_id,
+                                           rocprofiler_available_counters_cb_t cb,
+                                           void* user_data) ROCPROFILER_API ROCPROFILER_NONNULL(2);


Is this covering user_data? couldn't this be null? or is the non-null on cb?

non-null is on cb

@bwelton NONNULL starts at 1 for the arguments. The index of 0 is for the return type I believe

jrmadsen · 2026-03-05T19:10:44Z

Instead of removing <rocprofiler-sdk/spm.h>, use a #warning that the experimental header should be included for now and then just include the experimental header. Protect the #warning with a #ifdef that can be used to suppress the #warning message

ApoKalipse-V · 2026-03-10T15:58:43Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

File level: What happened to the device-based API?

I have added it back, but the file just includes experimental/spm.h now

ApoKalipse-V · 2026-03-10T16:02:47Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+    size_t                                                  record_count,
+    int                                                     flags,
+    rocprofiler_user_data_t                                 userdata,
+    void*                                                   record_callback_args);


Not a big deal, but I don't think this is necessary.
If the user wants to get their record_callback_args, they can link userdata to it from the dispatch_callback.

The record callback args is used by the user to send user data from configure function to this callback and userdata is the data passed from dispatch callback to this callback. I don't think we can remove record_callback_args.

Let's say user wants to pass a value (my_args) from configure() to dispatch_counting_record_cb_t. Call:

rocprofiler_configure_callback_spm_dispatch_service(..., my_args);

Then in rocprofiler_spm_dispatch_counting_service_cb_t(...):

user_data->ptr = callback_data_args;

Then in rocprofiler_spm_dispatch_counting_record_cb_t():

void* my_args = userdata.ptr;

Not saying it's good, just saying it's possible. It's how data was passed in SQTT. If we are going to have it this way in SPM, maybe we should change the SQTT API for standardization.

I have made API consistent with counter collection

ApoKalipse-V · 2026-03-12T20:49:21Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+    ROCPROFILER_SPM_RECORD_FLAG_DATA_LOST = 0,  ///< records with data loss
+    ROCPROFILER_SPM_RECORD_FLAG_DATA,           ///< records with data
+    ROCPROFILER_SPM_RECORD_FLAG_END,            ///< End of agent service
+    ROCPROFILER_SPM_RECORD_FLAG_DATA_NONE,      //< flag value none


What does flag_none mean here? Is this new or have I just forgot?

NONE is usually first... LOST should be something > 0

jrmadsen · 2026-03-05T19:13:23Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+ROCPROFILER_SDK_EXPERIMENTAL rocprofiler_status_t
+rocprofiler_iterate_spm_supported_counters(rocprofiler_agent_id_t              agent_id,
+                                           rocprofiler_available_counters_cb_t cb,
+                                           void* user_data) ROCPROFILER_API ROCPROFILER_NONNULL(2);


@bwelton NONNULL starts at 1 for the arguments. The index of 0 is for the return type I believe

jrmadsen · 2026-03-17T15:40:53Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+                       ///< to determine sample interval.
+    uint64_t buffer_size;  ///< Buffer size of user mode buffer in KB
+    uint64_t timeout;      ///< Timeout for the user mode buffer in ms
+


Suggested change

jrmadsen · 2026-03-17T15:42:04Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+                                      size_t                               counters_count,
+                                      rocprofiler_spm_configuration_t*     parameters,
+                                      rocprofiler_spm_counter_config_id_t* config_id)
+    ROCPROFILER_API ROCPROFILER_NONNULL(2);


Suggested change

ROCPROFILER_API ROCPROFILER_NONNULL(2);

ROCPROFILER_API ROCPROFILER_NONNULL(2, 4);

SPM parameters can be null, there are set to default values if not provided

jrmadsen · 2026-03-17T15:43:50Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+                                      rocprofiler_counter_id_t*            counters_list,
+                                      size_t                               counters_count,
+                                      rocprofiler_spm_configuration_t*     parameters,
+                                      rocprofiler_spm_counter_config_id_t* config_id)


We are re-using rocprofiler_counter_id_t so why not re-use rocprofiler_spm_counter_config_id_t?

jrmadsen · 2026-03-17T15:54:34Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+ * @param [in] config_id
+ * @return ::rocprofiler_status_t
+ * @retval ROCPROFILER_STATUS_SUCCESS if config destroyed
+ * @retval ROCPROFILER_STATUS_ERROR if config could not be destroyed


It seems like there should be a return value of ROCPROFILER_STATUS_ERROR_PROFILE_NOT_FOUND.

In another PR, we should alias ROCPROFILER_STATUS_ERROR_PROFILE_NOT_FOUND to ROCPROFILER_STATUS_ERROR_CONFIG_NOT_FOUND... e.g.:

... ROCPROFILER_STATUS_ERROR_CONFIG_NOT_FOUND, // replace the original PROFILE ... ROCPROFILER_STATUS_ERROR_PROFILE_NOT_FOUND = ROCPROFILER_STATUS_ERROR_CONFIG_NOT_FOUND, // add this to end with comment that it is deprecated.

Also, the rocprofiler_get_status_name should be updated to return ROCPROFILER_STATUS_ERROR_CONFIG_NOT_FOUND and rocprofiler_get_status_string should be updated (if necessary) to use "config" instead of "profile"

jrmadsen · 2026-03-17T15:56:08Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+    ROCPROFILER_SPM_RECORD_FLAG_DATA_LOST = 0,  ///< records with data loss
+    ROCPROFILER_SPM_RECORD_FLAG_DATA,           ///< records with data
+    ROCPROFILER_SPM_RECORD_FLAG_END,            ///< End of agent service
+    ROCPROFILER_SPM_RECORD_FLAG_DATA_NONE,      //< flag value none


NONE is usually first... LOST should be something > 0

jrmadsen · 2026-03-17T15:56:27Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+    ROCPROFILER_SPM_RECORD_FLAG_DATA_LOST = 0,  ///< records with data loss
+    ROCPROFILER_SPM_RECORD_FLAG_DATA,           ///< records with data
+    ROCPROFILER_SPM_RECORD_FLAG_END,            ///< End of agent service
+    ROCPROFILER_SPM_RECORD_FLAG_DATA_NONE,      //< flag value none


Suggested change

ROCPROFILER_SPM_RECORD_FLAG_DATA_NONE, //< flag value none

ROCPROFILER_SPM_RECORD_FLAG_DATA_NONE, ///< flag value none

jrmadsen · 2026-03-17T15:57:59Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+{
+    uint64_t                           size;            ///< Size of this struct
+    rocprofiler_async_correlation_id_t correlation_id;  ///< Correlation ID for this dispatch
+    rocprofiler_kernel_dispatch_info_t dispatch_info;   ///< Dispatch info


Where is the timestamp like in rocprofiler_dispatch_counting_service_data_t?

The start and end timestamp of kernel is not available in SPM due to it be streaming

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

jrmadsen · 2026-03-17T16:07:04Z

projects/rocprofiler-sdk/source/include/rocprofiler-sdk/experimental/spm.h

+} rocprofiler_spm_counter_record_t;
+
+/**
+ * @brief (experimental) Callback to receive SPM data


receive SPM data from? Callback? Buffer records?

SrirakshaNag requested review from a team as code owners March 3, 2026 01:11

github-actions bot added the project: rocprofiler-sdk label Mar 3, 2026

systems-assistant bot added the organization: ROCm label Mar 3, 2026

SrirakshaNag requested review from MythreyaK and bing-ma March 3, 2026 17:06

bwelton reviewed Mar 4, 2026

View reviewed changes

ApoKalipse-V reviewed Mar 10, 2026

View reviewed changes

SrirakshaNag force-pushed the users/SrirakshaNag/spm-patch-chain/spm_api branch from 3fe8fd2 to 71b724f Compare March 10, 2026 23:13

Add SPM experimental public API header

c2ab2ff

SrirakshaNag force-pushed the users/SrirakshaNag/spm-patch-chain/spm_api branch from 71b724f to c2ab2ff Compare March 10, 2026 23:44

ApoKalipse-V reviewed Mar 12, 2026

View reviewed changes

jrmadsen requested changes Mar 17, 2026

View reviewed changes

jrmadsen changed the title ~~Add SPM experimental public API header~~ [rocprofiler-sdk] Add SPM experimental public API header Mar 17, 2026

SrirakshaNag added 2 commits March 19, 2026 09:56

Add SPM experimental public API header

3a44e18

Add SPM experimental public API header

f940146

bwelton approved these changes Mar 23, 2026

View reviewed changes

	ROCPROFILER_API ROCPROFILER_NONNULL(2);
	ROCPROFILER_API ROCPROFILER_NONNULL(2, 4);

	ROCPROFILER_SPM_RECORD_FLAG_DATA_NONE, //< flag value none
	ROCPROFILER_SPM_RECORD_FLAG_DATA_NONE, ///< flag value none

Conversation

SrirakshaNag commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrmadsen commented Mar 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ApoKalipse-V Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SrirakshaNag commented Mar 3, 2026 •

edited

Loading

ApoKalipse-V Mar 12, 2026 •

edited

Loading